diff --git a/CHANGELOG.md b/CHANGELOG.md index 316ba17741f..0f93f9b02a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ - In addition to the `result.data` property, `useQuery` and `useLazyQuery` will now provide a `result.previousData` property, which can be useful when a network request is pending and `result.data` is undefined, since `result.previousData` can be rendered instead of rendering an empty/loading state.
[@hwillson](https://github.com/hwillson) in [#7082](https://github.com/apollographql/apollo-client/pull/7082) +- Ensure `cache.readQuery` and `cache.readFragment` always return `TData | null`, instead of throwing an exception when missing fields are encountered.
+ [@benjamn](https://github.com/benjamn) in [#7098](https://github.com/apollographql/apollo-client/pull/7098) + ## Apollo Client 3.2.1 ## Bug Fixes diff --git a/docs/source/caching/cache-interaction.md b/docs/source/caching/cache-interaction.md index 22b1151282d..46e6b46bd56 100644 --- a/docs/source/caching/cache-interaction.md +++ b/docs/source/caching/cache-interaction.md @@ -19,9 +19,11 @@ All code samples below assume that you have initialized an instance of `ApolloC The `readQuery` method enables you to run a GraphQL query directly on your cache. -* If your cache contains all of the data necessary to fulfill a specified query, `readQuery` returns a data object in the shape of that query, just like a GraphQL server does. +* If your cache contains all of the data necessary to fulfill the specified query, `readQuery` returns a data object in the shape of that query, just like a GraphQL server does. -* If your cache _doesn't_ contain all of the data necessary to fulfill a specified query, `readQuery` throws an error. It _never_ attempts to fetch data from a remote server. +* If your cache does not contain all of the data necessary to fulfill the specified query, `readQuery` returns `null`, without attempting to fetch data from a remote server. + +> Prior to Apollo Client 3.3, `readQuery` would throw `MissingFieldError` exceptions to report missing fields. Beginning with Apollo Client 3.3, `readQuery` always returns `null` to indicate fields were missing. Pass `readQuery` a GraphQL query string like so: @@ -81,12 +83,9 @@ const todo = client.readFragment({ The first argument, `id`, is the value of the unique identifier for the object you want to read from the cache. By default, this is the value of the object's `id` field, but you can [customize this behavior](./cache-configuration/#generating-unique-identifiers). -In the example above: +In the example above, `readFragment` returns `null` if no `Todo` object with an `id` of `5` exists in the cache, or if the object exists but is missing the `text` or `completed` fields. -* If a `Todo` object with an `id` of `5` is _not_ in the cache, -`readFragment` returns `null`. -* If the `Todo` object _is_ in the cache but it's -missing either a `text` or `completed` field, `readFragment` throws an error. +> Prior to Apollo Client 3.3, `readFragment` would throw `MissingFieldError` exceptions to report missing fields, and return `null` only when reading a fragment from a nonexistent ID. Beginning with Apollo Client 3.3, `readFragment` always returns `null` to indicate insufficient data (missing ID or missing fields), instead of throwing a `MissingFieldError`. ## `writeQuery` and `writeFragment` diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 19eb8dd6127..9cc45b7c3e1 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -104,13 +104,14 @@ export abstract class ApolloCache implements DataProxy { * @param optimistic */ public readQuery( - options: DataProxy.Query, - optimistic: boolean = false, + options: Cache.ReadQueryOptions, + optimistic = !!options.optimistic, ): QueryType | null { return this.read({ rootId: options.id || 'ROOT_QUERY', query: options.query, variables: options.variables, + returnPartialData: options.returnPartialData, optimistic, }); } @@ -120,13 +121,14 @@ export abstract class ApolloCache implements DataProxy { private getFragmentDoc = wrap(getFragmentQueryDocument); public readFragment( - options: DataProxy.Fragment, - optimistic: boolean = false, + options: Cache.ReadFragmentOptions, + optimistic = !!options.optimistic, ): FragmentType | null { return this.read({ query: this.getFragmentDoc(options.fragment, options.fragmentName), variables: options.variables, rootId: options.id, + returnPartialData: options.returnPartialData, optimistic, }); } diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index 8a9af7c3ff1..281e0a1599a 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -9,6 +9,7 @@ export namespace Cache { rootId?: string; previousResult?: any; optimistic: boolean; + returnPartialData?: boolean; } export interface WriteOptions @@ -19,7 +20,9 @@ export namespace Cache { } export interface DiffOptions extends ReadOptions { - returnPartialData?: boolean; + // The DiffOptions interface is currently just an alias for + // ReadOptions, though DiffOptions used to be responsible for + // declaring the returnPartialData option. } export interface WatchOptions extends ReadOptions { @@ -42,6 +45,8 @@ export namespace Cache { } export import DiffResult = DataProxy.DiffResult; + export import ReadQueryOptions = DataProxy.ReadQueryOptions; + export import ReadFragmentOptions = DataProxy.ReadFragmentOptions; export import WriteQueryOptions = DataProxy.WriteQueryOptions; export import WriteFragmentOptions = DataProxy.WriteFragmentOptions; export import Fragment = DataProxy.Fragment; diff --git a/src/cache/core/types/DataProxy.ts b/src/cache/core/types/DataProxy.ts index a256829d09f..0d6f0a774b5 100644 --- a/src/cache/core/types/DataProxy.ts +++ b/src/cache/core/types/DataProxy.ts @@ -20,7 +20,7 @@ export namespace DataProxy { /** * The root id to be used. Defaults to "ROOT_QUERY", which is the ID of the * root query object. This property makes writeQuery capable of writing data - * to any object in the cache, which renders writeFragment mostly useless. + * to any object in the cache. */ id?: string; } @@ -54,6 +54,36 @@ export namespace DataProxy { variables?: TVariables; } + export interface ReadQueryOptions + extends Query { + /** + * Whether to return incomplete data rather than null. + * Defaults to false. + */ + returnPartialData?: boolean; + /** + * Whether to read from optimistic or non-optimistic cache data. If + * this named option is provided, the optimistic parameter of the + * readQuery method can be omitted. Defaults to false. + */ + optimistic?: boolean; + } + + export interface ReadFragmentOptions + extends Fragment { + /** + * Whether to return incomplete data rather than null. + * Defaults to false. + */ + returnPartialData?: boolean; + /** + * Whether to read from optimistic or non-optimistic cache data. If + * this named option is provided, the optimistic parameter of the + * readQuery method can be omitted. Defaults to false. + */ + optimistic?: boolean; + } + export interface WriteQueryOptions extends Query { /** @@ -97,7 +127,7 @@ export interface DataProxy { * Reads a GraphQL query from the root query id. */ readQuery( - options: DataProxy.Query, + options: DataProxy.ReadQueryOptions, optimistic?: boolean, ): QueryType | null; @@ -107,7 +137,7 @@ export interface DataProxy { * provided to select the correct fragment. */ readFragment( - options: DataProxy.Fragment, + options: DataProxy.ReadFragmentOptions, optimistic?: boolean, ): FragmentType | null; diff --git a/src/cache/inmemory/__tests__/entityStore.ts b/src/cache/inmemory/__tests__/entityStore.ts index 052c05218ca..d1815c5b08b 100644 --- a/src/cache/inmemory/__tests__/entityStore.ts +++ b/src/cache/inmemory/__tests__/entityStore.ts @@ -1588,13 +1588,29 @@ describe('EntityStore', () => { }, }); - expect(() => cache.readQuery({ + function diff(query: DocumentNode) { + return cache.diff({ + query, + optimistic: true, + returnPartialData: false, + }); + } + + expect(cache.readQuery({ query: queryWithAliases, - })).toThrow(/Dangling reference to missing ABCs:.* object/); + })).toBe(null); - expect(() => cache.readQuery({ + expect(() => diff(queryWithAliases)).toThrow( + /Dangling reference to missing ABCs:.* object/, + ); + + expect(cache.readQuery({ query: queryWithoutAliases, - })).toThrow(/Dangling reference to missing ABCs:.* object/); + })).toBe(null); + + expect(() => diff(queryWithoutAliases)).toThrow( + /Dangling reference to missing ABCs:.* object/, + ); }); it("gracefully handles eviction amid optimistic updates", () => { diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 4d00064d3ec..6db285d11e6 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -2193,17 +2193,27 @@ describe("type policies", function () { expect(secretReadAttempted).toBe(false); - expect(() => { - cache.readQuery({ - query: gql` - query { - me { - secret - } + expect(cache.readQuery({ + query: gql` + query { + me { + secret } - ` - }); - }).toThrowError("Can't find field 'secret' "); + } + `, + })).toBe(null); + + expect(() => cache.diff({ + optimistic: true, + returnPartialData: false, + query: gql` + query { + me { + secret + } + } + `, + })).toThrowError("Can't find field 'secret' "); expect(secretReadAttempted).toBe(true); }); @@ -3517,6 +3527,15 @@ describe("type policies", function () { }); } + function diff(isbn = "156858217X") { + return cache.diff({ + query, + variables: { isbn }, + returnPartialData: false, + optimistic: true, + }); + } + expect(read()).toBe(null); cache.writeQuery({ @@ -3532,8 +3551,10 @@ describe("type policies", function () { }, }); - expect(read).toThrow( - /Dangling reference to missing Book:{"isbn":"156858217X"} object/ + expect(read()).toBe(null); + + expect(diff).toThrow( + /Dangling reference to missing Book:{"isbn":"156858217X"} object/, ); const stealThisData = { @@ -3664,11 +3685,13 @@ describe("type policies", function () { }, }); - expect(() => read("0393354326")).toThrow( + expect(read("0393354326")).toBe(null); + expect(() => diff("0393354326")).toThrow( /Dangling reference to missing Book:{"isbn":"0393354326"} object/ ); - expect(() => read("156858217X")).toThrow( + expect(read("156858217X")).toBe(null); + expect(() => diff("156858217X")).toThrow( /Dangling reference to missing Book:{"isbn":"156858217X"} object/ ); }); diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index fc1f02a9f8b..e44a44ee675 100644 --- a/src/cache/inmemory/__tests__/readFromStore.ts +++ b/src/cache/inmemory/__tests__/readFromStore.ts @@ -557,6 +557,85 @@ describe('reading from the store', () => { }).toThrowError(/Can't find field 'missingField' on ROOT_QUERY object/); }); + it('readQuery supports returnPartialData', () => { + const cache = new InMemoryCache; + const aQuery = gql`query { a }`; + const bQuery = gql`query { b }`; + const abQuery = gql`query { a b }`; + + cache.writeQuery({ + query: aQuery, + data: { a: 123 }, + }); + + expect(cache.readQuery({ query: bQuery })).toBe(null); + expect(cache.readQuery({ query: abQuery })).toBe(null); + + expect(cache.readQuery({ + query: bQuery, + returnPartialData: true, + })).toEqual({}); + + expect(cache.readQuery({ + query: abQuery, + returnPartialData: true, + })).toEqual({ a: 123 }); + }); + + it('readFragment supports returnPartialData', () => { + const cache = new InMemoryCache; + const id = cache.identify({ + __typename: "ABObject", + id: 321, + }); + + const aFragment = gql`fragment AFragment on ABObject { a }`; + const bFragment = gql`fragment BFragment on ABObject { b }`; + const abFragment = gql`fragment ABFragment on ABObject { a b }`; + + expect(cache.readFragment({ id, fragment: aFragment })).toBe(null); + expect(cache.readFragment({ id, fragment: bFragment })).toBe(null); + expect(cache.readFragment({ id, fragment: abFragment })).toBe(null); + + const ref = cache.writeFragment({ + id, + fragment: aFragment, + data: { + __typename: "ABObject", + a: 123, + }, + }); + expect(isReference(ref)).toBe(true); + expect(ref!.__ref).toBe(id); + + expect(cache.readFragment({ + id, + fragment: bFragment, + })).toBe(null); + + expect(cache.readFragment({ + id, + fragment: abFragment, + })).toBe(null); + + expect(cache.readFragment({ + id, + fragment: bFragment, + returnPartialData: true, + })).toEqual({ + __typename: "ABObject", + }); + + expect(cache.readFragment({ + id, + fragment: abFragment, + returnPartialData: true, + })).toEqual({ + __typename: "ABObject", + a: 123, + }); + }); + it('distinguishes between missing @client and non-@client fields', () => { const query = gql` query { diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index aa988941e1d..38c33e28812 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -6,6 +6,7 @@ import { dep, wrap } from 'optimism'; import { ApolloCache } from '../core/cache'; import { Cache } from '../core/types/Cache'; +import { MissingFieldError } from '../core/types/common'; import { addTypenameToDocument, StoreObject, @@ -106,18 +107,36 @@ export class InMemoryCache extends ApolloCache { } public read(options: Cache.ReadOptions): T | null { - const store = options.optimistic ? this.optimisticData : this.data; - if (typeof options.rootId === 'string' && !store.has(options.rootId)) { - return null; + const { + // Since read returns data or null, without any additional metadata + // about whether/where there might have been missing fields, the + // default behavior cannot be returnPartialData = true (like it is + // for the diff method), since defaulting to true would violate the + // integrity of the T in the return type. However, partial data may + // be useful in some cases, so returnPartialData:true may be + // specified explicitly. + returnPartialData = false, + } = options; + try { + return this.storeReader.diffQueryAgainstStore({ + store: options.optimistic ? this.optimisticData : this.data, + query: options.query, + variables: options.variables, + rootId: options.rootId, + config: this.config, + returnPartialData, + }).result || null; + } catch (e) { + if (e instanceof MissingFieldError) { + // Swallow MissingFieldError and return null, so callers do not + // need to worry about catching "normal" exceptions resulting from + // incomplete cache data. Unexpected errors will be re-thrown. If + // you need more information about which fields were missing, use + // cache.diff instead, and examine diffResult.missing. + return null; + } + throw e; } - return this.storeReader.diffQueryAgainstStore({ - store, - query: options.query, - variables: options.variables, - rootId: options.rootId, - config: this.config, - returnPartialData: false, - }).result || null; } public write(options: Cache.WriteOptions): Reference | undefined {