Skip to content

Commit

Permalink
Avoid warning about clobbering scalar fields.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
benjamn committed Sep 25, 2020
1 parent 91299cc commit db67108
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 2 deletions.
34 changes: 34 additions & 0 deletions src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap
Original file line number Diff line number Diff line change
@@ -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": 81,
},
"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 {
Expand Down
64 changes: 64 additions & 0 deletions src/cache/inmemory/__tests__/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.getMilliseconds(),
},
},
});

expect(cache.extract()).toMatchSnapshot();
expect(warnings).toEqual([]);
});
});

describe('writeResultToStore shape checking', () => {
const query = gql`
query {
Expand Down
16 changes: 14 additions & 2 deletions src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,27 @@ export class StoreWriter {
}

if (process.env.NODE_ENV !== "production") {
const hasSelectionSet = (storeFieldName: string) =>
fieldsWithSelectionSets.has(fieldNameFromStoreName(storeFieldName));
const fieldsWithSelectionSets = new Set<string>();
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);
};

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,
Expand Down

0 comments on commit db67108

Please sign in to comment.