major(graphcache): remove warning and lookup fields by name rather th…

…an alias (#2616) * remove warning and lookup fields by name rather than alias * remove test * Add execution of recursive field value functions * compose into func * types 😅 * Update optimistic mutation test * Update docs and changeset Co-authored-by: Phil Pluckthun <[email protected]>
urql-graphql · Aug 19, 2022 · c6244da · c6244da
1 parent 9a69d53
commit c6244da
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 96 deletions.
diff --git a/.changeset/slimy-shrimps-run.md b/.changeset/slimy-shrimps-run.md
@@ -0,0 +1,9 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+Graphcache's `optimistic` option now accepts optimistic mutation resolvers that return fields by
+name rather than alias. Previously, depending on which mutation was run, the optimistic resolvers
+would read your optimistic data by field alias (i.e. "alias" for `alias: id` rather than "id").
+Instead, optimistic updates now correctly use field names and allow you to also pass resolvers as
+values on your optimistic config.
diff --git a/docs/api/graphcache.md b/docs/api/graphcache.md
@@ -145,11 +145,11 @@ interface OptimisticMutationConfig {
 A `OptimisticMutationResolver` receives three arguments when it's called: `variables`, `cache`, and
 `info`.
 
-| Argument    | Type     | Description                                                                                                 |
-| ----------- | -------- | ----------------------------------------------------------------------------------------------------------- |
-| `variables` | `object` | The variables that the given mutation received.                                                             |
-| `cache`     | `Cache`  | The cache using which data can be read or written. [See `Cache`.](#cache)                                   |
-| `info`      | `Info`   | Additional metadata and information about the current operation and the current field. [See `Info`.](#info) |
+| Argument | Type     | Description                                                                                                 |
+| -------- | -------- | ----------------------------------------------------------------------------------------------------------- |
+| `args`   | `object` | The arguments that the given mutation field received.                                                       |
+| `cache`  | `Cache`  | The cache using which data can be read or written. [See `Cache`.](#cache)                                   |
+| `info`   | `Info`   | Additional metadata and information about the current operation and the current field. [See `Info`.](#info) |
 
 [Read more about how to set up `optimistic` on the "Custom Updates"
 page.](../graphcache/cache-updates.md)

diff --git a/docs/graphcache/cache-updates.md b/docs/graphcache/cache-updates.md
@@ -475,11 +475,13 @@ that imitates the result that the API is assumed to send back:
 ```js
 const cache = cacheExchange({
   optimistic: {
-    favoriteTodo: (variables, cache, info) => ({
-      __typename: 'Todo',
-      id: variables.id,
-      favorite: true,
-    }),
+    favoriteTodo(args, cache, info) {
+      return {
+        __typename: 'Todo',
+        id: variables.id,
+        favorite: true,
+      };
+    },
   },
 });
 ```
@@ -495,6 +497,29 @@ doesn't contain then we'll see a warning, since this is a common mistake. To wor
 enough data we may use methods like `cache.readFragment` and `cache.resolve` to retrieve more data
 from our cache.
 
+If we'd like to make sure we don't compute more fields than we need, for instance because one
+mutation is run with several different selection sets, then we may pass nested optimistic resolver
+functions in our optimistic object, like so:
+
+```js
+const cache = cacheExchange({
+  optimistic: {
+    favoriteTodo(variables, cache, info) {
+      return {
+        __typename: 'Todo',
+        id: variables.id,
+        favorite(args, cache, info) {
+          return true;
+        },
+      },
+    },
+  },
+});
+```
+
+The function signature and arguments it receives is identical to the toplevel optimistic function
+you define.
+
 ### Variables for Optimistic Updates
 
 Sometimes it's not possible for us to retrieve all data that an optimistic update requires to create

diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts
@@ -556,76 +556,6 @@ describe('data dependencies', () => {
     expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(2);
   });
 
-  it('respects aliases in the optimistic update data that is written', () => {
-    jest.useFakeTimers();
-
-    const mutation = gql`
-      mutation {
-        concealed: concealAuthor
-      }
-    `;
-
-    const mutationData = {
-      __typename: 'Mutation',
-      concealed: true,
-    };
-
-    const client = createClient({ url: 'http://0.0.0.0' });
-    const { source: ops$, next } = makeSubject<Operation>();
-
-    jest.spyOn(client, 'reexecuteOperation').mockImplementation(next);
-
-    const opMutation = client.createRequestOperation('mutation', {
-      key: 1,
-      query: mutation,
-    });
-
-    const response = jest.fn(
-      (forwardOp: Operation): OperationResult => {
-        if (forwardOp.key === 1) {
-          return { operation: opMutation, data: mutationData };
-        }
-
-        return undefined as any;
-      }
-    );
-
-    const result = jest.fn();
-    const forward: ExchangeIO = ops$ => pipe(ops$, delay(1), map(response));
-
-    const updates = {
-      Mutation: {
-        concealAuthor: jest.fn(),
-      },
-    };
-
-    const optimistic = {
-      concealAuthor: jest.fn(() => true) as any,
-    };
-
-    pipe(
-      cacheExchange({ updates, optimistic })({
-        forward,
-        client,
-        dispatchDebug,
-      })(ops$),
-      tap(result),
-      publish
-    );
-
-    next(opMutation);
-    expect(optimistic.concealAuthor).toHaveBeenCalledTimes(1);
-    expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(1);
-
-    const data = updates.Mutation.concealAuthor.mock.calls[0][0];
-    // Expect both fields to exist
-    expect(data.concealed).toBe(true);
-    expect(data.concealAuthor).toBe(true);
-
-    jest.runAllTimers();
-    expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(2);
-  });
-
   it('marks errored null fields as uncached but delivers them as expected', () => {
     const client = createClient({ url: 'http://0.0.0.0' });
     const { source: ops$, next } = makeSubject<Operation>();
@@ -708,7 +638,9 @@ describe('optimistic updates', () => {
       concealAuthor: {
         __typename: 'Author',
         id: '123',
-        name: '[REDACTED OFFLINE]',
+        name() {
+          return '[REDACTED OFFLINE]';
+        },
       },
     };
 
@@ -772,6 +704,11 @@ describe('optimistic updates', () => {
     expect(optimistic.concealAuthor).toHaveBeenCalledTimes(1);
     expect(reexec).toHaveBeenCalledTimes(1);
 
+    expect(result.mock.calls[1][0]?.data).toMatchObject({
+      __typename: 'Query',
+      author: { name: '[REDACTED OFFLINE]' },
+    });
+
     jest.runAllTimers();
     expect(response).toHaveBeenCalledTimes(2);
     expect(result).toHaveBeenCalledTimes(4);

diff --git a/exchanges/graphcache/src/operations/write.ts b/exchanges/graphcache/src/operations/write.ts
@@ -24,6 +24,7 @@ import {
   OperationRequest,
   Dependencies,
   EntityField,
+  OptimisticMutationResolver,
 } from '../types';
 
 import {
@@ -224,15 +225,16 @@ const writeSelection = (
     const fieldArgs = getFieldArguments(node, ctx.variables);
     const fieldKey = keyOfField(fieldName, fieldArgs);
     const fieldAlias = getFieldAlias(node);
-    let fieldValue = data[fieldAlias];
+    let fieldValue = data[ctx.optimistic ? fieldName : fieldAlias];
 
     // Development check of undefined fields
     if (process.env.NODE_ENV !== 'production') {
-      if (!isRoot && fieldValue === undefined && !deferRef.current) {
-        const advice = ctx.optimistic
-          ? '\nYour optimistic result may be missing a field!'
-          : '';
-
+      if (
+        !isRoot &&
+        fieldValue === undefined &&
+        !deferRef.current &&
+        !ctx.optimistic
+      ) {
         const expected =
           node.selectionSet === undefined
             ? 'scalar (number, boolean, etc)'
@@ -243,8 +245,7 @@ const writeSelection = (
             fieldKey +
             '` is `undefined`, but the GraphQL query expects a ' +
             expected +
-            ' for this field.' +
-            advice,
+            ' for this field.',
           13
         );
 
@@ -266,16 +267,21 @@ const writeSelection = (
     // Add the current alias to the walked path before processing the field's value
     ctx.__internal.path.push(fieldAlias);
 
-    // Execute optimistic mutation functions on root fields
+    // Execute optimistic mutation functions on root fields, or execute recursive functions
+    // that have been returned on optimistic objects
+    let resolver: OptimisticMutationResolver | void;
     if (ctx.optimistic && isRoot) {
-      const resolver = ctx.store.optimisticMutations[fieldName];
-
+      resolver = ctx.store.optimisticMutations[fieldName];
       if (!resolver) continue;
+    } else if (ctx.optimistic && typeof fieldValue === 'function') {
+      resolver = fieldValue as any;
+    }
+
+    // Execute the field-level resolver to retrieve its data
+    if (resolver) {
       // We have to update the context to reflect up-to-date ResolveInfo
       updateContext(ctx, data, typename, typename, fieldKey, fieldName);
-      fieldValue = data[fieldAlias] = ensureData(
-        resolver(fieldArgs || {}, ctx.store, ctx)
-      );
+      fieldValue = ensureData(resolver(fieldArgs || {}, ctx.store, ctx));
     }
 
     if (node.selectionSet) {

diff --git a/exchanges/graphcache/src/types.ts b/exchanges/graphcache/src/types.ts
@@ -188,11 +188,23 @@ export type UpdatesConfig = {
   };
 };
 
+export type MakeFunctional<T> = T extends { __typename: string }
+  ? WithTypename<
+      {
+        [P in keyof T]?: MakeFunctional<T[P]>;
+      }
+    >
+  : OptimisticMutationResolver<Variables, T> | T;
+
 export type OptimisticMutationResolver<
   Args = Variables,
   Result = Link<Data> | Scalar
 > = {
-  bivarianceHack(vars: Args, cache: Cache, info: ResolveInfo): Result;
+  bivarianceHack(
+    vars: Args,
+    cache: Cache,
+    info: ResolveInfo
+  ): MakeFunctional<Result>;
 }['bivarianceHack'];
 
 export type OptimisticMutationConfig = {
@@ -226,3 +238,7 @@ export type Dependencies = Set<string>;
 
 /** The type of cache operation being executed. */
 export type OperationType = 'read' | 'write';
+
+export type WithTypename<T extends { __typename?: any }> = T & {
+  __typename: NonNullable<T['__typename']>;
+};