From 94256e6b715bc4cdaa3b3200e142caec4667075c Mon Sep 17 00:00:00 2001 From: alessia Date: Wed, 21 Sep 2022 15:06:11 -0400 Subject: [PATCH 1/4] fix: call iterateObserversSafely if variables change --- src/__tests__/local-state/general.ts | 109 ++++++++++++++++++++++++++- src/core/ObservableQuery.ts | 13 +++- src/core/__tests__/fetchPolicies.ts | 10 +++ 3 files changed, 128 insertions(+), 4 deletions(-) diff --git a/src/__tests__/local-state/general.ts b/src/__tests__/local-state/general.ts index 47f595440ec..0ac52d61aca 100644 --- a/src/__tests__/local-state/general.ts +++ b/src/__tests__/local-state/general.ts @@ -1,5 +1,16 @@ import gql from 'graphql-tag'; -import { DocumentNode, GraphQLError, getIntrospectionQuery } from 'graphql'; +import { + graphql, + GraphQLInt, + print, + DocumentNode, + GraphQLError, + getIntrospectionQuery, + GraphQLSchema, + GraphQLObjectType, + GraphQLID, + GraphQLString +} from 'graphql'; import { Observable } from '../../utilities'; import { ApolloLink } from '../../link/core'; @@ -819,6 +830,102 @@ describe('Combining client and server state/operations', () => { }, 10); }); + itAsync('query resolves with loading: false if subsequent responses contain the same data', (resolve, reject) => { + const request = { + query: gql` + query people($id: Int) { + people(id: $id) { + id + name + } + } + `, + variables: { + id: 1, + }, + notifyOnNetworkStatusChange: true + }; + + const PersonType = new GraphQLObjectType({ + name: "Person", + fields: { + id: { type: GraphQLID }, + name: { type: GraphQLString } + } + }); + + const peopleData = [ + { id: 1, name: "John Smith" }, + { id: 2, name: "Sara Smith" }, + { id: 3, name: "Budd Deey" } + ]; + + const QueryType = new GraphQLObjectType({ + name: "Query", + fields: { + people: { + type: PersonType, + args: { + id: { + type: GraphQLInt + } + }, + resolve: (_, { id }) => { + return peopleData; + } + } + } + }); + + const schema = new GraphQLSchema({ query: QueryType }); + + const link = new ApolloLink(operation => { + // @ts-ignore + return new Observable(async observer => { + const { query, operationName, variables } = operation; + try { + const result = await graphql({ + schema, + source: print(query), + variableValues: variables, + operationName, + }); + observer.next(result); + observer.complete(); + } catch (err) { + observer.error(err); + } + }); + }); + + const client = new ApolloClient({ + cache: new InMemoryCache(), + link, + }); + + const observer = client.watchQuery(request); + + let count = 0; + observer.subscribe({ + next: ({ loading, data }) => { + if (count === 0) expect(loading).toBe(false); + if (count === 1) expect(loading).toBe(true); + if (count === 2) { + expect(loading).toBe(false) + resolve(); + }; + count++; + }, + error: reject, + }); + + setTimeout(() => { + observer.refetch({ + id: 2 + }); + }, 1); + }); + itAsync('should correctly propagate an error from a client resolver', async (resolve, reject) => { const data = { list: { diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index c1b1e255409..d42ffcea270 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -303,8 +303,15 @@ export class ObservableQuery< // Compares newResult to the snapshot we took of this.lastResult when it was // first received. - public isDifferentFromLastResult(newResult: ApolloQueryResult) { - return !this.last || !equal(this.last.result, newResult); + public isDifferentFromLastResult( + newResult: ApolloQueryResult, + variables?: TVariables + ) { + return ( + !this.last || + !equal(this.last.result, newResult) || + !equal(this.last.variables, variables) + ); } private getLast>( @@ -872,7 +879,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`); variables: TVariables | undefined, ) { const lastError = this.getLastError(); - if (lastError || this.isDifferentFromLastResult(result)) { + if (lastError || this.isDifferentFromLastResult(result, variables)) { if (lastError || !result.partial || this.options.returnPartialData) { this.updateLastResult(result, variables); } diff --git a/src/core/__tests__/fetchPolicies.ts b/src/core/__tests__/fetchPolicies.ts index 76ae26d41fe..2982f0ca048 100644 --- a/src/core/__tests__/fetchPolicies.ts +++ b/src/core/__tests__/fetchPolicies.ts @@ -1163,7 +1163,17 @@ describe("nextFetchPolicy", () => { // resets the fetchPolicy to context.initialPolicy), so cache-first is // still what we see here. expect(observable.options.fetchPolicy).toBe("cache-first"); + } else if (count === 3) { + expect(result.loading).toBe(false); + expect(result.data).toEqual({ + linkCounter: 2, + opName: "EchoQuery", + opVars: { + refetching: true, + }, + }); + expect(observable.options.fetchPolicy).toBe("cache-first"); setTimeout(resolve, 20); } else { reject(`Too many results (${count})`); From 21a618fbb884426880e2abf718026bfcdc706c88 Mon Sep 17 00:00:00 2001 From: alessia Date: Wed, 12 Oct 2022 16:00:42 -0400 Subject: [PATCH 2/4] fix: check variables truthiness before equality check --- src/core/ObservableQuery.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index d42ffcea270..0f601b73a6f 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -307,10 +307,11 @@ export class ObservableQuery< newResult: ApolloQueryResult, variables?: TVariables ) { + console.log({variables}) return ( !this.last || !equal(this.last.result, newResult) || - !equal(this.last.variables, variables) + (variables && !equal(this.last.variables, variables)) ); } From d5ece94c977e5f21bb5f43dca8ece690f8e87dbd Mon Sep 17 00:00:00 2001 From: alessia Date: Wed, 12 Oct 2022 16:02:17 -0400 Subject: [PATCH 3/4] fix: remove console.log --- src/core/ObservableQuery.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 0f601b73a6f..1bc872da4c6 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -307,7 +307,6 @@ export class ObservableQuery< newResult: ApolloQueryResult, variables?: TVariables ) { - console.log({variables}) return ( !this.last || !equal(this.last.result, newResult) || From 648473c0b6e3a50e049b71ea6da5e7bacd5fd69f Mon Sep 17 00:00:00 2001 From: alessia Date: Mon, 17 Oct 2022 11:04:19 -0400 Subject: [PATCH 4/4] chore: update CHANGELOG --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad6da8cae36..0e7bcb78074 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## Apollo Client 3.7.1 + +### Bug fixes + +- Fix issue where `loading` remains `true` after `observer.refetch` is called repeatedly with different variables when the same data are returned.
+ [@alessbell](https://github.com/alessbell) in [#10143](https://github.com/apollographql/apollo-client/pull/10143) + ## Apollo Client 3.7.0 (2022-09-30) ### New Features