RelayResponseNormalizer: don't write false __isInterface under deferDeduplicatedFields - #5390
Open
jonreading81 wants to merge 1 commit into
Open
RelayResponseNormalizer: don't write false __isInterface under deferDeduplicatedFields#5390jonreading81 wants to merge 1 commit into
jonreading81 wants to merge 1 commit into
Conversation
jonreading81
force-pushed
the
jr-deferdedup-typeregistry-fix
branch
2 times, most recently
from
August 4, 2026 13:00
98b3652 to
5e910cd
Compare
…eduplicatedFields
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 facebook#5389.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5389.
Summary
RelayResponseNormalizer._normalizeTypeDiscriminatorand._normalizeInlineFragmentwriteclient:__type:<ConcreteType>.__is<Interface>unconditionally, usingObject.prototype.hasOwnProperty.call(data, abstractKey)as the value. UnderEnvironment({ deferDeduplicatedFields: true }), that misinterprets a legitimately-omitted abstract typename in an incremental@deferpayload as "type does not implement" — it writesfalseinto the type registry,DataCheckerreads it and takes the "skip fragment" branch, and every subsequent... on <Interface>selection on that concrete type is silently dropped without a missing-data signal. DownstreamuseLazyLoadQuery/usePreloadedQueryreportavailable, never fetch, and reads inside the fragment collapse tonullunder$isWithinUnmatchedTypeRefinement.Full reproduction and evidence-based trace in #5389.
Change
Only skip the type-registry write when
implementsInterface === false && this._deferDeduplicatedFields. When the write is skipped,DataCheckertreats the registry entry as unknown (_implementsInterface == null) →_handleMissing()→ fetches on next check → response arrives with the tag →trueis written normally.Environments that don't opt into
deferDeduplicatedFieldsare byte-identical to before: the missing tag is still an authoritative "type doesn't implement" signal there, andfalseis still written.Both write sites (
TypeDiscriminatorin_traverseSelectionsand theabstractKey != nullbranch of_normalizeInlineFragment) get the same guard.Test plan
@defer+deferDeduplicatedFields) — the corrupted__type:X.__is<Interface>: falsewrites are gone, downstream queries fetch correctly. Confirmed by instrumentingstore.publishand dumping the registry.deferDeduplicatedFields + missing taginteraction.Notes
Filed as a targeted, minimal patch — the wider question of whether writing
falseon a missing abstract typename is ever the right thing (even withoutdeferDeduplicatedFields) can be answered separately. The narrower guard here keeps the blast radius as small as possible.