Refactor processSelectionSet field flattening to allow @client and @defer directives on fragment spreads
#8951
Conversation
| check(gql` | ||
| query Q { | ||
| ...FragAB @client | ||
| ...FragB | ||
| rootField | ||
| } | ||
|
|
||
| fragment FragAB on Query { | ||
| aField | ||
| ...FragB | ||
| } | ||
|
|
||
| fragment FragB on Query { | ||
| bField | ||
| } | ||
| `, { | ||
| aField: true, | ||
| bField: false, | ||
| rootField: false, | ||
| }); |
There was a problem hiding this comment.
Here's a good example of a field (bField) that's visited more than once by the flattening algorithm, since ...FragB is used twice, but the two different paths have different @client configurations, thanks to ...FragAB @client, so the non-@client one "wins" (because the bField cannot be entirely skipped on the server).
| selection.directives && | ||
| selection.directives.some(d => d.name.value === "client") | ||
| ); | ||
| ).forEach((context, field) => { |
There was a problem hiding this comment.
This context parameter shadows the one passed into processSelectionSet, though they will be === in many cases (for example, when @client and/or @defer are not present in the query).
| } else if ( | ||
| __DEV__ && | ||
| !context.clientOnly && | ||
| !context.deferred && |
There was a problem hiding this comment.
Here's the one place where we currently consume context.deferred.
| // If this field has been visited along another recursive path | ||
| // before, the final context should have clientOnly or deferred set | ||
| // to true only if *all* paths have the directive (hence the &&). | ||
| clientOnly = clientOnly && existing.clientOnly; | ||
| deferred = deferred && existing.deferred; |
There was a problem hiding this comment.
Here's where we ultimately determine if any of the paths to a given field do not have @client, because that means the field should not be considered client-only.
Similar logic applies to @defer: if the field shows up in any non-deferred path through the query, the field should be expected in the initial result, rather than arriving later.
src/cache/inmemory/writeToStore.ts
Outdated
| // Directive metadata for @client and @defer. We could use a bitfield for this | ||
| // information to save some space, if that matters. | ||
| clientOnly: boolean; | ||
| deferred: boolean; |
There was a problem hiding this comment.
As a counterpoint to the bitfield idea, we may eventually want these value types to provide more detailed metadata about the @client or @defer directive(s) that are currently active, rather than just a boolean, but that's all we need for now.
src/cache/inmemory/writeToStore.ts
Outdated
| getFragmentFromSelection(inlineOrSpread, context.fragmentMap); | ||
| if (inlineOrDefinition && | ||
| policies.fragmentMatches( | ||
| inlineOrDefinition, typename, result, context.variables)) { |
There was a problem hiding this comment.
Is it a problem that typename might not match for interface/union fragments?
There was a problem hiding this comment.
The policies.fragmentMatches method knows all about that, thanks to possibleTypes.
There was a problem hiding this comment.
Yes but it’s okay that typename is for the original selectionSet and not the typename from the fragment? I’m probably just being dumb about this.
benjamn
force-pushed
the
client-directive-on-fragment-spreads
branch
from
128722e
to
8f5708a
Compare
A query selection set may contain a mixture of fields and fragment spreads (either inline or named), but once a concrete result object with a
__typenameis available, the fragments can be flattened into their constituent fields (depending on whether each fragment matches the given__typename), following the CollectFields pseudocode algorithm.Previously, this flattening was intermingled with the processing of result data in
StoreWriter#processSelectionSet, but the flattening can be performed in its entirety before callingprocessFieldValuefor any fields, which makes the code easier to reason about, and possibly a bit faster. This PR introduces a newStoreWriterhelper method,flattenFields, which performs that field flattening, closely following CollectFields.A side-goal of this work was to fix a long-standing issue where
@clientdirectives on fragment spreads were ignored, because the fragment spreadSelectionNodethat has the relevantselection.directivesinformation was removed by the previous flattening algorithm. Unfortunately, this meansflattenFieldspotentially needs to provide differentWriteContextobjects for different fields, depending on whether the field's path has a@clientdirective. This new functionality requires a slight deviation from thevisitedFragmentslogic in CollectFields, because a fragment may now be revisited if encountered more than once with different@clientconfigurations. Fortunately, the revisitation logic can be confined withinflattenFields, so it does not need to clutterprocessSelectionSet.As a bonus, my effort to reimplement
@clienthandling extends naturally to@defer(see f8ccf8d), since@deferis also a directive that "cascades" (applies to the whole subtree). Initially, this allows us to avoid warning about missing fields that are deferred, the same way we avoid warning about missing@clientfields, but it will no doubt have more use when we fully implement@defersupport (see #8184).