More robust handling of local state default value when read function is defined (#12934)

Author: jerelmiller

.api-reports/api-report-cache.api.md

@@ -94,6 +94,7 @@ export abstract class ApolloCache {
     abstract removeOptimistic(id: string): void;
     // (undocumented)
     abstract reset(options?: Cache_2.ResetOptions): Promise<void>;
+    resolvesClientField?(typename: string, fieldName: string): boolean;
     abstract restore(serializedState: unknown): this;
     // (undocumented)
     transformDocument(document: DocumentNode): DocumentNode;
@@ -544,6 +545,8 @@ export class InMemoryCache extends ApolloCache {
     // (undocumented)
     reset(options?: Cache_2.ResetOptions): Promise<void>;
     // (undocumented)
+    resolvesClientField(typename: string, fieldName: string): boolean;
+    // (undocumented)
     restore(data: NormalizedCacheObject): this;
     // (undocumented)
     retain(rootId: string, optimistic?: boolean): number;

.api-reports/api-report-local-state.api.md

@@ -14,6 +14,7 @@ import type { NoInfer as NoInfer_2 } from '@apollo/client/utilities/internal';
 import type { OperationVariables } from '@apollo/client';
 import type { RemoveIndexSignature } from '@apollo/client/utilities/internal';
 import type { TypedDocumentNode } from '@apollo/client';
+import type { WatchQueryFetchPolicy } from '@apollo/client';
 
 // @public (undocumented)
 type InferContextValueFromResolvers<TResolvers> = TResolvers extends {
@@ -91,14 +92,15 @@ export class LocalState<TResolvers extends LocalState.Resolvers = LocalState.Res
     ]);
     addResolvers(resolvers: TResolvers): void;
     // (undocumented)
-    execute<TData = unknown, TVariables extends OperationVariables = OperationVariables>({ document, client, context, remoteResult, variables, onlyRunForcedResolvers, returnPartialData, }: {
+    execute<TData = unknown, TVariables extends OperationVariables = OperationVariables>({ document, client, context, remoteResult, variables, onlyRunForcedResolvers, returnPartialData, fetchPolicy, }: {
         document: DocumentNode | TypedDocumentNode<TData, TVariables>;
         client: ApolloClient;
         context: DefaultContext | undefined;
         remoteResult: FormattedExecutionResult<any> | undefined;
         variables: TVariables | undefined;
         onlyRunForcedResolvers?: boolean;
         returnPartialData?: boolean;
+        fetchPolicy: WatchQueryFetchPolicy;
     }): Promise<FormattedExecutionResult<TData>>;
     // (undocumented)
     getExportedVariables<TVariables extends OperationVariables = OperationVariables>({ document, client, context, variables, }: {

.api-reports/api-report.api.md

@@ -95,6 +95,7 @@ export abstract class ApolloCache {
     abstract removeOptimistic(id: string): void;
     // (undocumented)
     abstract reset(options?: Cache_2.ResetOptions): Promise<void>;
+    resolvesClientField?(typename: string, fieldName: string): boolean;
     abstract restore(serializedState: unknown): this;
     // (undocumented)
     transformDocument(document: DocumentNode): DocumentNode;
@@ -1381,6 +1382,8 @@ export class InMemoryCache extends ApolloCache {
     // (undocumented)
     reset(options?: Cache_2.ResetOptions): Promise<void>;
     // (undocumented)
+    resolvesClientField(typename: string, fieldName: string): boolean;
+    // (undocumented)
     restore(data: NormalizedCacheObject): this;
     // (undocumented)
     retain(rootId: string, optimistic?: boolean): number;
@@ -1577,14 +1580,15 @@ class LocalState<TResolvers extends LocalState.Resolvers = LocalState.Resolvers<
     ]);
     addResolvers(resolvers: TResolvers): void;
     // (undocumented)
-    execute<TData = unknown, TVariables extends OperationVariables = OperationVariables>({ document, client, context, remoteResult, variables, onlyRunForcedResolvers, returnPartialData, }: {
+    execute<TData = unknown, TVariables extends OperationVariables = OperationVariables>({ document, client, context, remoteResult, variables, onlyRunForcedResolvers, returnPartialData, fetchPolicy, }: {
         document: DocumentNode | TypedDocumentNode<TData, TVariables>;
         client: ApolloClient;
         context: DefaultContext | undefined;
         remoteResult: FormattedExecutionResult<any> | undefined;
         variables: TVariables | undefined;
         onlyRunForcedResolvers?: boolean;
         returnPartialData?: boolean;
+        fetchPolicy: WatchQueryFetchPolicy;
     }): Promise<FormattedExecutionResult<TData>>;
     // (undocumented)
     getExportedVariables<TVariables extends OperationVariables = OperationVariables>({ document, client, context, variables, }: {
@@ -2723,9 +2727,9 @@ interface WriteContext extends ReadMergeModifyContext {
 // src/core/ApolloClient.ts:362:5 - (ae-forgotten-export) The symbol "NextFetchPolicyContext" needs to be exported by the entry point index.d.ts
 // src/core/ObservableQuery.ts:368:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
 // src/core/QueryManager.ts:180:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
-// src/local-state/LocalState.ts:147:5 - (ae-forgotten-export) The symbol "LocalState" needs to be exported by the entry point index.d.ts
-// src/local-state/LocalState.ts:200:7 - (ae-forgotten-export) The symbol "LocalState" needs to be exported by the entry point index.d.ts
-// src/local-state/LocalState.ts:243:7 - (ae-forgotten-export) The symbol "LocalState" needs to be exported by the entry point index.d.ts
+// src/local-state/LocalState.ts:149:5 - (ae-forgotten-export) The symbol "LocalState" needs to be exported by the entry point index.d.ts
+// src/local-state/LocalState.ts:202:7 - (ae-forgotten-export) The symbol "LocalState" needs to be exported by the entry point index.d.ts
+// src/local-state/LocalState.ts:245:7 - (ae-forgotten-export) The symbol "LocalState" needs to be exported by the entry point index.d.ts
 
 // (No @packageDocumentation comment for this package)
 

.changeset/flat-worms-notice.md

@@ -0,0 +1,7 @@
+---
+"@apollo/client": minor
+---
+
+Don't set the fallback value of a `@client` field to `null` when a `read` function is defined. Instead the `read` function will be called with an `existing` value of `undefined` to allow default arguments to be used to set the returned value.
+
+When a `read` function is not defined nor is there a defined resolver for the field, warn and set the value to `null` only in that instance.

.changeset/perfect-crabs-smile.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Ensure `LocalState` doesn't try to read from the cache when using a `no-cache` fetch policy.

.changeset/shaggy-islands-yell.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Warn when using a `no-cache` fetch policy without a local resolver defined. `no-cache` queries do not read or write to the cache which meant `no-cache` queries are silently incomplete when the `@client` field value was handled by a cache `read` function.

.changeset/unlucky-cooks-rhyme.md

@@ -0,0 +1,7 @@
+---
+"@apollo/client": minor
+---
+
+Add an abstract `resolvesClientField` function to `ApolloCache` that can be used by caches to tell `LocalState` if it can resolve a `@client` field when a local resolver is not defined.
+
+`LocalState` will emit a warning and set a fallback value of `null` when no local resolver is defined and `resolvesClientField` returns `false`, or isn't defined. Returning `true` from `resolvesClientField` signals that a mechanism in the cache will set the field value. In this case, `LocalState` won't set the field value.

.size-limits.json

@@ -1,6 +1,6 @@
 {
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 44542,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 39461,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33696,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27707
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 44753,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 39420,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33901,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27727
 }

src/__tests__/local-state/general.ts

@@ -26,6 +26,13 @@ import {
 } from "@apollo/client/testing/internal";
 import { InvariantError } from "@apollo/client/utilities/invariant";
 
+const WARNINGS = {
+  MISSING_RESOLVER:
+    "Could not find a resolver for the '%s' field nor does the cache resolve the field. The field value has been set to `null`. Either define a resolver for the field or ensure the cache can resolve the value, for example, by adding a 'read' function to a field policy in 'InMemoryCache'.",
+  NO_CACHE:
+    "The '%s' field resolves the value from the cache, for example from a 'read' function, but a 'no-cache' fetch policy was used. The field value has been set to `null`. Either define a local resolver or use a fetch policy that uses the cache to ensure the field is resolved correctly.",
+};
+
 describe("General functionality", () => {
   test("should not impact normal non-@client use", async () => {
     const query = gql`
@@ -632,7 +639,7 @@ describe("Cache manipulation", () => {
     });
 
     expect(read).toHaveBeenCalledTimes(1);
-    expect(read).toHaveBeenCalledWith(null, expect.anything());
+    expect(read).toHaveBeenCalledWith(undefined, expect.anything());
     expect(console.warn).not.toHaveBeenCalled();
   });
 });
@@ -1510,3 +1517,149 @@ test("throws when executing subscriptions with client fields when local state is
     )
   );
 });
+
+test.each(["cache-first", "network-only"] as const)(
+  "sets existing value of `@client` field to undefined when read function is present",
+  async (fetchPolicy) => {
+    const query = gql`
+      query GetUser {
+        user {
+          firstName @client
+          lastName
+        }
+      }
+    `;
+
+    const read = jest.fn((value = "Fallback") => value);
+    const client = new ApolloClient({
+      cache: new InMemoryCache({
+        typePolicies: {
+          User: {
+            fields: {
+              firstName: {
+                read,
+              },
+            },
+          },
+        },
+      }),
+      link: new ApolloLink(() => {
+        return of({
+          data: { user: { __typename: "User", lastName: "Smith" } },
+        }).pipe(delay(10));
+      }),
+      localState: new LocalState(),
+    });
+
+    await expect(
+      client.query({ query, fetchPolicy })
+    ).resolves.toStrictEqualTyped({
+      data: {
+        user: { __typename: "User", firstName: "Fallback", lastName: "Smith" },
+      },
+    });
+
+    expect(read).toHaveBeenCalledTimes(1);
+    expect(read).toHaveBeenCalledWith(undefined, expect.anything());
+  }
+);
+
+test("sets existing value of `@client` field to null and warns when using no-cache with read function", async () => {
+  using _ = spyOnConsole("warn");
+  const query = gql`
+    query GetUser {
+      user {
+        firstName @client
+        lastName
+      }
+    }
+  `;
+
+  const read = jest.fn((value) => value ?? "Fallback");
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        User: {
+          fields: {
+            firstName: {
+              read,
+            },
+          },
+        },
+      },
+    }),
+    link: new ApolloLink(() => {
+      return of({
+        data: { user: { __typename: "User", lastName: "Smith" } },
+      }).pipe(delay(10));
+    }),
+    localState: new LocalState(),
+  });
+
+  await expect(
+    client.query({ query, fetchPolicy: "no-cache" })
+  ).resolves.toStrictEqualTyped({
+    data: {
+      user: { __typename: "User", firstName: null, lastName: "Smith" },
+    },
+  });
+
+  expect(read).not.toHaveBeenCalled();
+  expect(console.warn).toHaveBeenCalledTimes(1);
+  expect(console.warn).toHaveBeenCalledWith(
+    WARNINGS.NO_CACHE,
+    "User.firstName"
+  );
+});
+
+test("sets existing value of `@client` field to null and warns when merge function but not read function is present", async () => {
+  using _ = spyOnConsole("warn");
+  const query = gql`
+    query GetUser {
+      user {
+        firstName @client
+        lastName
+      }
+    }
+  `;
+
+  const merge = jest.fn(() => "Fallback");
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        User: {
+          fields: {
+            firstName: {
+              merge,
+            },
+          },
+        },
+      },
+    }),
+    link: new ApolloLink(() => {
+      return of({
+        data: { user: { __typename: "User", lastName: "Smith" } },
+      }).pipe(delay(10));
+    }),
+    localState: new LocalState(),
+  });
+
+  await expect(client.query({ query })).resolves.toStrictEqualTyped({
+    data: {
+      user: {
+        __typename: "User",
+        firstName: "Fallback",
+        lastName: "Smith",
+      },
+    },
+  });
+
+  expect(merge).toHaveBeenCalledTimes(1);
+  expect(merge).toHaveBeenCalledWith(undefined, null, expect.anything());
+
+  expect(console.warn).toHaveBeenCalledTimes(1);
+  expect(console.warn).toHaveBeenCalledWith(
+    WARNINGS.MISSING_RESOLVER,
+    "User.firstName"
+  );
+});

src/cache/core/cache.ts

@@ -178,6 +178,28 @@ export abstract class ApolloCache {
     return null;
   }
 
+  // Local state API
+
+  /**
+   * Determines whether a `@client` field can be resolved by the cache. Used
+   * when `LocalState` does not have a local resolver that can resolve the
+   * field.
+   *
+   * @remarks Cache implementations should return `true` if a mechanism in the
+   * cache is expected to provide a value for the field. `LocalState` will set
+   * the value of the field to `undefined` in order for the cache to handle it.
+   *
+   * Cache implementations should return `false` to indicate that it cannot
+   * handle resolving the field (either because it doesn't have a mechanism to
+   * do so, or because the user hasn't provided enough information to resolve
+   * the field). Returning `false` will emit a warning and set the value of the
+   * field to `null`.
+   *
+   * A cache that doesn't implement `resolvesClientField` will be treated the
+   * same as returning `false`.
+   */
+  public resolvesClientField?(typename: string, fieldName: string): boolean;
+
   // Transactional API
 
   // The batch method is intended to replace/subsume both performTransaction