From 1a90c5a16de4c572f160fa3db5f5618b4bd6ce1a Mon Sep 17 00:00:00 2001 From: Ben Best Date: Wed, 8 Jul 2020 18:36:53 -0700 Subject: [PATCH] Fix @client(always: true) bug with useQuery, and add regression test. (#6553) * Add failing useQuery test case useQuery fails to get updated values for forced client-side resolvers watchQuery performs as expected * Trust observableQuery.lastResult over cache when hasForcedResolvers. I'm not totally happy with this solution (see TODO), but it seems like the narrowest way to fix #6552 and #6553, for now. Co-authored-by: Ben Newman --- src/__tests__/local-state/general.ts | 93 ++++++++++++++++ src/core/ObservableQuery.ts | 18 ++- src/react/hooks/__tests__/useQuery.test.tsx | 116 ++++++++++++++++++++ 3 files changed, 225 insertions(+), 2 deletions(-) diff --git a/src/__tests__/local-state/general.ts b/src/__tests__/local-state/general.ts index b0aa7146b9c..a81401d72c7 100644 --- a/src/__tests__/local-state/general.ts +++ b/src/__tests__/local-state/general.ts @@ -439,6 +439,99 @@ describe('Cache manipulation', () => { }, }); }); + + itAsync("should rerun @client(always: true) fields on entity update", (resolve, reject) => { + const query = gql` + query GetClientData($id: ID) { + clientEntity(id: $id) @client(always: true) { + id + title + titleLength @client(always: true) + } + } + `; + + const mutation = gql` + mutation AddOrUpdate { + addOrUpdate(id: $id, title: $title) @client + } + `; + + const fragment = gql` + fragment ClientDataFragment on ClientData { + id + title + } + ` + const client = new ApolloClient({ + cache: new InMemoryCache(), + link: new ApolloLink(() => Observable.of({ data: { } })), + resolvers: { + ClientData: { + titleLength(data) { + return data.title.length + } + }, + Query: { + clientEntity(_root, {id}, {cache}) { + return cache.readFragment({ + id: cache.identify({id, __typename: "ClientData"}), + fragment, + }); + }, + }, + Mutation: { + addOrUpdate(_root, {id, title}, {cache}) { + return cache.writeFragment({ + id: cache.identify({id, __typename: "ClientData"}), + fragment, + data: {id, title, __typename: "ClientData"}, + }); + }, + } + }, + }); + + const entityId = 1; + const shortTitle = "Short"; + const longerTitle = "A little longer"; + client.mutate({ + mutation, + variables: { + id: entityId, + title: shortTitle, + }, + }); + let mutated = false; + client.watchQuery({ query, variables: {id: entityId}}).subscribe({ + next(result) { + if (!mutated) { + expect(result.data.clientEntity).toEqual({ + id: entityId, + title: shortTitle, + titleLength: shortTitle.length, + __typename: "ClientData", + }); + client.mutate({ + mutation, + variables: { + id: entityId, + title: longerTitle, + } + }); + mutated = true; + } else if (mutated) { + expect(result.data.clientEntity).toEqual({ + id: entityId, + title: longerTitle, + titleLength: longerTitle.length, + __typename: "ClientData", + }); + resolve(); + } + }, + }); + }); }); describe('Sample apps', () => { diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 813430dde8b..a767f2875f8 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -497,15 +497,16 @@ once, rather than every time you call fetchMore.`); partial: boolean; } { const { fetchPolicy } = this.options; + const lastData = this.lastResult?.data; if (fetchPolicy === 'no-cache' || fetchPolicy === 'network-only') { return { - data: this.lastResult?.data, + data: lastData, partial: false, }; } - const { result, complete } = this.queryManager.cache.diff({ + let { result, complete } = this.queryManager.cache.diff({ query: this.options.query, variables: this.variables, previousResult: this.lastResult?.data, @@ -513,6 +514,19 @@ once, rather than every time you call fetchMore.`); optimistic, }); + if (lastData && + !this.lastError && + // If this.options.query has @client(always: true) fields, we + // cannot trust 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). 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) { + result = lastData; + } + return { data: (complete || this.options.returnPartialData) ? result : void 0, partial: !complete, diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 1f17dd21dff..a769f1d186e 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -1933,4 +1933,120 @@ describe('useQuery Hook', () => { }).then(resolve, reject); }); }); + + describe('Client Resolvers', () => { + + itAsync("should receive up to date @client(always: true) fields on entity update", (resolve, reject) => { + const query = gql` + query GetClientData($id: ID) { + clientEntity(id: $id) @client(always: true) { + id + title + titleLength @client(always: true) + } + } + `; + + const mutation = gql` + mutation AddOrUpdate { + addOrUpdate(id: $id, title: $title) @client + } + `; + + const fragment = gql` + fragment ClientDataFragment on ClientData { + id + title + } + ` + const client = new ApolloClient({ + cache: new InMemoryCache(), + link: new ApolloLink(() => Observable.of({ data: { } })), + resolvers: { + ClientData: { + titleLength(data) { + return data.title.length + } + }, + Query: { + clientEntity(_root, {id}, {cache}) { + return cache.readFragment({ + id: cache.identify({id, __typename: "ClientData"}), + fragment, + }); + }, + }, + Mutation: { + addOrUpdate(_root, {id, title}, {cache}) { + return cache.writeFragment({ + id: cache.identify({id, __typename: "ClientData"}), + fragment, + data: {id, title, __typename: "ClientData"}, + }); + }, + } + }, + }); + + const entityId = 1; + const shortTitle = "Short"; + const longerTitle = "A little longer"; + client.mutate({ + mutation, + variables: { + id: entityId, + title: shortTitle, + }, + }); + let renderCount = 0; + function App() { + const { data } = useQuery(query, { + variables: { + id: entityId, + } + }); + + switch (++renderCount) { + case 2: + expect(data.clientEntity).toEqual({ + id: entityId, + title: shortTitle, + titleLength: shortTitle.length, + __typename: "ClientData", + }); + setTimeout(() => { + client.mutate({ + mutation, + variables: { + id: entityId, + title: longerTitle, + } + }); + }); + break; + case 3: + expect(data.clientEntity).toEqual({ + id: entityId, + title: longerTitle, + titleLength: longerTitle.length, + __typename: "ClientData", + }); + break; + default: // Do nothing + } + + return null; + } + + render( + + + + ); + + return wait(() => { + expect(renderCount).toBe(3); + }).then(resolve, reject); + }); + }); });