From a4f9ca54e05f337204df2b2e2d59c1a92afb8fa2 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 24 Sep 2020 19:12:36 -0400 Subject: [PATCH 1/3] Avoid warning about clobbering scalar fields. While it is possible to write a field merge function that handles updates for scalar field values, we should treat scalar data as opaque by default, without displaying "Cache data may be lost..." warnings that nudge the developer to write an unnecessary merge function. None: because this code is inside of a process.env.NODE_ENV block, it will not be executed or bundled in production. Should fix #7071. --- .../__snapshots__/writeToStore.ts.snap | 34 ++++++++++ src/cache/inmemory/__tests__/writeToStore.ts | 64 +++++++++++++++++++ src/cache/inmemory/writeToStore.ts | 16 ++++- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap index e5b7f49cdcb..894a3eab600 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap @@ -1,5 +1,39 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`writing to the store "Cache data maybe lost..." warnings should not warn when scalar fields are updated 1`] = ` +Object { + "ROOT_QUERY": Object { + "__typename": "Query", + "currentTime({\\"tz\\":\\"UTC-5\\"})": Object { + "localeString": "9/25/2020, 1:08:33 PM", + }, + "someJSON": Object { + "foos": Array [ + "bar", + "baz", + ], + "oyez": 3, + }, + }, +} +`; + +exports[`writing to the store "Cache data maybe lost..." warnings should not warn when scalar fields are updated 2`] = ` +Object { + "ROOT_QUERY": Object { + "__typename": "Query", + "currentTime({\\"tz\\":\\"UTC-5\\"})": Object { + "msSinceEpoch": 1601053713081, + }, + "someJSON": Object { + "asdf": "middle", + "qwer": "upper", + "zxcv": "lower", + }, + }, +} +`; + exports[`writing to the store user objects should be able to have { __typename: "Mutation" } 1`] = ` Object { "Gene:{\\"id\\":\\"SLC45A2\\"}": Object { diff --git a/src/cache/inmemory/__tests__/writeToStore.ts b/src/cache/inmemory/__tests__/writeToStore.ts index 668f299e29e..dd8b37beb69 100644 --- a/src/cache/inmemory/__tests__/writeToStore.ts +++ b/src/cache/inmemory/__tests__/writeToStore.ts @@ -1582,6 +1582,70 @@ describe('writing to the store', () => { }); }); + describe('"Cache data maybe lost..." warnings', () => { + const { warn } = console; + let warnings: any[][] = []; + + beforeEach(() => { + warnings.length = 0; + console.warn = (...args: any[]) => { + warnings.push(args); + }; + }); + + afterEach(() => { + console.warn = warn; + }); + + it("should not warn when scalar fields are updated", () => { + const cache = new InMemoryCache; + + const query = gql` + query { + someJSON + currentTime(tz: "UTC-5") + } + `; + + expect(warnings).toEqual([]); + + const date = new Date(1601053713081); + + cache.writeQuery({ + query, + data: { + someJSON: { + oyez: 3, + foos: ["bar", "baz"], + }, + currentTime: { + localeString: date.toLocaleString("en-US"), + }, + }, + }); + + expect(cache.extract()).toMatchSnapshot(); + expect(warnings).toEqual([]); + + cache.writeQuery({ + query, + data: { + someJSON: { + qwer: "upper", + asdf: "middle", + zxcv: "lower", + }, + currentTime: { + msSinceEpoch: date.getTime(), + }, + }, + }); + + expect(cache.extract()).toMatchSnapshot(); + expect(warnings).toEqual([]); + }); + }); + describe('writeResultToStore shape checking', () => { const query = gql` query { diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index bb0d8c43864..afef93e9a23 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -284,6 +284,15 @@ export class StoreWriter { } if (process.env.NODE_ENV !== "production") { + const hasSelectionSet = (storeFieldName: string) => + fieldsWithSelectionSets.has(fieldNameFromStoreName(storeFieldName)); + const fieldsWithSelectionSets = new Set(); + workSet.forEach(selection => { + if (isField(selection) && selection.selectionSet) { + fieldsWithSelectionSets.add(selection.name.value); + } + }); + const hasMergeFunction = (storeFieldName: string) => { const childTree = mergeTree.map.get(storeFieldName); return Boolean(childTree && childTree.info && childTree.info.merge); @@ -291,8 +300,11 @@ export class StoreWriter { Object.keys(incomingFields).forEach(storeFieldName => { // If a merge function was defined for this field, trust that it - // did the right thing about (not) clobbering data. - if (!hasMergeFunction(storeFieldName)) { + // did the right thing about (not) clobbering data. If the field + // has no selection set, it's a scalar field, so it doesn't need + // a merge function (even if it's an object, like JSON data). + if (hasSelectionSet(storeFieldName) && + !hasMergeFunction(storeFieldName)) { warnAboutDataLoss( entityRef, incomingFields, From 6950eb5e2be30e16f071cc76e84327e78eff4912 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 25 Sep 2020 13:29:22 -0400 Subject: [PATCH 2/3] Specify timeZone to avoid test failures in other time zones. --- src/cache/inmemory/__tests__/writeToStore.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cache/inmemory/__tests__/writeToStore.ts b/src/cache/inmemory/__tests__/writeToStore.ts index dd8b37beb69..2bb077325cc 100644 --- a/src/cache/inmemory/__tests__/writeToStore.ts +++ b/src/cache/inmemory/__tests__/writeToStore.ts @@ -1619,7 +1619,9 @@ describe('writing to the store', () => { foos: ["bar", "baz"], }, currentTime: { - localeString: date.toLocaleString("en-US"), + localeString: date.toLocaleString("en-US", { + timeZone: "America/New_York", + }), }, }, }); From 901bb18f360bf18972e0d869c61f21bca8d7e499 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 25 Sep 2020 13:32:41 -0400 Subject: [PATCH 3/3] Mention PR #7075 in CHANGELOG.md. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b58d6e4f71e..fa43090a6fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ - Shallow-merge `options.variables` when combining existing or default options with newly-provided options, so new variables do not completely overwrite existing variables.
[@amannn](https://github.com/amannn) in [#6927](https://github.com/apollographql/apollo-client/pull/6927) +- Avoid displaying `Cache data may be lost...` warnings for scalar field values that happen to be objects, such as JSON data.
+ [@benjamn](https://github.com/benjamn) in [#7075](https://github.com/apollographql/apollo-client/pull/7075) + ## Apollo Client 3.2.1 ## Bug Fixes