diff --git a/CHANGELOG.md b/CHANGELOG.md index dd2c67a52fd..8774adfe60f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,10 +27,14 @@ - `InMemoryCache` now coalesces `EntityStore` updates to guarantee only one `store.merge(id, fields)` call per `id` per cache write.
[@benjamn](https://github.com/benjamn) in [#8372](https://github.com/apollographql/apollo-client/pull/8372) -- Fix polling when used with `React.StrictMode`,
+- Fix polling when used with ``.
[@brainkim](https://github.com/brainkim) in [#8414](https://github.com/apollographql/apollo-client/pull/8414) -- Fix the React integration logging `Warning: Can't perform a React state update on an unmounted component`.
[@wuarmin](https://github.com/wuarmin) in [#7745](https://github.com/apollographql/apollo-client/pull/7745) +- Fix the React integration logging `Warning: Can't perform a React state update on an unmounted component`.
+ [@wuarmin](https://github.com/wuarmin) in [#7745](https://github.com/apollographql/apollo-client/pull/7745) + +- Make `ObservableQuery#getCurrentResult` always call `queryInfo.getDiff()`.
+ [@benjamn](https://github.com/benjamn) in [#8422](https://github.com/apollographql/apollo-client/pull/8422) ### Potentially disruptive changes diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 2cc2ae523f5..a0825f65888 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -169,7 +169,12 @@ export class ObservableQuery< } public getCurrentResult(saveAsLastResult = true): ApolloQueryResult { - const { lastResult } = this; + const { + lastResult, + options: { + fetchPolicy = "cache-first", + }, + } = this; const networkStatus = this.queryInfo.networkStatus || @@ -182,33 +187,18 @@ export class ObservableQuery< networkStatus, } as ApolloQueryResult; - if (this.isTornDown) { - return result; - } - - const { fetchPolicy = 'cache-first' } = this.options; - if (fetchPolicy === 'no-cache' || - fetchPolicy === 'network-only') { - // Similar to setting result.partial to false, but taking advantage - // of the falsiness of missing fields. - delete result.partial; - } else if ( - !result.data || - // If this.options.query has @client(always: true) fields, we cannot - // trust result.data, 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). TODO In the future - // (after Apollo Client 3.0), we should find a way to trust - // this.lastResult in more cases, and read from the cache only in - // cases when no result has been received yet. - !this.queryManager.transform(this.options.query).hasForcedResolvers - ) { + // 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). + if (!this.queryManager.transform(this.options.query).hasForcedResolvers) { const diff = this.queryInfo.getDiff(); - // XXX the only reason this typechecks is that diff.result is inferred as any + result.data = ( diff.complete || this.options.returnPartialData ) ? diff.result : void 0; + if (diff.complete) { // If the diff is complete, and we're using a FetchPolicy that // terminates after a complete cache read, we can assume the next @@ -220,7 +210,10 @@ export class ObservableQuery< result.loading = false; } delete result.partial; - } else { + } else if (fetchPolicy !== "no-cache") { + // Since result.partial comes from diff.complete, and we shouldn't be + // using cache data to provide a DiffResult when the fetchPolicy is + // "no-cache", avoid annotating result.partial for "no-cache" results. result.partial = true; } diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index cc8ec7d8de8..d0a2b27175c 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -1648,15 +1648,16 @@ describe('ObservableQuery', () => { }); expect(observable.getCurrentResult()).toEqual({ - data: void 0, + data: dataOne, loading: true, - networkStatus: 1, + networkStatus: NetworkStatus.loading, }); subscribeAndCount(reject, observable, (handleCount, subResult) => { if (handleCount === 1) { expect(subResult).toEqual({ loading: true, + data: dataOne, networkStatus: NetworkStatus.loading, }); } else if (handleCount === 2) { diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 76feb391398..41e7f61a328 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -3107,6 +3107,232 @@ describe('useQuery Hook', () => { expect(renderCount).toBe(5); }).then(resolve, reject); }); + + itAsync("should be cleared when variables change causes cache miss", (resolve, reject) => { + const peopleData = [ + { id: 1, name: 'John Smith', gender: 'male' }, + { id: 2, name: 'Sara Smith', gender: 'female' }, + { id: 3, name: 'Budd Deey', gender: 'nonbinary' }, + { id: 4, name: 'Johnny Appleseed', gender: 'male' }, + { id: 5, name: 'Ada Lovelace', gender: 'female' }, + ]; + + const link = new ApolloLink(operation => { + return new Observable(observer => { + const { gender } = operation.variables; + new Promise(resolve => setTimeout(resolve, 300)).then(() => { + observer.next({ + data: { + people: gender === "all" ? peopleData : + gender ? peopleData.filter( + person => person.gender === gender + ) : peopleData, + } + }); + observer.complete(); + }); + }); + }); + + type Person = { + __typename: string; + id: string; + name: string; + }; + + const ALL_PEOPLE: TypedDocumentNode<{ + people: Person[]; + }> = gql` + query AllPeople($gender: String!) { + people(gender: $gender) { + id + name + } + } + `; + + let renderCount = 0; + function App() { + const [gender, setGender] = useState("all"); + const { + loading, + networkStatus, + data, + } = useQuery(ALL_PEOPLE, { + variables: { gender }, + fetchPolicy: "network-only", + }); + + const currentPeopleNames = data?.people?.map(person => person.name); + + switch (++renderCount) { + case 1: + expect(gender).toBe("all"); + expect(loading).toBe(true); + expect(networkStatus).toBe(NetworkStatus.loading); + expect(data).toBeUndefined(); + expect(currentPeopleNames).toBeUndefined(); + break; + + case 2: + expect(gender).toBe("all"); + expect(loading).toBe(false); + expect(networkStatus).toBe(NetworkStatus.ready); + expect(data).toEqual({ + people: peopleData.map(({ gender, ...person }) => person), + }); + expect(currentPeopleNames).toEqual([ + "John Smith", + "Sara Smith", + "Budd Deey", + "Johnny Appleseed", + "Ada Lovelace", + ]); + act(() => { + setGender("female"); + }); + break; + + case 3: + expect(gender).toBe("female"); + expect(loading).toBe(true); + expect(networkStatus).toBe(NetworkStatus.setVariables); + expect(data).toBeUndefined(); + expect(currentPeopleNames).toBeUndefined(); + break; + + case 4: + expect(gender).toBe("female"); + expect(loading).toBe(false); + expect(networkStatus).toBe(NetworkStatus.ready); + expect(data!.people.length).toBe(2); + expect(currentPeopleNames).toEqual([ + "Sara Smith", + "Ada Lovelace", + ]); + act(() => { + setGender("nonbinary"); + }); + break; + + case 5: + expect(gender).toBe("nonbinary"); + expect(loading).toBe(true); + expect(networkStatus).toBe(NetworkStatus.setVariables); + expect(data).toBeUndefined(); + expect(currentPeopleNames).toBeUndefined(); + break; + + case 6: + expect(gender).toBe("nonbinary"); + expect(loading).toBe(false); + expect(networkStatus).toBe(NetworkStatus.ready); + expect(data!.people.length).toBe(1); + expect(currentPeopleNames).toEqual([ + "Budd Deey", + ]); + act(() => { + setGender("male"); + }); + break; + + case 7: + expect(gender).toBe("male"); + expect(loading).toBe(true); + expect(networkStatus).toBe(NetworkStatus.setVariables); + expect(data).toBeUndefined(); + expect(currentPeopleNames).toBeUndefined(); + break; + + case 8: + expect(gender).toBe("male"); + expect(loading).toBe(false); + expect(networkStatus).toBe(NetworkStatus.ready); + expect(data!.people.length).toBe(2); + expect(currentPeopleNames).toEqual([ + "John Smith", + "Johnny Appleseed", + ]); + act(() => { + setGender("female"); + }); + break; + + case 9: + expect(gender).toBe("female"); + expect(loading).toBe(true); + expect(networkStatus).toBe(NetworkStatus.setVariables); + expect(data!.people.length).toBe(2); + expect(currentPeopleNames).toEqual([ + "Sara Smith", + "Ada Lovelace", + ]); + break; + + case 10: + expect(gender).toBe("female"); + expect(loading).toBe(false); + expect(networkStatus).toBe(NetworkStatus.ready); + expect(data!.people.length).toBe(2); + expect(currentPeopleNames).toEqual([ + "Sara Smith", + "Ada Lovelace", + ]); + act(() => { + setGender("all"); + }); + break; + + case 11: + expect(gender).toBe("all"); + expect(loading).toBe(true); + expect(networkStatus).toBe(NetworkStatus.setVariables); + expect(data!.people.length).toBe(5); + expect(currentPeopleNames).toEqual([ + "John Smith", + "Sara Smith", + "Budd Deey", + "Johnny Appleseed", + "Ada Lovelace", + ]); + break; + + case 12: + expect(gender).toBe("all"); + expect(loading).toBe(false); + expect(networkStatus).toBe(NetworkStatus.ready); + expect(data!.people.length).toBe(5); + expect(currentPeopleNames).toEqual([ + "John Smith", + "Sara Smith", + "Budd Deey", + "Johnny Appleseed", + "Ada Lovelace", + ]); + break; + + default: + reject(`too many (${renderCount}) renders`); + } + + return null; + } + + const client = new ApolloClient({ + cache: new InMemoryCache(), + link + }); + + render( + + + , + ); + + return wait(() => { + expect(renderCount).toBe(12); + }).then(resolve, reject); + }); }); describe("canonical cache results", () => {