From 5e8b5e09e504e2780e02526acba792b340a571a4 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 24 Feb 2023 12:27:37 -0500 Subject: [PATCH 1/8] Avoid destructively modifying result.data in QueryInfo#markResult. Should help with #9293. --- src/core/QueryInfo.ts | 31 ++++++++++++++++++++----------- src/core/QueryManager.ts | 4 ++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index 4ddb60fce3..44540c281b 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -361,7 +361,7 @@ export class QueryInfo { "variables" | "fetchPolicy" | "errorPolicy" >, cacheWriteBehavior: CacheWriteBehavior - ) { + ): typeof result { const merger = new DeepMerger(); const graphQLErrors = isNonEmptyArray(result.errors) ? result.errors.slice(0) @@ -408,7 +408,10 @@ export class QueryInfo { }); this.lastWrite = { - result, + // Make a shallow defensive copy of the result object, in case we + // later later modify result.data in place, since we don't want + // that mutation affecting the saved lastWrite.result.data. + result: { ...result }, variables: options.variables, dmCount: destructiveMethodCounts.get(this.cache), }; @@ -448,7 +451,10 @@ export class QueryInfo { if (this.lastDiff && this.lastDiff.diff.complete) { // Reuse data from the last good (complete) diff that we // received, when possible. - result.data = this.lastDiff.diff.result; + result = { + ...result, + data: this.lastDiff.diff.result, + }; return; } // If the previous this.diff was incomplete, fall through to @@ -470,20 +476,23 @@ export class QueryInfo { this.updateWatch(options.variables); } - // If we're allowed to write to the cache, and we can read a - // complete result from the cache, update result.data to be the - // result from the cache, rather than the raw network result. - // Set without setDiff to avoid triggering a notify call, since - // we have other ways of notifying for this result. + // If we're allowed to write to the cache, update result.data to be + // the result as re-read from the cache, rather than the raw network + // result. Set without setDiff to avoid triggering a notify call, + // since we have other ways of notifying for this result. this.updateLastDiff(diff, diffOptions); - if (diff.complete) { - result.data = diff.result; - } + result = { + ...result, + // TODO Improve types so we don't need this cast. + data: diff.result as any, + }; }); } else { this.lastWrite = void 0; } } + + return result; } public markReady() { diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index efdd3c05a9..799fcba922 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1176,11 +1176,11 @@ export class QueryManager { // Use linkDocument rather than queryInfo.document so the // operation/fragments used to write the result are the same as the // ones used to obtain it from the link. - queryInfo.markResult( + result = queryInfo.markResult( result, linkDocument, options, - cacheWriteBehavior + cacheWriteBehavior, ); queryInfo.markReady(); } From 7f9c5ac620112d2f7cab5d419bf8b414d74ca844 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 29 Aug 2023 15:19:27 -0400 Subject: [PATCH 2/8] Test adjustments after fixing QueryInfo#markResult. Most of these test tweaks are reasonable improvements necessitated by fixing a bug that allowed queries to receive raw network results with extraneous fields when the results were incomplete. Now, the extraneous fields are no longer delivered, since they were not requested. The test I removed completely does not make sense, and was only passing previously because of the mock link running out of results. --- src/__tests__/client.ts | 7 +- src/core/__tests__/QueryManager/index.ts | 113 +------------------- src/react/hooks/__tests__/useQuery.test.tsx | 7 +- 3 files changed, 13 insertions(+), 114 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index c89098362d..4a3d7426f0 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -2921,7 +2921,12 @@ describe("client", () => { return client .query({ query }) .then(({ data }) => { - expect(data).toEqual(result.data); + const { price, ...todoWithoutPrice } = data.todos[0]; + expect(data).toEqual({ + todos: [ + todoWithoutPrice, + ], + }); }) .then(resolve, reject); } diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 77fd0fe6db..e1eeb02893 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -66,14 +66,6 @@ export function resetStore(qm: QueryManager) { } describe("QueryManager", () => { - // Standard "get id from object" method. - const dataIdFromObject = (object: any) => { - if (object.__typename && object.id) { - return object.__typename + "__" + object.id; - } - return undefined; - }; - // Helper method that serves as the constructor method for // QueryManager but has defaults that make sense for these // tests. @@ -2224,107 +2216,6 @@ describe("QueryManager", () => { } ); - itAsync( - "should not return stale data when we orphan a real-id node in the store with a real-id node", - (resolve, reject) => { - const query1 = gql` - query { - author { - name { - firstName - lastName - } - age - id - __typename - } - } - `; - const query2 = gql` - query { - author { - name { - firstName - } - id - __typename - } - } - `; - const data1 = { - author: { - name: { - firstName: "John", - lastName: "Smith", - }, - age: 18, - id: "187", - __typename: "Author", - }, - }; - const data2 = { - author: { - name: { - firstName: "John", - }, - id: "197", - __typename: "Author", - }, - }; - const reducerConfig = { dataIdFromObject }; - const queryManager = createQueryManager({ - link: mockSingleLink( - { - request: { query: query1 }, - result: { data: data1 }, - }, - { - request: { query: query2 }, - result: { data: data2 }, - }, - { - request: { query: query1 }, - result: { data: data1 }, - } - ).setOnError(reject), - config: reducerConfig, - }); - - const observable1 = queryManager.watchQuery({ query: query1 }); - const observable2 = queryManager.watchQuery({ query: query2 }); - - // I'm not sure the waiting 60 here really is required, but the test used to do it - return Promise.all([ - observableToPromise( - { - observable: observable1, - wait: 60, - }, - (result) => { - expect(result).toEqual({ - data: data1, - loading: false, - networkStatus: NetworkStatus.ready, - }); - } - ), - observableToPromise( - { - observable: observable2, - wait: 60, - }, - (result) => { - expect(result).toEqual({ - data: data2, - loading: false, - networkStatus: NetworkStatus.ready, - }); - } - ), - ]).then(resolve, reject); - } - ); - itAsync( "should return partial data when configured when we orphan a real-id node in the store with a real-id node", (resolve, reject) => { @@ -2519,9 +2410,7 @@ describe("QueryManager", () => { loading: false, networkStatus: NetworkStatus.ready, data: { - info: { - a: "ay", - }, + info: {}, }, }); setTimeout(resolve, 100); diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 96810b6414..6846523e69 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -5706,7 +5706,12 @@ describe("useQuery Hook", () => { }, { interval: 1 } ); - expect(result.current.data).toEqual(carData); + const { vine, ...carDataWithoutVine } = carData.cars[0]; + expect(result.current.data).toEqual({ + cars: [ + carDataWithoutVine, + ], + }); expect(result.current.error).toBeUndefined(); expect(errorSpy).toHaveBeenCalled(); From a2011110a7f0c90286de15ce788f5fb4226c0c18 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 29 Aug 2023 17:09:11 -0400 Subject: [PATCH 3/8] New test of re-reading incomplete network results from cache. --- src/cache/inmemory/__tests__/client.ts | 156 +++++++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 src/cache/inmemory/__tests__/client.ts diff --git a/src/cache/inmemory/__tests__/client.ts b/src/cache/inmemory/__tests__/client.ts new file mode 100644 index 0000000000..dd85d7c331 --- /dev/null +++ b/src/cache/inmemory/__tests__/client.ts @@ -0,0 +1,156 @@ +// This file contains InMemoryCache-specific tests that exercise the +// ApolloClient class. Other test modules in this directory only test +// InMemoryCache and related utilities, without involving ApolloClient. + +import { ApolloClient, WatchQueryFetchPolicy, gql } from "../../../core"; +import { ApolloLink } from "../../../link/core"; +import { Observable } from "../../../utilities"; +import { InMemoryCache } from "../.."; +import { subscribeAndCount } from "../../../testing"; + +describe("InMemoryCache tests exercising ApolloClient", () => { + it.each([ + "cache-first", + "network-only", + "cache-and-network", + "cache-only", + "no-cache", + ])("results should be read from cache even when incomplete (fetchPolicy %s)", fetchPolicy => { + const dateString = new Date().toISOString(); + + const cache = new InMemoryCache({ + typePolicies: { + Query: { + fields: { + date: { + read(existing) { + return new Date(existing || dateString); + }, + }, + }, + }, + }, + }); + + const client = new ApolloClient({ + link: new ApolloLink(operation => new Observable(observer => { + observer.next({ + data: { + // This raw string should be converted to a Date by the Query.date + // read function passed to InMemoryCache below. + date: dateString, + // Make sure we don't accidentally return fields not mentioned in + // the query just because the result is incomplete. + ignored: "irrelevant to the subscribed query", + // Note the Query.missing field is, well, missing. + }, + }); + setTimeout(() => { + observer.complete(); + }, 10); + })), + cache, + }); + + const query = gql` + query { + date + missing + } + `; + + const observable = client.watchQuery({ + query, + fetchPolicy, // Varies with each test iteration + returnPartialData: true, + }); + + return new Promise((resolve, reject) => { + subscribeAndCount(reject, observable, (handleCount, result) => { + let adjustedCount = handleCount; + if ( + fetchPolicy === "network-only" || + fetchPolicy === "no-cache" || + fetchPolicy === "cache-only" + ) { + // The network-only, no-cache, and cache-only fetch policies do not + // deliver a loading:true result initially, so we adjust the + // handleCount to skip that case. + ++adjustedCount; + } + + // The only fetch policy that does not re-read results from the cache is + // the "no-cache" policy. In this test, that means the Query.date field + // will remain as a raw string rather than being converted to a Date by + // the read function. + const expectedDate = + fetchPolicy === "no-cache" ? dateString : new Date(dateString); + + if (adjustedCount === 1) { + expect(result.loading).toBe(true); + expect(result.data).toEqual({ + date: expectedDate, + }); + + } else if (adjustedCount === 2) { + expect(result.loading).toBe(false); + expect(result.data).toEqual({ + date: expectedDate, + // The no-cache fetch policy does return extraneous fields from the + // raw network result that were not requested in the query, since + // the cache is not consulted. + ...(fetchPolicy === "no-cache" ? { + ignored: "irrelevant to the subscribed query" + } : null), + }); + + if (fetchPolicy === "no-cache") { + // The "no-cache" fetch policy does not receive updates from the + // cache, so we finish the test early (passing). + setTimeout(() => resolve(), 20); + + } else { + cache.writeQuery({ + query: gql`query { missing }`, + data: { + missing: "not missing anymore", + }, + }); + } + + } else if (adjustedCount === 3) { + expect(result.loading).toBe(false); + expect(result.data).toEqual({ + date: expectedDate, + missing: "not missing anymore", + }); + + expect(cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + // The cache-only fetch policy does not receive updates from the + // network, so it never ends up writing the date field into the + // cache explicitly, though Query.date can still be synthesized by + // the read function. + ...(fetchPolicy === "cache-only" ? null : { + // Make sure this field is stored internally as a raw string. + date: dateString, + }), + // Written explicitly with cache.writeQuery above. + missing: "not missing anymore", + // The ignored field is never written to the cache, because it is + // not included in the query. + }, + }); + + // Wait 20ms to give the test a chance to fail if there are unexpected + // additional results. + setTimeout(() => resolve(), 20); + + } else { + reject(new Error(`Unexpected count ${adjustedCount}`)); + } + }); + }); + }); +}); From 7c2bc08b2ab46b9aa181d187a27aec2ad7129599 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 8 Sep 2023 16:53:57 -0400 Subject: [PATCH 4/8] Run 'npx changeset' for PR #11202. --- .changeset/shaggy-ears-scream.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shaggy-ears-scream.md diff --git a/.changeset/shaggy-ears-scream.md b/.changeset/shaggy-ears-scream.md new file mode 100644 index 0000000000..3ec33bfab5 --- /dev/null +++ b/.changeset/shaggy-ears-scream.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": minor +--- + +Prevent `QueryInfo#markResult` mutation of `result.data` and return cache data consistently whether complete or incomplete. From a031860ae85b85d5aafb945c4cf1ae61844262b1 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 13 Sep 2023 16:48:47 -0400 Subject: [PATCH 5/8] Run 'npm run format' for code modified in PR #11202. --- src/__tests__/client.ts | 4 +- src/cache/inmemory/__tests__/client.ts | 252 ++++++++++---------- src/core/QueryManager.ts | 2 +- src/react/hooks/__tests__/useQuery.test.tsx | 4 +- 4 files changed, 134 insertions(+), 128 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 4a3d7426f0..9df645767c 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -2923,9 +2923,7 @@ describe("client", () => { .then(({ data }) => { const { price, ...todoWithoutPrice } = data.todos[0]; expect(data).toEqual({ - todos: [ - todoWithoutPrice, - ], + todos: [todoWithoutPrice], }); }) .then(resolve, reject); diff --git a/src/cache/inmemory/__tests__/client.ts b/src/cache/inmemory/__tests__/client.ts index dd85d7c331..86c4e8ee07 100644 --- a/src/cache/inmemory/__tests__/client.ts +++ b/src/cache/inmemory/__tests__/client.ts @@ -15,142 +15,152 @@ describe("InMemoryCache tests exercising ApolloClient", () => { "cache-and-network", "cache-only", "no-cache", - ])("results should be read from cache even when incomplete (fetchPolicy %s)", fetchPolicy => { - const dateString = new Date().toISOString(); - - const cache = new InMemoryCache({ - typePolicies: { - Query: { - fields: { - date: { - read(existing) { - return new Date(existing || dateString); + ])( + "results should be read from cache even when incomplete (fetchPolicy %s)", + (fetchPolicy) => { + const dateString = new Date().toISOString(); + + const cache = new InMemoryCache({ + typePolicies: { + Query: { + fields: { + date: { + read(existing) { + return new Date(existing || dateString); + }, }, }, }, }, - }, - }); - - const client = new ApolloClient({ - link: new ApolloLink(operation => new Observable(observer => { - observer.next({ - data: { - // This raw string should be converted to a Date by the Query.date - // read function passed to InMemoryCache below. - date: dateString, - // Make sure we don't accidentally return fields not mentioned in - // the query just because the result is incomplete. - ignored: "irrelevant to the subscribed query", - // Note the Query.missing field is, well, missing. - }, - }); - setTimeout(() => { - observer.complete(); - }, 10); - })), - cache, - }); - - const query = gql` - query { - date - missing - } - `; + }); - const observable = client.watchQuery({ - query, - fetchPolicy, // Varies with each test iteration - returnPartialData: true, - }); + const client = new ApolloClient({ + link: new ApolloLink( + (operation) => + new Observable((observer) => { + observer.next({ + data: { + // This raw string should be converted to a Date by the Query.date + // read function passed to InMemoryCache below. + date: dateString, + // Make sure we don't accidentally return fields not mentioned in + // the query just because the result is incomplete. + ignored: "irrelevant to the subscribed query", + // Note the Query.missing field is, well, missing. + }, + }); + setTimeout(() => { + observer.complete(); + }, 10); + }) + ), + cache, + }); - return new Promise((resolve, reject) => { - subscribeAndCount(reject, observable, (handleCount, result) => { - let adjustedCount = handleCount; - if ( - fetchPolicy === "network-only" || - fetchPolicy === "no-cache" || - fetchPolicy === "cache-only" - ) { - // The network-only, no-cache, and cache-only fetch policies do not - // deliver a loading:true result initially, so we adjust the - // handleCount to skip that case. - ++adjustedCount; + const query = gql` + query { + date + missing } + `; - // The only fetch policy that does not re-read results from the cache is - // the "no-cache" policy. In this test, that means the Query.date field - // will remain as a raw string rather than being converted to a Date by - // the read function. - const expectedDate = - fetchPolicy === "no-cache" ? dateString : new Date(dateString); + const observable = client.watchQuery({ + query, + fetchPolicy, // Varies with each test iteration + returnPartialData: true, + }); - if (adjustedCount === 1) { - expect(result.loading).toBe(true); - expect(result.data).toEqual({ - date: expectedDate, - }); + return new Promise((resolve, reject) => { + subscribeAndCount(reject, observable, (handleCount, result) => { + let adjustedCount = handleCount; + if ( + fetchPolicy === "network-only" || + fetchPolicy === "no-cache" || + fetchPolicy === "cache-only" + ) { + // The network-only, no-cache, and cache-only fetch policies do not + // deliver a loading:true result initially, so we adjust the + // handleCount to skip that case. + ++adjustedCount; + } - } else if (adjustedCount === 2) { - expect(result.loading).toBe(false); - expect(result.data).toEqual({ - date: expectedDate, - // The no-cache fetch policy does return extraneous fields from the - // raw network result that were not requested in the query, since - // the cache is not consulted. - ...(fetchPolicy === "no-cache" ? { - ignored: "irrelevant to the subscribed query" - } : null), - }); + // The only fetch policy that does not re-read results from the cache is + // the "no-cache" policy. In this test, that means the Query.date field + // will remain as a raw string rather than being converted to a Date by + // the read function. + const expectedDate = + fetchPolicy === "no-cache" ? dateString : new Date(dateString); + + if (adjustedCount === 1) { + expect(result.loading).toBe(true); + expect(result.data).toEqual({ + date: expectedDate, + }); + } else if (adjustedCount === 2) { + expect(result.loading).toBe(false); + expect(result.data).toEqual({ + date: expectedDate, + // The no-cache fetch policy does return extraneous fields from the + // raw network result that were not requested in the query, since + // the cache is not consulted. + ...(fetchPolicy === "no-cache" + ? { + ignored: "irrelevant to the subscribed query", + } + : null), + }); - if (fetchPolicy === "no-cache") { - // The "no-cache" fetch policy does not receive updates from the - // cache, so we finish the test early (passing). - setTimeout(() => resolve(), 20); + if (fetchPolicy === "no-cache") { + // The "no-cache" fetch policy does not receive updates from the + // cache, so we finish the test early (passing). + setTimeout(() => resolve(), 20); + } else { + cache.writeQuery({ + query: gql` + query { + missing + } + `, + data: { + missing: "not missing anymore", + }, + }); + } + } else if (adjustedCount === 3) { + expect(result.loading).toBe(false); + expect(result.data).toEqual({ + date: expectedDate, + missing: "not missing anymore", + }); - } else { - cache.writeQuery({ - query: gql`query { missing }`, - data: { + expect(cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + // The cache-only fetch policy does not receive updates from the + // network, so it never ends up writing the date field into the + // cache explicitly, though Query.date can still be synthesized by + // the read function. + ...(fetchPolicy === "cache-only" + ? null + : { + // Make sure this field is stored internally as a raw string. + date: dateString, + }), + // Written explicitly with cache.writeQuery above. missing: "not missing anymore", + // The ignored field is never written to the cache, because it is + // not included in the query. }, }); - } - - } else if (adjustedCount === 3) { - expect(result.loading).toBe(false); - expect(result.data).toEqual({ - date: expectedDate, - missing: "not missing anymore", - }); - - expect(cache.extract()).toEqual({ - ROOT_QUERY: { - __typename: "Query", - // The cache-only fetch policy does not receive updates from the - // network, so it never ends up writing the date field into the - // cache explicitly, though Query.date can still be synthesized by - // the read function. - ...(fetchPolicy === "cache-only" ? null : { - // Make sure this field is stored internally as a raw string. - date: dateString, - }), - // Written explicitly with cache.writeQuery above. - missing: "not missing anymore", - // The ignored field is never written to the cache, because it is - // not included in the query. - }, - }); - // Wait 20ms to give the test a chance to fail if there are unexpected - // additional results. - setTimeout(() => resolve(), 20); - - } else { - reject(new Error(`Unexpected count ${adjustedCount}`)); - } + // Wait 20ms to give the test a chance to fail if there are unexpected + // additional results. + setTimeout(() => resolve(), 20); + } else { + reject(new Error(`Unexpected count ${adjustedCount}`)); + } + }); }); - }); - }); + } + ); }); diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 799fcba922..15f8113219 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1180,7 +1180,7 @@ export class QueryManager { result, linkDocument, options, - cacheWriteBehavior, + cacheWriteBehavior ); queryInfo.markReady(); } diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 6846523e69..3cd7a0158e 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -5708,9 +5708,7 @@ describe("useQuery Hook", () => { ); const { vine, ...carDataWithoutVine } = carData.cars[0]; expect(result.current.data).toEqual({ - cars: [ - carDataWithoutVine, - ], + cars: [carDataWithoutVine], }); expect(result.current.error).toBeUndefined(); From d8c6072e255529a42448e26d00e8d849e4e1c219 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 13 Sep 2023 16:51:34 -0400 Subject: [PATCH 6/8] Bump size-limit +8 bytes. --- .size-limit.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index 096d4ebcf8..064303a8ea 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -10,7 +10,7 @@ const checks = [ { path: "dist/index.js", import: "{ ApolloClient, InMemoryCache, HttpLink }", - limit: "32044", + limit: "32052", }, ...[ "ApolloProvider", From 2ad84850ca5c812ca9d0bbb818190389b2bd43d2 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 14 Sep 2023 15:24:35 +0200 Subject: [PATCH 7/8] extend test to distinguish between cache & network --- src/cache/inmemory/__tests__/client.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/cache/inmemory/__tests__/client.ts b/src/cache/inmemory/__tests__/client.ts index 86c4e8ee07..c3844cb20c 100644 --- a/src/cache/inmemory/__tests__/client.ts +++ b/src/cache/inmemory/__tests__/client.ts @@ -18,7 +18,8 @@ describe("InMemoryCache tests exercising ApolloClient", () => { ])( "results should be read from cache even when incomplete (fetchPolicy %s)", (fetchPolicy) => { - const dateString = new Date().toISOString(); + const dateFromCache = "2023-09-14T13:03:22.616Z"; + const dateFromNetwork = "2023-09-15T13:03:22.616Z"; const cache = new InMemoryCache({ typePolicies: { @@ -26,7 +27,7 @@ describe("InMemoryCache tests exercising ApolloClient", () => { fields: { date: { read(existing) { - return new Date(existing || dateString); + return new Date(existing || dateFromCache); }, }, }, @@ -42,7 +43,7 @@ describe("InMemoryCache tests exercising ApolloClient", () => { data: { // This raw string should be converted to a Date by the Query.date // read function passed to InMemoryCache below. - date: dateString, + date: dateFromNetwork, // Make sure we don't accidentally return fields not mentioned in // the query just because the result is incomplete. ignored: "irrelevant to the subscribed query", @@ -88,18 +89,22 @@ describe("InMemoryCache tests exercising ApolloClient", () => { // the "no-cache" policy. In this test, that means the Query.date field // will remain as a raw string rather than being converted to a Date by // the read function. - const expectedDate = - fetchPolicy === "no-cache" ? dateString : new Date(dateString); + const expectedDateAfterResult = + fetchPolicy === "cache-only" + ? new Date(dateFromCache) + : fetchPolicy === "no-cache" + ? dateFromNetwork + : new Date(dateFromNetwork); if (adjustedCount === 1) { expect(result.loading).toBe(true); expect(result.data).toEqual({ - date: expectedDate, + date: new Date(dateFromCache), }); } else if (adjustedCount === 2) { expect(result.loading).toBe(false); expect(result.data).toEqual({ - date: expectedDate, + date: expectedDateAfterResult, // The no-cache fetch policy does return extraneous fields from the // raw network result that were not requested in the query, since // the cache is not consulted. @@ -129,7 +134,7 @@ describe("InMemoryCache tests exercising ApolloClient", () => { } else if (adjustedCount === 3) { expect(result.loading).toBe(false); expect(result.data).toEqual({ - date: expectedDate, + date: expectedDateAfterResult, missing: "not missing anymore", }); @@ -144,7 +149,7 @@ describe("InMemoryCache tests exercising ApolloClient", () => { ? null : { // Make sure this field is stored internally as a raw string. - date: dateString, + date: dateFromNetwork, }), // Written explicitly with cache.writeQuery above. missing: "not missing anymore", From ec5d86c2efb676870da4e1857c1fc578fa32958c Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 14 Sep 2023 18:12:24 +0200 Subject: [PATCH 8/8] defensive copy of `result` at the top of `markResult` --- src/core/QueryInfo.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index 44540c281b..f666456160 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -362,6 +362,7 @@ export class QueryInfo { >, cacheWriteBehavior: CacheWriteBehavior ): typeof result { + result = { ...result }; const merger = new DeepMerger(); const graphQLErrors = isNonEmptyArray(result.errors) ? result.errors.slice(0) @@ -451,10 +452,7 @@ export class QueryInfo { if (this.lastDiff && this.lastDiff.diff.complete) { // Reuse data from the last good (complete) diff that we // received, when possible. - result = { - ...result, - data: this.lastDiff.diff.result, - }; + result.data = this.lastDiff.diff.result; return; } // If the previous this.diff was incomplete, fall through to @@ -481,11 +479,7 @@ export class QueryInfo { // result. Set without setDiff to avoid triggering a notify call, // since we have other ways of notifying for this result. this.updateLastDiff(diff, diffOptions); - result = { - ...result, - // TODO Improve types so we don't need this cast. - data: diff.result as any, - }; + result.data = diff.result; }); } else { this.lastWrite = void 0;