Skip to content

Commit 419353c

Browse files
monicatangfacebook-github-bot
authored andcommitted
add id collision logging in RelayResponseNormalizer with typename metadata
Reviewed By: captbaritone Differential Revision: D71346432 fbshipit-source-id: 29bfb713161c28776a75c69ac62280cdcc176dfb
1 parent 9a7d202 commit 419353c

9 files changed

+54
-190
lines changed

packages/relay-runtime/store/OperationExecutor.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ class Executor<TMutation: MutationParameters> {
612612
{
613613
actorIdentifier: this._actorIdentifier,
614614
getDataID: this._getDataID,
615+
log: this._log,
615616
path: [],
616617
shouldProcessClientComponents: this._shouldProcessClientComponents,
617618
treatMissingFieldsAsNull,
@@ -721,6 +722,7 @@ class Executor<TMutation: MutationParameters> {
721722
{
722723
actorIdentifier: this._actorIdentifier,
723724
getDataID: this._getDataID,
725+
log: this._log,
724726
path: followupPayload.path,
725727
treatMissingFieldsAsNull: this._treatMissingFieldsAsNull,
726728
shouldProcessClientComponents: this._shouldProcessClientComponents,
@@ -803,6 +805,7 @@ class Executor<TMutation: MutationParameters> {
803805
{
804806
actorIdentifier: this._actorIdentifier,
805807
getDataID: this._getDataID,
808+
log: this._log,
806809
path: [],
807810
treatMissingFieldsAsNull: this._treatMissingFieldsAsNull,
808811
shouldProcessClientComponents: this._shouldProcessClientComponents,
@@ -1259,6 +1262,7 @@ class Executor<TMutation: MutationParameters> {
12591262
{
12601263
actorIdentifier: this._actorIdentifier,
12611264
getDataID: this._getDataID,
1265+
log: this._log,
12621266
path: placeholder.path,
12631267
treatMissingFieldsAsNull: this._treatMissingFieldsAsNull,
12641268
shouldProcessClientComponents: this._shouldProcessClientComponents,
@@ -1486,6 +1490,7 @@ class Executor<TMutation: MutationParameters> {
14861490
const relayPayload = this._normalizeResponse(response, selector, typeName, {
14871491
actorIdentifier: this._actorIdentifier,
14881492
getDataID: this._getDataID,
1493+
log: this._log,
14891494
path: [...normalizationPath, responseKey, String(itemIndex)],
14901495
treatMissingFieldsAsNull: this._treatMissingFieldsAsNull,
14911496
shouldProcessClientComponents: this._shouldProcessClientComponents,

packages/relay-runtime/store/RelayModernStore.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,7 @@ class RelayModernStore implements Store {
811811
return {
812812
path,
813813
getDataID: this._getDataID,
814+
log: this.__log,
814815
treatMissingFieldsAsNull: this._treatMissingFieldsAsNull,
815816
shouldProcessClientComponents: this._shouldProcessClientComponents,
816817
actorIdentifier: this._actorIdentifier,

packages/relay-runtime/store/RelayResponseNormalizer.js

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import type {
3030
FollowupPayload,
3131
HandleFieldPayload,
3232
IncrementalDataPlaceholder,
33+
LogFunction,
3334
MutableRecordSource,
3435
NormalizationSelector,
3536
Record,
@@ -72,6 +73,7 @@ export type GetDataID = (
7273
export type NormalizationOptions = {
7374
+getDataID: GetDataID,
7475
+treatMissingFieldsAsNull: boolean,
76+
+log: ?LogFunction,
7577
+path?: $ReadOnlyArray<string>,
7678
+shouldProcessClientComponents?: ?boolean,
7779
+actorIdentifier?: ?ActorIdentifier,
@@ -116,6 +118,7 @@ class RelayResponseNormalizer {
116118
_variables: Variables;
117119
_shouldProcessClientComponents: ?boolean;
118120
_errorTrie: RelayErrorTrie | null;
121+
_log: ?LogFunction;
119122

120123
constructor(
121124
recordSource: MutableRecordSource,
@@ -134,6 +137,7 @@ class RelayResponseNormalizer {
134137
this._recordSource = recordSource;
135138
this._variables = variables;
136139
this._shouldProcessClientComponents = options.shouldProcessClientComponents;
140+
this._log = options.log;
137141
}
138142

139143
normalizeResponse(
@@ -792,19 +796,22 @@ class RelayResponseNormalizer {
792796
field: NormalizationLinkedField,
793797
payload: Object,
794798
): void {
795-
const log = RelayFeatureFlags.LOG_STORE_ID_COLLISION;
799+
const log = RelayFeatureFlags.ENABLE_STORE_ID_COLLISION_LOGGING;
796800
if (log) {
797801
const typeName = field.concreteType ?? this._getRecordType(payload);
798802
const dataID = RelayModernRecord.getDataID(record);
799803
const shouldLogWarning =
800804
(isClientID(dataID) && dataID !== ROOT_ID) ||
801805
RelayModernRecord.getType(record) === typeName;
802806
if (shouldLogWarning) {
803-
log({
807+
const logEvent = {
804808
name: 'idCollision.typename',
805809
previous_typename: RelayModernRecord.getType(record),
806810
new_typename: typeName,
807-
});
811+
};
812+
if (this._log != null) {
813+
this._log(logEvent);
814+
}
808815
}
809816
}
810817
// NOTE: Only emit a warning in DEV
@@ -836,17 +843,6 @@ class RelayResponseNormalizer {
836843
storageKey: string,
837844
fieldValue: mixed,
838845
): void {
839-
const log = RelayFeatureFlags.LOG_STORE_ID_COLLISION;
840-
if (log) {
841-
const previousValue = RelayModernRecord.getValue(record, storageKey);
842-
const shouldLogWarning =
843-
storageKey === TYPENAME_KEY ||
844-
previousValue === undefined ||
845-
areEqual(previousValue, fieldValue);
846-
if (shouldLogWarning) {
847-
log({name: 'idCollision.field'});
848-
}
849-
}
850846
// NOTE: Only emit a warning in DEV
851847
if (__DEV__) {
852848
const previousValue = RelayModernRecord.getValue(record, storageKey);
@@ -877,13 +873,6 @@ class RelayResponseNormalizer {
877873
nextID: DataID,
878874
storageKey: string,
879875
): void {
880-
const log = RelayFeatureFlags.LOG_STORE_ID_COLLISION;
881-
if (log) {
882-
const shouldLogWarning = prevID === undefined || prevID === nextID;
883-
if (shouldLogWarning) {
884-
log({name: 'idCollision.field'});
885-
}
886-
}
887876
// NOTE: Only emit a warning in DEV
888877
if (__DEV__) {
889878
const shouldLogWarning = prevID === undefined || prevID === nextID;

packages/relay-runtime/store/RelayStoreTypes.js

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -796,23 +796,13 @@ export type IdCollisionTypenameLogEvent = {
796796
+new_typename: string,
797797
};
798798

799-
/**
800-
* This event is logged when a response contains fields with different
801-
* values but which have the same corresponding id, resulting in a
802-
* collision in the store.
803-
*/
804-
export type IdCollisionFieldLogEvent = {
805-
+name: 'idCollision.field',
806-
};
807-
808799
export type LogEvent =
809800
| SuspenseFragmentLogEvent
810801
| SuspenseQueryLogEvent
811802
| QueryResourceFetchLogEvent
812803
| QueryResourceRetainLogEvent
813804
| FragmentResourceMissingDataLogEvent
814805
| IdCollisionTypenameLogEvent
815-
| IdCollisionFieldLogEvent
816806
| PendingOperationFoundLogEvent
817807
| NetworkInfoLogEvent
818808
| NetworkStartLogEvent

packages/relay-runtime/store/__tests__/RelayExperimentalGraphResponseHandler-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const {getRequest} = require('relay-runtime/query/GraphQLTag');
3030
const defaultOptions = {
3131
getDataID: defaultGetDataID,
3232
treatMissingFieldsAsNull: false,
33+
log: null,
3334
};
3435

3536
function applyTransform(

packages/relay-runtime/store/__tests__/RelayExperimentalGraphResponseTransform-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {ROOT_ID} = require('relay-runtime/store/RelayStoreUtils');
3232
const defaultOptions = {
3333
getDataID: defaultGetDataID,
3434
treatMissingFieldsAsNull: false,
35+
log: null,
3536
};
3637

3738
function applyTransform(

0 commit comments

Comments
 (0)