Skip to content

Commit 5e910cd

Browse files
committed
Don't corrupt type registry with false abstract-type tag under deferDeduplicatedFields
RelayResponseNormalizer's TypeDiscriminator and InlineFragment normalization paths unconditionally write 'client:__type:<ConcreteType>.__is<Interface>: <hasOwnProperty>' during payload traversal. Without deferDeduplicatedFields, this is fine: a missing __isFoo tag in a normalized payload is a real 'type does not implement Foo' signal, and the false write is load-bearing so DataChecker can skip future '... on Foo' selections for that concrete type. Under Environment({ deferDeduplicatedFields: true }), the same missing tag can just mean 'delivered earlier in a different chunk of this operation, dedup'd out of the current chunk'. Writing false in that case is wrong: it plants a schema-contradicting negative claim in the type registry, and DataChecker (which treats false as authoritative non-implementation) then silently skips '... on <Interface>' subtrees for every future query touching that concrete type — never marking anything missing, never fetching. Symptom in our app: after an SSR page load that fetches a defer'd query, client-navigations to any page that reads a Cloudcast via 'node(id) { ... on Node { ... on Cloudcast ... } }' rendered null for every field inside — reader saw $isWithinUnmatchedTypeRefinement:true because DataChecker had marked the query 'available' without ever fetching, thanks to __type:Cloudcast.__isNode=false written during the defer'd chunk. Fix: skip the write when 'hasOwnProperty === false && this ._deferDeduplicatedFields'. When the write is skipped, DataChecker treats the registry entry as unknown (_implementsInterface == null) → handles missing → fetches on next check → the response arrives with the tag → true written normally. Non-dedup environments are byte-identical to the previous behaviour. Filed as #5389.
1 parent 5bd5328 commit 5e910cd

2 files changed

Lines changed: 166 additions & 18 deletions

File tree

