diff --git a/.changeset/neat-toes-leave.md b/.changeset/neat-toes-leave.md new file mode 100644 index 00000000000..5584adab85c --- /dev/null +++ b/.changeset/neat-toes-leave.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix issue where partial data is reported to `useQuery` when using `notifyOnNetworkStatusChange` after it errors while another overlapping query succeeds. diff --git a/.changeset/seven-shoes-invite.md b/.changeset/seven-shoes-invite.md new file mode 100644 index 00000000000..dc3363b894e --- /dev/null +++ b/.changeset/seven-shoes-invite.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix an issue where a partial cache write for an errored query would result in automatically refetching that query. diff --git a/.size-limits.json b/.size-limits.json index 762fec516d0..8d2f9f6c9d7 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 39042, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32550 + "dist/apollo-client.min.cjs": 39075, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32584 } diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index c9c62973d90..8863d7415ee 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -210,7 +210,28 @@ 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. + // + // 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) + ) { + return; + } + this.updateLastDiff(diff); + if (!this.dirty && !equal(oldDiff && oldDiff.result, diff && diff.result)) { this.dirty = true; if (!this.notifyTimeout) { diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index d25765e9c9b..98c1f735026 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -24,6 +24,7 @@ import { itAsync, MockLink, mockSingleLink, + MockSubscriptionLink, subscribeAndCount, wait, } from "../../testing"; @@ -2389,6 +2390,124 @@ describe("ObservableQuery", () => { } ); + it("handles multiple calls to getCurrentResult without losing data", async () => { + const query = gql` + { + greeting { + message + ... on Greeting @defer { + recipient { + name + } + } + } + } + `; + + const link = new MockSubscriptionLink(); + + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const obs = client.watchQuery({ query }); + const stream = new ObservableStream(obs); + + link.simulateResult({ + result: { + data: { + greeting: { + message: "Hello world", + __typename: "Greeting", + }, + }, + hasNext: true, + }, + }); + + { + const result = await stream.takeNext(); + expect(result.data).toEqual({ + greeting: { + message: "Hello world", + __typename: "Greeting", + }, + }); + } + + expect(obs.getCurrentResult().data).toEqual({ + greeting: { + message: "Hello world", + __typename: "Greeting", + }, + }); + + expect(obs.getCurrentResult().data).toEqual({ + greeting: { + message: "Hello world", + __typename: "Greeting", + }, + }); + + link.simulateResult( + { + result: { + incremental: [ + { + data: { + recipient: { + name: "Alice", + __typename: "Person", + }, + __typename: "Greeting", + }, + path: ["greeting"], + }, + ], + hasNext: false, + }, + }, + true + ); + + { + const result = await stream.takeNext(); + expect(result.data).toEqual({ + greeting: { + message: "Hello world", + recipient: { + name: "Alice", + __typename: "Person", + }, + __typename: "Greeting", + }, + }); + } + + expect(obs.getCurrentResult().data).toEqual({ + greeting: { + message: "Hello world", + recipient: { + name: "Alice", + __typename: "Person", + }, + __typename: "Greeting", + }, + }); + + expect(obs.getCurrentResult().data).toEqual({ + greeting: { + message: "Hello world", + recipient: { + name: "Alice", + __typename: "Person", + }, + __typename: "Greeting", + }, + }); + }); + { type Result = Partial>; diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 072ab6d922c..072c3c9e5cb 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -26,8 +26,13 @@ import { import { QueryResult } from "../../types/types"; import { useQuery } from "../useQuery"; import { useMutation } from "../useMutation"; -import { profileHook, spyOnConsole } from "../../../testing/internal"; +import { + createProfiler, + profileHook, + spyOnConsole, +} from "../../../testing/internal"; import { useApolloClient } from "../useApolloClient"; +import { useLazyQuery } from "../useLazyQuery"; describe("useQuery Hook", () => { describe("General use", () => { @@ -4018,6 +4023,264 @@ describe("useQuery Hook", () => { }); }); + // https://github.com/apollographql/apollo-client/issues/11400 + it("does not return partial data unexpectedly when one query errors, then another succeeds with overlapping data", 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")], + }, + maxUsageCount: Number.POSITIVE_INFINITY, + delay: 20, + }, + { + 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 }, + // 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(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + loading: false, + networkStatus: NetworkStatus.error, + }); + + // ensure we aren't setting a value on the observable query that contains + // the partial result + expect( + snapshot.useQueryResult?.observable.getCurrentResult(false) + ).toEqual({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + errors: [new GraphQLError("Intentional error")], + loading: false, + networkStatus: NetworkStatus.error, + partial: true, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: { person: { __typename: "Person", id: 1, lastName: "Doe" } }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await act(() => user.click(screen.getByText("Reload 1st query"))); + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + loading: true, + networkStatus: NetworkStatus.loading, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: { person: { __typename: "Person", id: 1, lastName: "Doe" } }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + { + const { snapshot } = await Profiler.takeRender(); + + expect(snapshot.useQueryResult).toMatchObject({ + data: undefined, + loading: false, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + networkStatus: NetworkStatus.error, + }); + + // ensure we aren't setting a value on the observable query that contains + // the partial result + expect( + snapshot.useQueryResult?.observable.getCurrentResult(false) + ).toEqual({ + data: undefined, + error: new ApolloError({ + graphQLErrors: [new GraphQLError("Intentional error")], + }), + errors: [new GraphQLError("Intentional error")], + loading: false, + networkStatus: NetworkStatus.error, + partial: true, + }); + + expect(snapshot.useLazyQueryResult).toMatchObject({ + called: true, + data: { person: { __typename: "Person", id: 1, lastName: "Doe" } }, + loading: false, + networkStatus: NetworkStatus.ready, + }); + } + + await expect(Profiler).not.toRerender(); + }); + describe("Refetching", () => { it("refetching with different variables", async () => { const query = gql`