diff --git a/.changeset/khaki-months-rhyme.md b/.changeset/khaki-months-rhyme.md new file mode 100644 index 00000000000..acaacebb712 --- /dev/null +++ b/.changeset/khaki-months-rhyme.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +ObservableQuery.getCurrentResult: skip the cache if the running query should not access the cache diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 1e4198a980d..25ba2f6611e 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -1,5 +1,6 @@ +# format "ObservableQuery" test +0a67647b73abd94b706242f32b88d21a1400ad50 # format "ObservableQuery" test (in #10597) 104bf11765b1db50292f9656aa8fe48e2d749a83 - # Format "DisplayClientError.js" (#10909) 0cb68364f2c3828badde8c74de44e9c1864fb6d4 diff --git a/.vscode/settings.json b/.vscode/settings.json index f8ace00ded3..e343acdb09c 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -7,5 +7,6 @@ "typescript.tsdk": "node_modules/typescript/lib", "cSpell.enableFiletypes": [ "mdx" - ] + ], + "jest.jestCommandLine": "node_modules/.bin/jest --config ./config/jest.config.js --ignoreProjects 'ReactDOM 17' --runInBand", } diff --git a/config/bundlesize.ts b/config/bundlesize.ts index 31be6ceb8ed..6f64659851c 100644 --- a/config/bundlesize.ts +++ b/config/bundlesize.ts @@ -3,7 +3,7 @@ import { join } from "path"; import { gzipSync } from "zlib"; import bytes from "bytes"; -const gzipBundleByteLengthLimit = bytes("33.02KB"); +const gzipBundleByteLengthLimit = bytes("33.07KB"); const minFile = join("dist", "apollo-client.min.cjs"); const minPath = join(__dirname, "..", minFile); const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength; diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 22331cac1b6..aa71ff4df67 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -28,6 +28,7 @@ import { FetchMoreQueryOptions, SubscribeToMoreOptions, NextFetchPolicyContext, + WatchQueryFetchPolicy, } from './watchQueryOptions'; import { QueryInfo } from './QueryInfo'; import { MissingFieldError } from '../cache'; @@ -86,6 +87,7 @@ export class ObservableQuery< private observers = new Set>>(); private subscriptions = new Set(); + private waitForOwnResult: boolean; private last?: Last; private queryInfo: QueryInfo; @@ -152,6 +154,7 @@ export class ObservableQuery< this.queryManager = queryManager; // active state + this.waitForOwnResult = skipCacheDataFor(options.fetchPolicy); this.isTornDown = false; const { @@ -240,16 +243,19 @@ export class ObservableQuery< if ( // These fetch policies should never deliver data from the cache, unless // redelivering a previously delivered result. - fetchPolicy === 'network-only' || - fetchPolicy === 'no-cache' || - fetchPolicy === 'standby' || + skipCacheDataFor(fetchPolicy) || // If this.options.query has @client(always: true) fields, we cannot // trust diff.result, since it was read from the cache without running // local resolvers (and it's too late to run resolvers now, since we must // return a result synchronously). this.queryManager.transform(this.options.query).hasForcedResolvers ) { - // Fall through. + // Fall through. + } else if (this.waitForOwnResult) { + // This would usually be a part of `QueryInfo.getDiff()`. + // which we skip in the waitForOwnResult case since we are not + // interested in the diff. + this.queryInfo['updateWatch'](); } else { const diff = this.queryInfo.getDiff(); @@ -833,13 +839,22 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`); } } + this.waitForOwnResult &&= skipCacheDataFor(options.fetchPolicy); + const finishWaitingForOwnResult = () => { + if (this.concast === concast) { + this.waitForOwnResult = false; + } + }; + const variables = options.variables && { ...options.variables }; const { concast, fromLink } = this.fetch(options, newNetworkStatus); const observer: Observer> = { next: result => { + finishWaitingForOwnResult(); this.reportResult(result, variables); }, error: error => { + finishWaitingForOwnResult(); this.reportError(error, variables); }, }; @@ -990,3 +1005,7 @@ export function logMissingFieldErrors( }`, missing); } } + +function skipCacheDataFor(fetchPolicy?: WatchQueryFetchPolicy /* `undefined` would mean `"cache-first"` */) { + return fetchPolicy === "network-only" || fetchPolicy === "no-cache" || fetchPolicy === "standby"; +} diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index 17a3a72b6f1..7f819a0e702 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -2,7 +2,7 @@ import gql from "graphql-tag"; import { GraphQLError } from "graphql"; import { TypedDocumentNode } from "@graphql-typed-document-node/core"; -import { ApolloClient, NetworkStatus, WatchQueryFetchPolicy } from "../../core"; +import { ApolloClient, ApolloQueryResult, NetworkStatus, WatchQueryFetchPolicy } from "../../core"; import { ObservableQuery } from "../ObservableQuery"; import { QueryManager } from "../QueryManager"; @@ -2192,8 +2192,303 @@ describe("ObservableQuery", () => { } }); }); + }); + + { + type Result = Partial>; + + const cacheValues = { + initial: { hello: "world (initial)" }, + link: { hello: "world (from link)" }, + refetch: { hello: "world (from link again)" }, + update1: { hello: "world (from cache again, 1)" }, + update2: { hello: "world (from cache again, 2)" }, + update3: { hello: "world (from cache again, 3)" }, + update4: { hello: "world (from cache again, 4)" }, + } as const; + + const loadingStates = { + loading: { + loading: true, + networkStatus: NetworkStatus.loading, + }, + done: { + loading: false, + networkStatus: NetworkStatus.ready, + }, + refetching: { + loading: true, + networkStatus: NetworkStatus.refetch, + }, + } as const; + + type TestDetails = { + // writeCache: cacheValues.initial + resultBeforeSubscribe: Result, + // observableQuery.subscribe + resultAfterSubscribe: Result, + // writeCache: cacheValues.update1 + resultAfterCacheUpdate1: Result, + // incoming result: cacheValues.link + resultAfterLinkNext: Result, + // writeCache: cacheValues.update2 + resultAfterCacheUpdate2: Result, + // observableQuery.refetch + // writeCache: cacheValues.update3 + resultAfterCacheUpdate3: Result, + // incoming result: cacheValues.refetch + resultAfterRefetchNext: Result, + // writeCache: cacheValues.update4 + resultAfterCacheUpdate4: Result } - ); + + const cacheAndLink: TestDetails = { + resultBeforeSubscribe: { + ...loadingStates.loading, + data: cacheValues.initial, + }, + resultAfterSubscribe: { + ...loadingStates.loading, + data: cacheValues.initial, + }, + resultAfterCacheUpdate1: { + ...loadingStates.loading, + data: cacheValues.update1, + }, + resultAfterLinkNext: { + ...loadingStates.done, + data: cacheValues.link, + }, + resultAfterCacheUpdate2: { + ...loadingStates.done, + data: cacheValues.update2, + }, + resultAfterCacheUpdate3: { + ...loadingStates.refetching, + data: cacheValues.update3, + }, + resultAfterRefetchNext: { + ...loadingStates.done, + data: cacheValues.refetch, + }, + resultAfterCacheUpdate4: { + ...loadingStates.done, + data: cacheValues.update4, + }, + }; + + const linkOnly: TestDetails = { + resultBeforeSubscribe: { + ...loadingStates.loading, + }, + resultAfterSubscribe: { + ...loadingStates.loading, + }, + resultAfterCacheUpdate1: { + ...loadingStates.loading, + }, + resultAfterLinkNext: { + ...loadingStates.done, + data: cacheValues.link, + }, + resultAfterCacheUpdate2: { + ...loadingStates.done, + data: cacheValues.link, + }, + resultAfterCacheUpdate3: { + ...loadingStates.refetching, + data: cacheValues.link, + }, + resultAfterRefetchNext: { + ...loadingStates.done, + data: cacheValues.refetch, + }, + resultAfterCacheUpdate4: { + ...loadingStates.done, + data: cacheValues.refetch, + }, + }; + + const standbyOnly: TestDetails = { + ...linkOnly, + resultBeforeSubscribe: { + ...loadingStates.loading, + }, + resultAfterSubscribe: { + ...loadingStates.loading, + }, + resultAfterCacheUpdate1: { + ...loadingStates.loading, + }, + resultAfterLinkNext: { + ...loadingStates.loading, + }, + resultAfterCacheUpdate2: { + ...loadingStates.loading, + }, + resultAfterCacheUpdate3: { + ...loadingStates.refetching, + }, + // like linkOnly: + // resultAfterRefetchNext + // resultAfterCacheUpdate4 + }; + + const linkOnlyThenCacheAndLink: TestDetails = { + ...cacheAndLink, + resultBeforeSubscribe: { + ...loadingStates.loading, + }, + resultAfterSubscribe: { + ...loadingStates.loading, + }, + resultAfterCacheUpdate1: { + ...loadingStates.loading, + }, + // like cacheAndLink: + // resultAfterLinkNext + // resultAfterCacheUpdate2 + // resultAfterCacheUpdate3 + // resultAfterRefetchNext + // resultAfterCacheUpdate4 + }; + + const cacheOnlyThenCacheAndLink: TestDetails = { + ...cacheAndLink, + resultBeforeSubscribe: { + ...loadingStates.done, + data: cacheValues.initial, + }, + resultAfterSubscribe: { + ...loadingStates.done, + data: cacheValues.initial, + }, + resultAfterCacheUpdate1: { + ...loadingStates.done, + data: cacheValues.update1, + }, + resultAfterLinkNext: { + ...loadingStates.done, + data: cacheValues.update1, + }, + // like cacheAndLink: + // resultAfterCacheUpdate2 + // resultAfterCacheUpdate3 + // resultAfterRefetchNext + // resultAfterCacheUpdate4 + }; + + it.each< + [ + initialFetchPolicy: WatchQueryFetchPolicy, + nextFetchPolicy: WatchQueryFetchPolicy, + testDetails: TestDetails + ] + >([ + ["cache-and-network", "cache-and-network", cacheAndLink], + ["cache-first", "cache-first", cacheOnlyThenCacheAndLink], + ["cache-first", "cache-and-network", cacheOnlyThenCacheAndLink], + ["no-cache", "no-cache", linkOnly], + ["no-cache", "cache-and-network", linkOnlyThenCacheAndLink], + ["standby", "standby", standbyOnly], + ["standby", "cache-and-network", standbyOnly], + ["cache-only", "cache-only", cacheOnlyThenCacheAndLink], + ["cache-only", "cache-and-network", cacheOnlyThenCacheAndLink], + + ])( + "fetchPolicy %s -> %s", + async ( + fetchPolicy, + nextFetchPolicy, + { + resultBeforeSubscribe, + resultAfterSubscribe, + resultAfterCacheUpdate1, + resultAfterLinkNext, + resultAfterCacheUpdate2, + resultAfterCacheUpdate3, + resultAfterRefetchNext, + resultAfterCacheUpdate4, + } + ) => { + const query = gql` + { + hello + } + `; + let observer!: SubscriptionObserver; + const link = new ApolloLink(() => { + return new Observable((o) => { + observer = o; + }); + }); + const cache = new InMemoryCache({}); + cache.writeQuery({ query, data: cacheValues.initial }); + + const queryManager = new QueryManager({ link, cache }); + const observableQuery = queryManager.watchQuery({ + query, + fetchPolicy, + nextFetchPolicy, + }); + + expect(observableQuery.getCurrentResult()).toStrictEqual( + resultBeforeSubscribe + ); + + observableQuery.subscribe({}); + expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterSubscribe + ); + + cache.writeQuery({ query, data: cacheValues.update1 }); + expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterCacheUpdate1 + ); + + if (observer) { + observer.next({ data: cacheValues.link }); + observer.complete(); + } + await waitFor( + () => + void expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterLinkNext + ), + { interval: 1 } + ); + + cache.writeQuery({ query, data: cacheValues.update2 }); + expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterCacheUpdate2 + ); + + observableQuery.refetch(); + + cache.writeQuery({ query, data: cacheValues.update3 }); + expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterCacheUpdate3 + ); + + if (observer) { + observer.next({ data: cacheValues.refetch }); + observer.complete(); + } + await waitFor( + () => + void expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterRefetchNext + ), + { interval: 1 } + ); + + cache.writeQuery({ query, data: cacheValues.update4 }); + expect(observableQuery.getCurrentResult()).toStrictEqual( + resultAfterCacheUpdate4 + ); + } + ); + } describe("mutations", () => { const mutation = gql` diff --git a/src/react/hoc/__tests__/queries/loading.test.tsx b/src/react/hoc/__tests__/queries/loading.test.tsx index e92583441d4..678f5388942 100644 --- a/src/react/hoc/__tests__/queries/loading.test.tsx +++ b/src/react/hoc/__tests__/queries/loading.test.tsx @@ -434,11 +434,7 @@ describe('[queries] loading', () => { ]); }, { interval: 1 }); await waitFor(() => { - if (IS_REACT_18) { expect(count).toBe(5); - } else { - expect(count).toBe(6); - } }, { interval: 1 }); }); diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 880ed1c404e..bae91cdfb95 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -1,4 +1,4 @@ -import React, { Fragment, useEffect, useState } from 'react'; +import React, { Fragment, ReactNode, useEffect, useState } from 'react'; import { DocumentNode, GraphQLError } from 'graphql'; import gql from 'graphql-tag'; import { act } from 'react-dom/test-utils'; @@ -5949,6 +5949,84 @@ describe('useQuery Hook', () => { }); }); + // https://github.com/apollographql/apollo-client/issues/10222 + describe('regression test issue #10222', () => { + it('maintains initial fetch policy when component unmounts and remounts', async () => { + let helloCount = 1; + const query = gql`{ hello }`; + const link = new ApolloLink(() => { + return new Observable(observer => { + const timer = setTimeout(() => { + observer.next({ data: { hello: `hello ${helloCount++}` } }); + observer.complete(); + }, 50); + + return () => { + clearTimeout(timer); + } + }) + }) + + const cache = new InMemoryCache(); + + const client = new ApolloClient({ + link, + cache + }); + + let setShow: Function + const Toggler = ({ children }: { children: ReactNode }) => { + const [show, _setShow] = useState(true); + setShow = _setShow; + + return show ? <>{children} : null; + } + + const { result } = renderHook( + () => useQuery(query, { + fetchPolicy: 'network-only', + nextFetchPolicy: 'cache-first' + }), + { + wrapper: ({ children }) => ( + + {children} + + ), + }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBeUndefined(); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.data).toEqual({ hello: 'hello 1' }); + expect(cache.readQuery({ query })).toEqual({ hello: 'hello 1' }) + + act(() => { + setShow(false); + }); + + act(() => { + setShow(true); + }); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBeUndefined(); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }) + + expect(result.current.data).toEqual({ hello: 'hello 2' }); + expect(cache.readQuery({ query })).toEqual({ hello: 'hello 2' }) + }); + }); + + describe('defer', () => { it('should handle deferred queries', async () => { const query = gql`