packages/relay-runtime/store/RelayResponseNormalizer.js

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -283,18 +283,24 @@ class RelayResponseNormalizer {
283283
data,
284284
abstractKey,
285285
);
286-
const typeName = RelayModernRecord.getType(record);
287-
const typeID = generateTypeID(typeName);
288-
let typeRecord = this._recordSource.get(typeID);
289-
if (typeRecord == null) {
290-
typeRecord = RelayModernRecord.create(typeID, TYPE_SCHEMA_TYPE);
291-
this._recordSource.set(typeID, typeRecord);
286+
// Under `deferDeduplicatedFields` a missing abstract typename means
287+
// the server omitted it as already-delivered, not that the concrete
288+
// type fails the type refinement. Skip the write; DataChecker then
289+
// treats the entry as unknown and refetches on the next check.
290+
if (implementsInterface || !this._deferDeduplicatedFields) {
291+
const typeName = RelayModernRecord.getType(record);
292+
const typeID = generateTypeID(typeName);
293+
let typeRecord = this._recordSource.get(typeID);
294+
if (typeRecord == null) {
295+
typeRecord = RelayModernRecord.create(typeID, TYPE_SCHEMA_TYPE);
296+
this._recordSource.set(typeID, typeRecord);
297+
}
298+
RelayModernRecord.setValue(
299+
typeRecord,
300+
abstractKey,
301+
implementsInterface,
302+
);
292303
}
293-
RelayModernRecord.setValue(
294-
typeRecord,
295-
abstractKey,
296-
implementsInterface,
297-
);
298304
break;
299305
}
300306
case 'LinkedHandle':
@@ -402,14 +408,21 @@ class RelayResponseNormalizer {
402408
data,
403409
abstractKey,
404410
);
405-
const typeName = RelayModernRecord.getType(record);
406-
const typeID = generateTypeID(typeName);
407-
let typeRecord = this._recordSource.get(typeID);
408-
if (typeRecord == null) {
409-
typeRecord = RelayModernRecord.create(typeID, TYPE_SCHEMA_TYPE);
410-
this._recordSource.set(typeID, typeRecord);
411+
// See the `TypeDiscriminator` case above for the reasoning.
412+
if (implementsInterface || !this._deferDeduplicatedFields) {
413+
const typeName = RelayModernRecord.getType(record);
414+
const typeID = generateTypeID(typeName);
415+
let typeRecord = this._recordSource.get(typeID);
416+
if (typeRecord == null) {
417+
typeRecord = RelayModernRecord.create(typeID, TYPE_SCHEMA_TYPE);
418+
this._recordSource.set(typeID, typeRecord);
419+
}
420+
RelayModernRecord.setValue(
421+
typeRecord,
422+
abstractKey,
423+
implementsInterface,
424+
);
411425
}
412-
RelayModernRecord.setValue(typeRecord, abstractKey, implementsInterface);
413426
if (implementsInterface) {
414427
this._traverseSelections(selection, record, data);
415428
}

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4390,4 +4390,139 @@ describe('RelayResponseNormalizer', () => {
43904390
});
43914391
});
43924392
});
4393+
4394+
describe('deferDeduplicatedFields', () => {
4395+
it('does not write `__is<Interface>: false` when the abstract typename is missing from the payload', () => {
4396+
const query = graphql`
4397+
query RelayResponseNormalizerTest44Query($id: ID!) {
4398+
node(id: $id) {
4399+
...RelayResponseNormalizerTest44Fragment
4400+
}
4401+
}
4402+
`;
4403+
4404+
graphql`
4405+
fragment RelayResponseNormalizerTest44Fragment on Node {
4406+
id
4407+
... on User {
4408+
name
4409+
}
4410+
}
4411+
`;
4412+
4413+
// Payload omits `__isNode`: under deferDeduplicatedFields this signals
4414+
// that the server dedup'd an already-delivered field, not that the
4415+
// concrete type fails the type refinement.
4416+
const payload = {
4417+
node: {
4418+
__typename: 'User',
4419+
id: '1',
4420+
name: 'Alice',
4421+
},
4422+
};
4423+
4424+
const recordSource = new RelayRecordSource();
4425+
recordSource.set(ROOT_ID, RelayModernRecord.create(ROOT_ID, ROOT_TYPE));
4426+
4427+
normalize(
4428+
recordSource,
4429+
createNormalizationSelector(query.operation, ROOT_ID, {id: '1'}),
4430+
payload,
4431+
{...defaultOptions, deferDeduplicatedFields: true},
4432+
);
4433+
4434+
// No `client:__type:User` record — the write is skipped so
4435+
// DataChecker treats the entry as unknown and refetches on next
4436+
// check.
4437+
expect(recordSource.get('client:__type:User')).toBe(undefined);
4438+
});
4439+
4440+
it('still writes `__is<Interface>: true` when the abstract typename is present in the payload', () => {
4441+
const query = graphql`
4442+
query RelayResponseNormalizerTest45Query($id: ID!) {
4443+
node(id: $id) {
4444+
...RelayResponseNormalizerTest45Fragment
4445+
}
4446+
}
4447+
`;
4448+
4449+
graphql`
4450+
fragment RelayResponseNormalizerTest45Fragment on Node {
4451+
id
4452+
... on User {
4453+
name
4454+
}
4455+
}
4456+
`;
4457+
4458+
const payload = {
4459+
node: {
4460+
__typename: 'User',
4461+
__isNode: 'User',
4462+
id: '1',
4463+
name: 'Alice',
4464+
},
4465+
};
4466+
4467+
const recordSource = new RelayRecordSource();
4468+
recordSource.set(ROOT_ID, RelayModernRecord.create(ROOT_ID, ROOT_TYPE));
4469+
4470+
normalize(
4471+
recordSource,
4472+
createNormalizationSelector(query.operation, ROOT_ID, {id: '1'}),
4473+
payload,
4474+
{...defaultOptions, deferDeduplicatedFields: true},
4475+
);
4476+
4477+
expect(recordSource.get('client:__type:User')).toEqual({
4478+
__id: 'client:__type:User',
4479+
__typename: '__TypeSchema',
4480+
__isNode: true,
4481+
});
4482+
});
4483+
4484+
it('still writes `__is<Interface>: false` when deferDeduplicatedFields is off (default behaviour preserved)', () => {
4485+
const query = graphql`
4486+
query RelayResponseNormalizerTest46Query($id: ID!) {
4487+
node(id: $id) {
4488+
...RelayResponseNormalizerTest46Fragment
4489+
}
4490+
}
4491+
`;
4492+
4493+
graphql`
4494+
fragment RelayResponseNormalizerTest46Fragment on Node {
4495+
id
4496+
... on User {
4497+
name
4498+
}
4499+
}
4500+
`;
4501+
4502+
const payload = {
4503+
node: {
4504+
__typename: 'Page',
4505+
id: '1',
4506+
},
4507+
};
4508+
4509+
const recordSource = new RelayRecordSource();
4510+
recordSource.set(ROOT_ID, RelayModernRecord.create(ROOT_ID, ROOT_TYPE));
4511+
4512+
normalize(
4513+
recordSource,
4514+
createNormalizationSelector(query.operation, ROOT_ID, {id: '1'}),
4515+
payload,
4516+
defaultOptions,
4517+
);
4518+
4519+
// Without deferDeduplicatedFields the missing `__isNode` is still a
4520+
// real "type does not implement" signal — behaviour unchanged.
4521+
expect(recordSource.get('client:__type:Page')).toEqual({
4522+
__id: 'client:__type:Page',
4523+
__typename: '__TypeSchema',
4524+
__isNode: false,
4525+
});
4526+
});
4527+
});
43934528
});

0 commit comments

Comments
 (0)