Repository: urql-graphql/urql
Pull request #3517 (2024-03-02 17:30:03)

fix(graphcache): Cascade `@defer`, `@_optional`, and `@_required` state only per nested fragment spread
## Summary

This fixes `@_optional` to work when the selection contains a sub-selection fragment spread. Previously, other fragment spreads inside an `@_optional` spread would become non-optional/non-required again, since the value of `deferRef` would be reset with another `isOptional()` check.

This also applies the same principle to `@defer`. Previously, `@defer` would cascade to all child selection fields. This meant that any field nested inside a `@defer`-ed fragment would be able to be deferred and be completely optional, even if it's direct parent wasn't a deferred fragment spread.

## Set of changes

- Remove `deferRef` reset from `makeContext`
- Reset `deferRef` with every new child field and only cascade it across fragment spreads
- Cascade `optionalRef` across fragment spreads (but also not new child fields)
- Add overload signature to annotate what the preferred call signature of `makeSelectionIterator` is to prevent future mistakes


changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 605066c2f19104f415f574d208849d5969221db7

**The changes in this PR will be included in the next version bump.**

<details><summary>This PR includes changesets to release 1 package</summary>

  | Name                      | Type  |
| ------------------------- | ----- |
| @urql/exchange-graphcache | Patch |

</details>

Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/urql-graphql/urql/new/fix/defer-optional-required-update?filename=.changeset/new-humans-jump.md&value=---%0A%22%40urql%2Fexchange-graphcache%22%3A%20patch%0A---%0A%0Afix(graphcache)%3A%20Cascade%20%60%40defer%60%2C%20%60%40_optional%60%2C%20and%20%60%40_required%60%20state%20only%20per%20nested%20fragment%20spread%0A)



Patch:
diff --git a/.changeset/sixty-needles-exercise.md b/.changeset/sixty-needles-exercise.md
new file mode 100644
index 00000000..b788d682
--- /dev/null
+++ b/.changeset/sixty-needles-exercise.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+Prevent `@defer` from being applied in child field selections. Previously, a child field (i.e. a nested field) under a `@defer`-ed fragment would also become optional, which was based on a prior version of the DeferStream spec which didn't require deferred fields to be delivered as a group.
diff --git a/exchanges/graphcache/src/operations/query.ts b/exchanges/graphcache/src/operations/query.ts
index c85725e7..55e2ad35 100644
--- a/exchanges/graphcache/src/operations/query.ts
+++ b/exchanges/graphcache/src/operations/query.ts
@@ -145,7 +145,7 @@ const readRoot = (
   const iterate = makeSelectionIterator(
     entityKey,
     entityKey,
-    deferRef,
+    false,
     undefined,
     select,
     ctx
@@ -390,7 +390,7 @@ const readSelection = (
   const iterate = makeSelectionIterator(
     typename,
     entityKey,
-    deferRef,
+    false,
     undefined,
     select,
     ctx
diff --git a/exchanges/graphcache/src/operations/shared.ts b/exchanges/graphcache/src/operations/shared.ts
index 7ad1800e..2bb14782 100644
--- a/exchanges/graphcache/src/operations/shared.ts
+++ b/exchanges/graphcache/src/operations/shared.ts
@@ -71,8 +71,6 @@ export const makeContext = (
   entityKey: string,
   error: CombinedError | undefined
 ): Context => {
-  deferRef = false;
-
   const ctx: Context = {
     store,
     variables,
@@ -163,14 +161,33 @@ interface SelectionIterator {
   (): FormattedNode<FieldNode> | undefined;
 }
 
-export const makeSelectionIterator = (
-  typename: void | string,
+// NOTE: Outside of this file, we expect `_defer` to always be reset to `false`
+export function makeSelectionIterator(
+  typename: undefined | string,
+  entityKey: string,
+  _defer: false,
+  _optional: undefined,
+  selectionSet: FormattedNode<SelectionSet>,
+  ctx: Context
+): SelectionIterator;
+// NOTE: Inside this file we expect the state to be recursively passed on
+export function makeSelectionIterator(
+  typename: undefined | string,
+  entityKey: string,
+  _defer: boolean,
+  _optional: undefined | boolean,
+  selectionSet: FormattedNode<SelectionSet>,
+  ctx: Context
+): SelectionIterator;
+
+export function makeSelectionIterator(
+  typename: undefined | string,
   entityKey: string,
-  defer: boolean,
-  optional: boolean | undefined,
+  _defer: boolean,
+  _optional: boolean | undefined,
   selectionSet: FormattedNode<SelectionSet>,
   ctx: Context
-): SelectionIterator => {
+): SelectionIterator {
   let child: SelectionIterator | void;
   let index = 0;
 
@@ -178,8 +195,8 @@ export const makeSelectionIterator = (
     let node: FormattedNode<FieldNode> | undefined;
     while (child || index < selectionSet.length) {
       node = undefined;
-      deferRef = defer;
-      optionalRef = optional;
+      deferRef = _defer;
+      optionalRef = _optional;
       if (child) {
         if ((node = child())) {
           return node;
@@ -212,13 +229,14 @@ export const makeSelectionIterator = (
             if (isMatching) {
               if (process.env.NODE_ENV !== 'production')
                 pushDebugNode(typename, fragment);
-
               const isFragmentOptional = isOptional(select);
               child = makeSelectionIterator(
                 typename,
                 entityKey,
-                defer || isDeferred(select, ctx.variables),
-                isFragmentOptional,
+                _defer || isDeferred(select, ctx.variables),
+                isFragmentOptional !== undefined
+                  ? isFragmentOptional
+                  : _optional,
                 getSelectionSet(fragment),
                 ctx
               );
@@ -230,7 +248,7 @@ export const makeSelectionIterator = (
       }
     }
   };
-};
+}
 
 export const ensureData = (x: DataField): Data | NullArray<Data> | null =>
   x == null ? null : (x as Data | NullArray<Data>);
diff --git a/exchanges/graphcache/src/operations/write.ts b/exchanges/graphcache/src/operations/write.ts
index a2410178..e16065f3 100644
--- a/exchanges/graphcache/src/operations/write.ts
+++ b/exchanges/graphcache/src/operations/write.ts
@@ -239,7 +239,7 @@ const writeSelection = (
   const iterate = makeSelectionIterator(
     typename,
     entityKey || typename,
-    deferRef,
+    false,
     undefined,
     select,
     ctx


Test patch:
diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts
index cc115732..2d2a04c5 100644
--- a/exchanges/graphcache/src/cacheExchange.test.ts
+++ b/exchanges/graphcache/src/cacheExchange.test.ts
@@ -909,8 +909,10 @@ describe('directives', () => {
         todos {
           id
           text
-          ... on Todo @_optional {
-            completed
+          ... @_optional {
+            ... on Todo {
+              completed
+            }
           }
         }
       }