Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Efficiently canonicalize InMemoryCache result objects. #7439

Merged
merged 12 commits into from Dec 16, 2020
Merged
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -56,7 +56,7 @@
{
"name": "apollo-client",
"path": "./dist/apollo-client.cjs.min.js",
"maxSize": "25.5 kB"
"maxSize": "25.9 kB"
}
],
"peerDependencies": {
Expand Down
12 changes: 2 additions & 10 deletions src/__tests__/client.ts
Expand Up @@ -3037,11 +3037,9 @@ describe('@connection', () => {
client.cache.evict({ fieldName: "a" });
await wait();

// The results are structurally the same, but the result objects have
// been recomputed for queries that involved the ROOT_QUERY.a field.
expect(checkLastResult(aResults, a456)).not.toBe(a456);
expect(checkLastResult(aResults, a456)).toBe(a456);
expect(checkLastResult(bResults, bOyez)).toBe(bOyez);
expect(checkLastResult(abResults, a456bOyez)).not.toBe(a456bOyez);
expect(checkLastResult(abResults, a456bOyez)).toBe(a456bOyez);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very Shakespearean of you


const cQuery = gql`{ c }`;
// Passing cache-only as the fetchPolicy allows the { c: "see" }
Expand Down Expand Up @@ -3090,25 +3088,19 @@ describe('@connection', () => {
{ a: 123 },
{ a: 234 },
{ a: 456 },
// Delivered again because we explicitly called resetLastResults.
{ a: 456 },
]);

expect(bResults).toEqual([
{ b: "asdf" },
{ b: "ASDF" },
{ b: "oyez" },
// Delivered again because we explicitly called resetLastResults.
{ b: "oyez" },
]);

expect(abResults).toEqual([
{ a: 123, b: "asdf" },
{ a: 234, b: "asdf" },
{ a: 234, b: "ASDF" },
{ a: 456, b: "oyez" },
// Delivered again because we explicitly called resetLastResults.
{ a: 456, b: "oyez" },
]);

expect(cResults).toEqual([
Expand Down
18 changes: 18 additions & 0 deletions src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap
@@ -0,0 +1,18 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`reading from the store returns === results for different queries 1`] = `
Object {
"ROOT_QUERY": Object {
"__typename": "Query",
"a": Array [
"a",
"y",
"y",
],
"b": Object {
"c": "C",
"d": "D",
},
},
}
`;
7 changes: 3 additions & 4 deletions src/cache/inmemory/__tests__/cache.ts
Expand Up @@ -1728,8 +1728,8 @@ describe("InMemoryCache#modify", () => {
})).toBe(false); // Nothing actually modified.

const resultAfterAuthorInvalidation = read();
expect(resultAfterAuthorInvalidation).not.toBe(initialResult);
expect(resultAfterAuthorInvalidation).toEqual(initialResult);
expect(resultAfterAuthorInvalidation).toBe(initialResult);

expect(cache.modify({
id: cache.identify({
Expand All @@ -1743,8 +1743,8 @@ describe("InMemoryCache#modify", () => {
})).toBe(false); // Nothing actually modified.

const resultAfterBookInvalidation = read();
expect(resultAfterBookInvalidation).not.toBe(resultAfterAuthorInvalidation);
expect(resultAfterBookInvalidation).toEqual(resultAfterAuthorInvalidation);
expect(resultAfterBookInvalidation).toBe(resultAfterAuthorInvalidation);
expect(resultAfterBookInvalidation.currentlyReading.author).toEqual({
__typename: "Author",
name: "Maria Dahvana Headley",
Expand Down Expand Up @@ -2591,9 +2591,8 @@ describe("ReactiveVar and makeVar", () => {
});

const result2 = cache.readQuery({ query });
// Without resultCaching, equivalent results will not be ===.
expect(result2).not.toBe(result1);
expect(result2).toEqual(result1);
expect(result2).toBe(result1);

expect(nameVar()).toBe("Ben");
expect(nameVar("Hugh")).toBe("Hugh");
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/__tests__/optimistic.ts
Expand Up @@ -431,7 +431,7 @@ describe('optimistic cache layers', () => {

const resultAfterRemovingBuzzLayer = readWithAuthors();
expect(resultAfterRemovingBuzzLayer).toEqual(resultWithBuzz);
expect(resultAfterRemovingBuzzLayer).not.toBe(resultWithBuzz);
expect(resultAfterRemovingBuzzLayer).toBe(resultWithBuzz);
resultWithTwoAuthors.books.forEach((book, i) => {
expect(book).toEqual(resultAfterRemovingBuzzLayer.books[i]);
expect(book).toBe(resultAfterRemovingBuzzLayer.books[i]);
Expand Down
13 changes: 1 addition & 12 deletions src/cache/inmemory/__tests__/policies.ts
Expand Up @@ -4692,19 +4692,8 @@ describe("type policies", function () {
});

const thirdFirstBookResult = readFirstBookResult();

// A change in VW's books field triggers rereading of result objects
// that previously involved her books field.
expect(thirdFirstBookResult).not.toBe(secondFirstBookResult);

// However, since the new Book was not the earliest published, the
// second and third results are structurally the same.
expect(thirdFirstBookResult).toEqual(secondFirstBookResult);

// In fact, the original author.firstBook object has been reused!
expect(thirdFirstBookResult.author.firstBook).toBe(
secondFirstBookResult.author.firstBook,
);
expect(thirdFirstBookResult).toBe(secondFirstBookResult);
});

it("readField can read fields with arguments", function () {
Expand Down
152 changes: 151 additions & 1 deletion src/cache/inmemory/__tests__/readFromStore.ts
Expand Up @@ -2,13 +2,19 @@ import { assign, omit } from 'lodash';
import gql from 'graphql-tag';

import { stripSymbols } from '../../../utilities/testing/stripSymbols';
import { InMemoryCache } from '../inMemoryCache';
import { StoreObject } from '../types';
import { StoreReader } from '../readFromStore';
import { makeReference, InMemoryCache, Reference, isReference } from '../../../core';
import { Cache } from '../../core/types/Cache';
import { MissingFieldError } from '../../core/types/common';
import { defaultNormalizedCacheFactory, readQueryFromStore } from './helpers';
import { withError } from './diffAgainstStore';
import {
makeReference,
Reference,
isReference,
TypedDocumentNode,
} from '../../../core';

describe('reading from the store', () => {
const reader = new StoreReader({
Expand Down Expand Up @@ -1827,4 +1833,148 @@ describe('reading from the store', () => {
},
});
});

it("returns === results for different queries", function () {
const cache = new InMemoryCache;

const aQuery: TypedDocumentNode<{
a: string[];
}> = gql`query { a }`;

const abQuery: TypedDocumentNode<{
a: string[];
b: {
c: string;
d: string;
};
}> = gql`query { a b { c d } }`;

const bQuery: TypedDocumentNode<{
b: {
c: string;
d: string;
};
}> = gql`query { b { d c } }`;

const abData1 = {
a: ["a", "y"],
b: {
c: "see",
d: "dee",
},
};

cache.writeQuery({
query: abQuery,
data: abData1,
});

function read<Data, Vars>(query: TypedDocumentNode<Data, Vars>) {
return cache.readQuery({ query })!;
}

const aResult1 = read(aQuery);
const abResult1 = read(abQuery);
const bResult1 = read(bQuery);

expect(aResult1.a).toBe(abResult1.a);
expect(abResult1).toEqual(abData1);
expect(aResult1).toEqual({ a: abData1.a });
expect(bResult1).toEqual({ b: abData1.b });
expect(abResult1.b).toBe(bResult1.b);

const aData2 = {
a: "ayy".split(""),
};

cache.writeQuery({
query: aQuery,
data: aData2,
});

const aResult2 = read(aQuery);
const abResult2 = read(abQuery);
const bResult2 = read(bQuery);

expect(aResult2).toEqual(aData2);
expect(abResult2).toEqual({ ...abData1, ...aData2 });
expect(aResult2.a).toBe(abResult2.a);
expect(bResult2).toBe(bResult1);
expect(abResult2.b).toBe(bResult2.b);
expect(abResult2.b).toBe(bResult1.b);

const bData3 = {
b: {
d: "D",
c: "C",
},
};

cache.writeQuery({
query: bQuery,
data: bData3,
});

const aResult3 = read(aQuery);
const abResult3 = read(abQuery);
const bResult3 = read(bQuery);

expect(aResult3).toBe(aResult2);
expect(bResult3).toEqual(bData3);
expect(bResult3).not.toBe(bData3);
expect(abResult3).toEqual({
...abResult2,
...bData3,
});

expect(cache.extract()).toMatchSnapshot();
});

it("does not canonicalize custom scalar objects", function () {
const now = new Date;
const abc = { a: 1, b: 2, c: 3 };

const cache = new InMemoryCache({
typePolicies: {
Query: {
fields: {
now() {
return now;
},

abc() {
return abc;
},
},
},
},
});

const query: TypedDocumentNode<{
now: typeof now;
abc: typeof abc;
}> = gql`query { now abc }`;

const result1 = cache.readQuery({ query })!;
const result2 = cache.readQuery({ query })!;

expect(result1).toBe(result2);
expect(result1.now).toBeInstanceOf(Date);

// We already know result1.now === result2.now, but it's also
// important that it be the very same (===) Date object that was
// returned from the read function for the Query.now field, not a
// canonicalized version.
expect(result1.now).toBe(now);
expect(result2.now).toBe(now);

// The Query.abc field returns a "normal" object, but we know from the
// structure of the query that it's a scalar object, so it will not be
// canonicalized.
expect(result1.abc).toEqual(abc);
expect(result2.abc).toEqual(abc);
expect(result1.abc).toBe(result2.abc);
expect(result1.abc).toBe(abc);
expect(result2.abc).toBe(abc);
});
});
5 changes: 4 additions & 1 deletion src/cache/inmemory/helpers.ts
Expand Up @@ -12,7 +12,10 @@ import {
shouldInclude,
} from '../../utilities';

export const hasOwn = Object.prototype.hasOwnProperty;
export const {
hasOwnProperty: hasOwn,
toString: objToStr,
} = Object.prototype;

export function getTypenameFromStoreObject(
store: NormalizedCache,
Expand Down