From 6481fe1196cedee987781dcb45ebdc0cafb3998c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 14 May 2024 11:13:42 -0600 Subject: [PATCH] Fix merge function that returns incomplete result that did not refetch from network (#11839) --- .changeset/itchy-dodos-approve.md | 5 + .size-limits.json | 4 +- src/core/QueryInfo.ts | 20 +- src/react/hooks/__tests__/useQuery.test.tsx | 727 ++++++++++++++++++++ 4 files changed, 741 insertions(+), 15 deletions(-) create mode 100644 .changeset/itchy-dodos-approve.md diff --git a/.changeset/itchy-dodos-approve.md b/.changeset/itchy-dodos-approve.md new file mode 100644 index 00000000000..947b12b636c --- /dev/null +++ b/.changeset/itchy-dodos-approve.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix a regression in [3.9.5](https://github.com/apollographql/apollo-client/releases/tag/v3.9.5) where a merge function that returned an incomplete result would not allow the client to refetch in order to fulfill the query. diff --git a/.size-limits.json b/.size-limits.json index e90e42c4eea..baaaf675f5a 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 39579, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32825 + "dist/apollo-client.min.cjs": 39573, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32821 } diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index 8863d7415ee..2e8c149eedd 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -211,22 +211,16 @@ export class QueryInfo { setDiff(diff: Cache.DiffResult | null) { const oldDiff = this.lastDiff && this.lastDiff.diff; - // If we do not tolerate partial results, skip this update to prevent it - // from being reported. This prevents a situtuation where a query that - // errors and another succeeds with overlapping data does not report the - // partial data result to the errored query. + // If we are trying to deliver an incomplete cache result, we avoid + // reporting it if the query has errored, otherwise we let the broadcast try + // and repair the partial result by refetching the query. This check avoids + // a situation where a query that errors and another succeeds with + // overlapping data does not report the partial data result to the errored + // query. // // See https://github.com/apollographql/apollo-client/issues/11400 for more // information on this issue. - if ( - diff && - !diff.complete && - !this.observableQuery?.options.returnPartialData && - // In the case of a cache eviction, the diff will become partial so we - // schedule a notification to send a network request (this.oqListener) to - // go and fetch the missing data. - !(oldDiff && oldDiff.complete) - ) { + if (diff && !diff.complete && this.observableQuery?.getLastError()) { return; } diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 268ca9f3134..457a6fb6fed 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -4440,6 +4440,733 @@ describe("useQuery Hook", () => { await expect(Profiler).not.toRerender(); }); + it("rerenders errored query for full cache write", async () => { + interface Query1 { + person: { + __typename: "Person"; + id: number; + firstName: string; + } | null; + } + + interface Query2 { + person: { + __typename: "Person"; + id: number; + firstName: string; + lastName: string; + } | null; + } + + interface Variables { + id: number; + } + + const user = userEvent.setup(); + + const query1: TypedDocumentNode = gql` + query PersonQuery1($id: ID!) { + person(id: $id) { + id + firstName + } + } + `; + + const query2: TypedDocumentNode = gql` + query PersonQuery2($id: ID!) { + person(id: $id) { + id + firstName + lastName + } + } + `; + + const Profiler = createProfiler({ + initialSnapshot: { + useQueryResult: null as QueryResult | null, + useLazyQueryResult: null as QueryResult | null, + }, + }); + + const client = new ApolloClient({ + link: new MockLink([ + { + request: { query: query1, variables: { id: 1 } }, + result: { + data: { person: null }, + errors: [new GraphQLError("Intentional error")], + }, + delay: 20, + }, + { + request: { query: query2, variables: { id: 1 } }, + result: { + data: { + person: { + __typename: "Person", + id: 1, + firstName: "John", + lastName: "Doe", + }, + }, + }, + delay: 20, + }, + ]), + cache: new InMemoryCache(), + }); + + function App() { + const useQueryResult = useQuery(query1, { + variables: { id: 1 }, + // This is necessary to reproduce the behavior + notifyOnNetworkStatusChange: true, + }); + + const [execute, useLazyQueryResult] = useLazyQuery(query2, { + variables: { id: 1 }, + }); + + Profiler.replaceSnapshot({ useQueryResult, useLazyQueryResult }); + + return ; + } + + render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: false, + data: undefined, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: false, + data: undefined, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await act(() => user.click(screen.getByText("Run 2nd query"))); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + // We don't see the update from the cache for one more render cycle, hence + // why this is still showing the error result even though the result from + // the other query has finished and re-rendered. + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: { + person: { + __typename: "Person", + id: 1, + firstName: "John", + lastName: "Doe", + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: { + person: { + __typename: "Person", + id: 1, + firstName: "John", + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: { + person: { + __typename: "Person", + id: 1, + firstName: "John", + lastName: "Doe", + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await expect(Profiler).not.toRerender(); + }); + + it("does not rerender or refetch queries with errors for partial cache writes with returnPartialData: true", async () => { + interface Query1 { + person: { + __typename: "Person"; + id: number; + firstName: string; + alwaysFails: boolean; + } | null; + } + + interface Query2 { + person: { + __typename: "Person"; + id: number; + lastName: string; + } | null; + } + + interface Variables { + id: number; + } + + const user = userEvent.setup(); + + const query1: TypedDocumentNode = gql` + query PersonQuery1($id: ID!) { + person(id: $id) { + id + firstName + alwaysFails + } + } + `; + + const query2: TypedDocumentNode = gql` + query PersonQuery2($id: ID!) { + person(id: $id) { + id + lastName + } + } + `; + + const Profiler = createProfiler({ + initialSnapshot: { + useQueryResult: null as QueryResult | null, + useLazyQueryResult: null as QueryResult | null, + }, + }); + + const client = new ApolloClient({ + link: new MockLink([ + { + request: { query: query1, variables: { id: 1 } }, + result: { + data: { person: null }, + errors: [new GraphQLError("Intentional error")], + }, + delay: 20, + maxUsageCount: Number.POSITIVE_INFINITY, + }, + { + request: { query: query2, variables: { id: 1 } }, + result: { + data: { + person: { + __typename: "Person", + id: 1, + lastName: "Doe", + }, + }, + }, + delay: 20, + }, + ]), + cache: new InMemoryCache(), + }); + + function App() { + const useQueryResult = useQuery(query1, { + variables: { id: 1 }, + notifyOnNetworkStatusChange: true, + returnPartialData: true, + }); + + const [execute, useLazyQueryResult] = useLazyQuery(query2, { + variables: { id: 1 }, + }); + + Profiler.replaceSnapshot({ useQueryResult, useLazyQueryResult }); + + return ; + } + + render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: false, + data: undefined, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: false, + data: undefined, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await act(() => user.click(screen.getByText("Run 2nd query"))); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: { + person: { + __typename: "Person", + id: 1, + lastName: "Doe", + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await expect(Profiler).not.toRerender(); + }); + + it("delivers the full network response when a merge function returns an incomplete result", async () => { + const query = gql` + query { + author { + id + name + post { + id + title + } + } + } + `; + + const Profiler = createProfiler({ + initialSnapshot: { + useQueryResult: null as QueryResult | null, + }, + }); + + const client = new ApolloClient({ + link: new MockLink([ + { + request: { query }, + result: { + data: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee", + post: { + __typename: "Post", + id: 1, + title: "Title", + }, + }, + }, + }, + delay: 20, + }, + ]), + cache: new InMemoryCache({ + typePolicies: { + Author: { + fields: { + post: { + merge: () => { + return {}; + }, + }, + }, + }, + }, + }), + }); + + function App() { + const useQueryResult = useQuery(query); + + Profiler.replaceSnapshot({ useQueryResult }); + + return null; + } + + render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee", + post: { + __typename: "Post", + title: "Title", + }, + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await expect(Profiler).not.toRerender(); + }); + + it("triggers a network request and rerenders with the new result when a mutation causes a partial cache update due to an incomplete merge function result", async () => { + const query = gql` + query { + author { + id + name + post { + id + title + } + } + } + `; + const mutation = gql` + mutation { + updateAuthor { + author { + id + name + post { + id + title + } + } + } + } + `; + + const user = userEvent.setup(); + + const Profiler = createProfiler({ + initialSnapshot: { + useQueryResult: null as QueryResult | null, + }, + }); + + const client = new ApolloClient({ + link: new MockLink([ + { + request: { query }, + result: { + data: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee", + post: { + __typename: "Post", + id: 1, + title: "Title", + }, + }, + }, + }, + delay: 20, + }, + { + request: { query }, + result: { + data: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee (refetch)", + post: { + __typename: "Post", + id: 1, + title: "Title", + }, + }, + }, + }, + delay: 20, + }, + { + request: { query: mutation }, + result: { + data: { + updateAuthor: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee (mutation)", + post: { + __typename: "Post", + id: 1, + title: "Title", + }, + }, + }, + }, + }, + delay: 20, + }, + ]), + cache: new InMemoryCache({ + typePolicies: { + Author: { + fields: { + post: { + // this is necessary to reproduce the issue + merge: () => { + return {}; + }, + }, + }, + }, + }, + }), + }); + + function App() { + const useQueryResult = useQuery(query); + const [mutate] = useMutation(mutation); + + Profiler.replaceSnapshot({ useQueryResult }); + + return ; + } + + render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee", + post: { + __typename: "Post", + title: "Title", + }, + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await act(() => user.click(screen.getByText("Run mutation"))); + await Profiler.takeRender(); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: { + author: { + __typename: "Author", + id: 1, + name: "Author Lee", + post: { + __typename: "Post", + title: "Title", + }, + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: { + author: { + __typename: "Author", + id: 1, + // Because of the merge function returning an incomplete result, we + // don't expect to see the value returned from the mutation. The + // partial result from the mutation causes a network fetch which + // renders the refetched result. + name: "Author Lee (refetch)", + post: { + __typename: "Post", + title: "Title", + }, + }, + }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await expect(Profiler).not.toRerender(); + }); + describe("Refetching", () => { it("refetching with different variables", async () => { const query = gql`