From 7ff1b31d3404eacf2de3c7862d98f70d42160510 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 23 Nov 2020 18:31:13 -0500 Subject: [PATCH 01/12] Efficiently canonicalize InMemoryCache result objects. https://github.com/apollographql/apollo-client/issues/4141#issuecomment-733091694 --- src/__tests__/client.ts | 12 +-- src/cache/inmemory/__tests__/cache.ts | 7 +- src/cache/inmemory/__tests__/optimistic.ts | 2 +- src/cache/inmemory/__tests__/policies.ts | 13 +-- src/cache/inmemory/canon.ts | 115 +++++++++++++++++++++ src/cache/inmemory/helpers.ts | 5 +- src/cache/inmemory/readFromStore.ts | 17 ++- src/core/__tests__/QueryManager/index.ts | 5 +- 8 files changed, 133 insertions(+), 43 deletions(-) create mode 100644 src/cache/inmemory/canon.ts diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index dcf4f745c8c..84536300382 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -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); const cQuery = gql`{ c }`; // Passing cache-only as the fetchPolicy allows the { c: "see" } @@ -3090,16 +3088,12 @@ 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([ @@ -3107,8 +3101,6 @@ describe('@connection', () => { { 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([ diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index f58542e4449..1e4089893c7 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -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({ @@ -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", @@ -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"); diff --git a/src/cache/inmemory/__tests__/optimistic.ts b/src/cache/inmemory/__tests__/optimistic.ts index b5d7b79a29f..8030d43ed49 100644 --- a/src/cache/inmemory/__tests__/optimistic.ts +++ b/src/cache/inmemory/__tests__/optimistic.ts @@ -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]); diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 86315bbc84f..33fc3bc4b99 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -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 () { diff --git a/src/cache/inmemory/canon.ts b/src/cache/inmemory/canon.ts new file mode 100644 index 00000000000..628dbcd79bf --- /dev/null +++ b/src/cache/inmemory/canon.ts @@ -0,0 +1,115 @@ +import { KeyTrie } from "optimism"; +import { canUseWeakMap } from "../../utilities"; +import { objToStr } from "./helpers"; + +// When we say an object is "canonical" in programming, we mean it has been +// admitted into some abstract "canon" of official/blessed objects. This +// Canon class is a representation of such a collection, with the property +// that canon.admit(value1) === canon.admit(value2) if value1 and value2 are +// deeply equal to each other. The canonicalization process involves looking +// at every property in the provided object tree, so it takes the same order +// of time as deep equality checking (linear time), but already-admitted +// objects are returned immediately from canon.admit, so ensuring subtrees +// have already been canonized tends to speed up canonicalization. Of +// course, since canonized objects may be shared widely between unrelated +// consumers, it's important to regard them as immutable. No detection of +// cycles is needed by the StoreReader class right now, so we don't bother +// keeping track of objects we've already seen during the recursion of the +// admit method. Objects whose internal class name is neither Array nor +// Object can be included in the value tree, but they will not be replaced +// with a canonical version (to put it another way, they are assumed to be +// canonical already). We can easily add additional cases to the switch +// statement to handle other common object types, such as "[object Date]" +// objects, as needed. +export class Canon { + // All known objects this Canon has admitted. + private known = new (canUseWeakMap ? WeakSet : Set)(); + + // Efficient storage/lookup structure for admitting objects. + private pool = new KeyTrie<{ + array?: any[]; + object?: Record; + keys?: SortedKeysInfo; + }>(canUseWeakMap); + + // Returns the canonical version of value. + public admit(value: T): T; + public admit(value: any) { + if (value && typeof value === "object") { + switch (objToStr.call(value)) { + case "[object Array]": { + if (this.known.has(value)) return value; + const array: any[] = value.map(this.admit, this); + // Arrays are looked up in the KeyTrie using their recursively + // canonicalized elements, and the known version of the array is + // preserved as node.array. + const node = this.pool.lookupArray(array); + if (!node.array) { + this.known.add(node.array = array); + if (process.env.NODE_ENV !== "production") { + Object.freeze(array); + } + } + return node.array; + } + + case "[object Object]": { + if (this.known.has(value)) return value; + const proto = Object.getPrototypeOf(value); + const array = [proto]; + const keys = this.sortedKeys(value); + array.push(keys.json); + keys.sorted.forEach(key => { + array.push(this.admit(value[key])); + }); + // Objects are looked up in the KeyTrie by their prototype + // (which is *not* recursively canonicalized), followed by a + // JSON representation of their (sorted) keys, followed by the + // sequence of recursively canonicalized values corresponding to + // those keys. To keep the final results unambiguous with other + // sequences (such as arrays that just happen to contain [proto, + // keys.json, value1, value2, ...]), the known version of the + // object is stored as node.object. + const node = this.pool.lookupArray(array); + if (!node.object) { + const obj = node.object = Object.create(proto); + this.known.add(obj); + keys.sorted.forEach((key, i) => { + obj[key] = array[i + 2]; + }); + if (process.env.NODE_ENV !== "production") { + Object.freeze(obj); + } + } + return node.object; + } + } + } + return value; + } + + // It's worthwhile to cache the sorting of arrays of strings, since the + // same initial unsorted arrays tend to be encountered many times. + // Fortunately, we can reuse the KeyTrie machinery to look up the sorted + // arrays in linear time (which is faster than sorting large arrays). + private sortedKeys(obj: object) { + const keys = Object.keys(obj); + const node = this.pool.lookupArray(keys); + if (!node.keys) { + keys.sort(); + const json = JSON.stringify(keys); + if (!(node.keys = this.keysByJSON.get(json))) { + this.keysByJSON.set(json, node.keys = { sorted: keys, json }); + } + } + return node.keys; + } + // Arrays that contain the same elements in a different order can share + // the same SortedKeysInfo object, to save memory. + private keysByJSON = new Map(); +} + +type SortedKeysInfo = { + sorted: string[]; + json: string; +}; diff --git a/src/cache/inmemory/helpers.ts b/src/cache/inmemory/helpers.ts index dc12bc65838..9777c8330a0 100644 --- a/src/cache/inmemory/helpers.ts +++ b/src/cache/inmemory/helpers.ts @@ -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, diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 1abafb40f30..464a50a4548 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -36,6 +36,7 @@ import { getTypenameFromStoreObject } from './helpers'; import { Policies } from './policies'; import { InMemoryCache } from './inMemoryCache'; import { MissingFieldError } from '../core/types/common'; +import { Canon } from './canon'; export type VariableMap = { [name: string]: any }; @@ -324,11 +325,7 @@ export class StoreReader { // Perform a single merge at the end so that we can avoid making more // defensive shallow copies than necessary. - finalResult.result = mergeDeepArray(objectsToMerge); - - if (process.env.NODE_ENV !== 'production') { - Object.freeze(finalResult.result); - } + finalResult.result = this.canon.admit(mergeDeepArray(objectsToMerge)); // Store this result with its selection set so that we can quickly // recognize it again in the StoreReader#isFresh method. @@ -337,6 +334,8 @@ export class StoreReader { return finalResult; } + private canon = new Canon; + private knownResults = new WeakMap, SelectionSetNode>(); // Cached version of execSubSelectedArrayImpl. @@ -377,7 +376,7 @@ export class StoreReader { array = array.filter(context.store.canRead); } - array = array.map((item, i) => { + array = this.canon.admit(array.map((item, i) => { // null value in array if (item === null) { return null; @@ -410,11 +409,7 @@ export class StoreReader { invariant(context.path.pop() === i); return item; - }); - - if (process.env.NODE_ENV !== 'production') { - Object.freeze(array); - } + })); return { result: array, missing }; } diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 26e052834e9..66cfb7c86cb 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -902,10 +902,7 @@ describe('QueryManager', () => { break; case 2: expect(stripSymbols(result.data)).toEqual(data3); - expect(result.data).not.toBe(firstResultData); - expect(result.data.b).toEqual(firstResultData.b); - expect(result.data.d).not.toBe(firstResultData.d); - expect(result.data.d.f).toEqual(firstResultData.d.f); + expect(result.data).toBe(firstResultData); resolve(); break; default: From 789f46b83a0009d428e277f45a312c1f5b670b07 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 15:42:10 -0500 Subject: [PATCH 02/12] Pass scalar field values through without canonicalizing them. --- src/cache/inmemory/canon.ts | 15 +++++++++++++++ src/cache/inmemory/readFromStore.ts | 16 ++++------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/cache/inmemory/canon.ts b/src/cache/inmemory/canon.ts index 628dbcd79bf..f3b20e52ef5 100644 --- a/src/cache/inmemory/canon.ts +++ b/src/cache/inmemory/canon.ts @@ -2,6 +2,10 @@ import { KeyTrie } from "optimism"; import { canUseWeakMap } from "../../utilities"; import { objToStr } from "./helpers"; +class Pass { + constructor(public readonly value: T) {} +} + // When we say an object is "canonical" in programming, we mean it has been // admitted into some abstract "canon" of official/blessed objects. This // Canon class is a representation of such a collection, with the property @@ -32,10 +36,21 @@ export class Canon { keys?: SortedKeysInfo; }>(canUseWeakMap); + // Make the ObjectCanon assume this value has already been + // canonicalized. + public pass(value: T): Pass; + public pass(value: any) { + return new Pass(value); + } + // Returns the canonical version of value. public admit(value: T): T; public admit(value: any) { if (value && typeof value === "object") { + if (value instanceof Pass) { + return value.value; + } + switch (objToStr.call(value)) { case "[object Array]": { if (this.known.has(value)) return value; diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 464a50a4548..5414ce26de0 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -21,7 +21,6 @@ import { getFragmentDefinitions, getMainDefinition, getQueryDefinition, - maybeDeepFreeze, mergeDeepArray, getFragmentFromSelection, } from '../../utilities'; @@ -280,17 +279,10 @@ export class StoreReader { } else if (!selection.selectionSet) { // If the field does not have a selection set, then we handle it - // as a scalar value. However, that value should not contain any - // Reference objects, and should be frozen in development, if it - // happens to be an object that is mutable. - if (process.env.NODE_ENV !== 'production') { - assertSelectionSetForIdValue( - context.store, - selection, - fieldValue, - ); - maybeDeepFreeze(fieldValue); - } + // as a scalar value. To keep this.canon from canonicalizing + // this value, we use this.canon.pass to wrap fieldValue in a + // Pass object that this.canon.admit will later unwrap as-is. + fieldValue = this.canon.pass(fieldValue); } else if (fieldValue != null) { // In this case, because we know the field has a selection set, From 05c362f24a5f2c247e150318ab76eb0fd97272ae Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 15:42:17 -0500 Subject: [PATCH 03/12] Test === equality for partial results from different queries. --- .../__snapshots__/readFromStore.ts.snap | 18 +++ src/cache/inmemory/__tests__/readFromStore.ts | 104 +++++++++++++++++- 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap diff --git a/src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap new file mode 100644 index 00000000000..46732d054ee --- /dev/null +++ b/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", + }, + }, +} +`; diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index d46c79b89ca..bf3003c4b5b 100644 --- a/src/cache/inmemory/__tests__/readFromStore.ts +++ b/src/cache/inmemory/__tests__/readFromStore.ts @@ -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({ @@ -1827,4 +1833,100 @@ 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(query: TypedDocumentNode) { + 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(); + }); }); From 3059ee351381f68616cd8431b68de653380b7aab Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 16:20:07 -0500 Subject: [PATCH 04/12] Bump bundlesize limit from 24.5kB to 24.9kB. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e9d5ba125e9..610e333294f 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ { "name": "apollo-client", "path": "./dist/apollo-client.cjs.min.js", - "maxSize": "25.5 kB" + "maxSize": "25.9 kB" } ], "peerDependencies": { From 8989ad3e97cd3da750afc753f0cf18d40b1b894d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 16:44:28 -0500 Subject: [PATCH 05/12] Test that canonicalization does not modify scalar objects. --- src/cache/inmemory/__tests__/readFromStore.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index bf3003c4b5b..bec47f710eb 100644 --- a/src/cache/inmemory/__tests__/readFromStore.ts +++ b/src/cache/inmemory/__tests__/readFromStore.ts @@ -1929,4 +1929,52 @@ describe('reading from the store', () => { 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); + }); }); From cad1a06c18d963130539fd31f7067288132cb259 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 17:50:31 -0500 Subject: [PATCH 06/12] Improve Canon class comments. --- src/cache/inmemory/canon.ts | 81 ++++++++++++++++++++++++++++--------- 1 file changed, 62 insertions(+), 19 deletions(-) diff --git a/src/cache/inmemory/canon.ts b/src/cache/inmemory/canon.ts index f3b20e52ef5..7572c4fd4fa 100644 --- a/src/cache/inmemory/canon.ts +++ b/src/cache/inmemory/canon.ts @@ -6,30 +6,67 @@ class Pass { constructor(public readonly value: T) {} } -// When we say an object is "canonical" in programming, we mean it has been -// admitted into some abstract "canon" of official/blessed objects. This -// Canon class is a representation of such a collection, with the property -// that canon.admit(value1) === canon.admit(value2) if value1 and value2 are -// deeply equal to each other. The canonicalization process involves looking -// at every property in the provided object tree, so it takes the same order -// of time as deep equality checking (linear time), but already-admitted -// objects are returned immediately from canon.admit, so ensuring subtrees -// have already been canonized tends to speed up canonicalization. Of -// course, since canonized objects may be shared widely between unrelated -// consumers, it's important to regard them as immutable. No detection of -// cycles is needed by the StoreReader class right now, so we don't bother -// keeping track of objects we've already seen during the recursion of the -// admit method. Objects whose internal class name is neither Array nor -// Object can be included in the value tree, but they will not be replaced -// with a canonical version (to put it another way, they are assumed to be -// canonical already). We can easily add additional cases to the switch +// When programmers talk about the "canonical form" of an object, they +// usually have the following meaning in mind, which I've copied from +// https://en.wiktionary.org/wiki/canonical_form: +// +// 1. A standard or normal presentation of a mathematical entity [or +// object]. A canonical form is an element of a set of representatives +// of equivalence classes of forms such that there is a function or +// procedure which projects every element of each equivalence class +// onto that one element, the canonical form of that equivalence +// class. The canonical form is expected to be simpler than the rest of +// the forms in some way. +// +// That's a long-winded way of saying any two objects that have the same +// canonical form may be considered equivalent, even if they are !==, +// which usually means the objects are structurally equivalent (deeply +// equal), but don't necessarily use the same memory. +// +// Like a literary or musical canon, this Canon class represents a +// collection of unique canonical items (JavaScript objects), with the +// important property that canon.admit(a) === canon.admit(b) if a and b +// are deeply equal to each other. In terms of the definition above, the +// canon.admit method is the "function or procedure which projects every" +// object "onto that one element, the canonical form." +// +// In the worst case, the canonicalization process may involve looking at +// every property in the provided object tree, so it takes the same order +// of time as deep equality checking. Fortunately, already-canonicalized +// objects are returned immediately from canon.admit, so the presence of +// canonical subtrees tends to speed up canonicalization. +// +// Since consumers of canonical objects can check for deep equality in +// constant time, canonicalizing cache results can massively improve the +// performance of application code that skips re-rendering unchanged +// results, such as "pure" UI components in a framework like React. +// +// Of course, since canonical objects may be shared widely between +// unrelated consumers, it's important to think of them as immutable, even +// though they are not actually frozen with Object.freeze in production, +// due to the extra performance overhead that comes with frozen objects. +// +// Custom scalar objects whose internal class name is neither Array nor +// Object can be included safely in the admitted tree, but they will not +// be replaced with a canonical version (to put it another way, they are +// assumed to be canonical already). +// +// If we ignore custom objects, no detection of cycles or repeated object +// references is currently required by the StoreReader class, since +// GraphQL result objects are JSON-serializable trees (and thus contain +// neither cycles nor repeated subtrees), so we can avoid the complexity +// of keeping track of objects we've already seen during the recursion of +// the admit method. +// +// In the future, we may consider adding additional cases to the switch // statement to handle other common object types, such as "[object Date]" // objects, as needed. export class Canon { - // All known objects this Canon has admitted. + // Set of all canonical objects this Canon has admitted, allowing + // canon.admit to return previously-canonicalized objects immediately. private known = new (canUseWeakMap ? WeakSet : Set)(); - // Efficient storage/lookup structure for admitting objects. + // Efficient storage/lookup structure for canonical objects. private pool = new KeyTrie<{ array?: any[]; object?: Record; @@ -61,6 +98,9 @@ export class Canon { const node = this.pool.lookupArray(array); if (!node.array) { this.known.add(node.array = array); + // Since canonical arrays may be shared widely between + // unrelated consumers, it's important to regard them as + // immutable, even if they are not frozen in production. if (process.env.NODE_ENV !== "production") { Object.freeze(array); } @@ -92,6 +132,9 @@ export class Canon { keys.sorted.forEach((key, i) => { obj[key] = array[i + 2]; }); + // Since canonical objects may be shared widely between + // unrelated consumers, it's important to regard them as + // immutable, even if they are not frozen in production. if (process.env.NODE_ENV !== "production") { Object.freeze(obj); } From 9c6a09cfcea64abd69daae82a4fb8740e6d21f5b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 18:23:39 -0500 Subject: [PATCH 07/12] Rename Canon class to ObjectCanon. --- src/cache/inmemory/{canon.ts => object-canon.ts} | 6 +++--- src/cache/inmemory/readFromStore.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) rename src/cache/inmemory/{canon.ts => object-canon.ts} (97%) diff --git a/src/cache/inmemory/canon.ts b/src/cache/inmemory/object-canon.ts similarity index 97% rename from src/cache/inmemory/canon.ts rename to src/cache/inmemory/object-canon.ts index 7572c4fd4fa..2dc82210a7a 100644 --- a/src/cache/inmemory/canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -23,7 +23,7 @@ class Pass { // which usually means the objects are structurally equivalent (deeply // equal), but don't necessarily use the same memory. // -// Like a literary or musical canon, this Canon class represents a +// Like a literary or musical canon, this ObjectCanon class represents a // collection of unique canonical items (JavaScript objects), with the // important property that canon.admit(a) === canon.admit(b) if a and b // are deeply equal to each other. In terms of the definition above, the @@ -61,8 +61,8 @@ class Pass { // In the future, we may consider adding additional cases to the switch // statement to handle other common object types, such as "[object Date]" // objects, as needed. -export class Canon { - // Set of all canonical objects this Canon has admitted, allowing +export class ObjectCanon { + // Set of all canonical objects this ObjectCanon has admitted, allowing // canon.admit to return previously-canonicalized objects immediately. private known = new (canUseWeakMap ? WeakSet : Set)(); diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 5414ce26de0..7d487bb890c 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -35,7 +35,7 @@ import { getTypenameFromStoreObject } from './helpers'; import { Policies } from './policies'; import { InMemoryCache } from './inMemoryCache'; import { MissingFieldError } from '../core/types/common'; -import { Canon } from './canon'; +import { ObjectCanon } from './object-canon'; export type VariableMap = { [name: string]: any }; @@ -326,7 +326,7 @@ export class StoreReader { return finalResult; } - private canon = new Canon; + private canon = new ObjectCanon; private knownResults = new WeakMap, SelectionSetNode>(); From e8f3ef893c6904594abf399e0ddcd81980c5101c Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Dec 2020 19:53:08 -0500 Subject: [PATCH 08/12] Avoid wrapping non-object values with Pass wrappers. --- src/cache/inmemory/object-canon.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index 2dc82210a7a..4e87018ffd0 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -6,6 +6,10 @@ class Pass { constructor(public readonly value: T) {} } +function isObjectOrArray(value: any): boolean { + return !!value && typeof value === "object"; +} + // When programmers talk about the "canonical form" of an object, they // usually have the following meaning in mind, which I've copied from // https://en.wiktionary.org/wiki/canonical_form: @@ -75,15 +79,17 @@ export class ObjectCanon { // Make the ObjectCanon assume this value has already been // canonicalized. - public pass(value: T): Pass; + public pass(value: T): T extends object ? Pass : T; public pass(value: any) { - return new Pass(value); + return isObjectOrArray(value) ? new Pass(value) : value; } // Returns the canonical version of value. public admit(value: T): T; public admit(value: any) { - if (value && typeof value === "object") { + if (isObjectOrArray(value)) { + // If value is a Pass object returned by canon.pass, unwrap it + // as-is, without canonicalizing its value. if (value instanceof Pass) { return value.value; } From 9fe217378f392ae092bab7976951e9b2d038e834 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 16 Dec 2020 13:15:59 -0500 Subject: [PATCH 09/12] Update optimism and import Trie from @wry/trie instead. As suggested by @hwillson in this comment: https://github.com/apollographql/apollo-client/pull/7439#discussion_r544250587 --- package-lock.json | 24 ++++++++++++++++++++---- package.json | 3 ++- src/cache/inmemory/entityStore.ts | 7 ++++--- src/cache/inmemory/object-canon.ts | 14 +++++++------- src/cache/inmemory/policies.ts | 4 ++-- 5 files changed, 35 insertions(+), 17 deletions(-) diff --git a/package-lock.json b/package-lock.json index 14d2a6fb2fd..e15b92eda36 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2775,6 +2775,21 @@ "tslib": "^1.9.3" } }, + "@wry/trie": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/@wry/trie/-/trie-0.2.1.tgz", + "integrity": "sha512-sYkuXZqArky2MLQCv4tLW6hX3N8AfTZ5ZMBc8jC6Yy35WYr82UYLLtjS7k/uRGHOA0yTSjuNadG6QQ6a5CS5hQ==", + "requires": { + "tslib": "^1.14.1" + }, + "dependencies": { + "tslib": { + "version": "1.14.1", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", + "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==" + } + } + }, "abab": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/abab/-/abab-2.0.5.tgz", @@ -8720,11 +8735,12 @@ } }, "optimism": { - "version": "0.13.1", - "resolved": "https://registry.npmjs.org/optimism/-/optimism-0.13.1.tgz", - "integrity": "sha512-16RRVYZe8ODcUqpabpY7Gb91vCAbdhn8FHjlUb2Hqnjjow1j8Z1dlppds+yAsLbreNTVylLC+tNX6DuC2vt3Kw==", + "version": "0.14.0", + "resolved": "https://registry.npmjs.org/optimism/-/optimism-0.14.0.tgz", + "integrity": "sha512-ygbNt8n4DOCVpkwiLF+IrKKeNHOjtr9aXLWGP9HNJGoblSGsnVbJLstcH6/nE9Xy5ZQtlkSioFQNnthmENW6FQ==", "requires": { - "@wry/context": "^0.5.2" + "@wry/context": "^0.5.2", + "@wry/trie": "^0.2.1" } }, "optionator": { diff --git a/package.json b/package.json index 610e333294f..38f1a8225c4 100644 --- a/package.json +++ b/package.json @@ -77,10 +77,11 @@ "@types/zen-observable": "^0.8.0", "@wry/context": "^0.5.2", "@wry/equality": "^0.3.0", + "@wry/trie": "^0.2.1", "fast-json-stable-stringify": "^2.0.0", "graphql-tag": "^2.11.0", "hoist-non-react-statics": "^3.3.2", - "optimism": "^0.13.1", + "optimism": "^0.14.0", "prop-types": "^15.7.2", "symbol-observable": "^2.0.0", "ts-invariant": "^0.6.0", diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index a69b6ec599b..26ca0b4b73d 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -1,5 +1,6 @@ -import { dep, OptimisticDependencyFunction, KeyTrie } from 'optimism'; +import { dep, OptimisticDependencyFunction } from 'optimism'; import { equal } from '@wry/equality'; +import { Trie } from '@wry/trie'; import { isReference, @@ -496,7 +497,7 @@ class CacheGroup { // Used by the EntityStore#makeCacheKey method to compute cache keys // specific to this CacheGroup. - public readonly keyMaker = new KeyTrie(canUseWeakMap); + public readonly keyMaker = new Trie(canUseWeakMap); } function makeDepKey(dataId: string, storeFieldName: string) { @@ -543,7 +544,7 @@ export namespace EntityStore { return this; } - public readonly storageTrie = new KeyTrie(canUseWeakMap); + public readonly storageTrie = new Trie(canUseWeakMap); public getStorage(): StorageType { return this.storageTrie.lookupArray(arguments); } diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index 4e87018ffd0..b35a2ba03c1 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -1,4 +1,4 @@ -import { KeyTrie } from "optimism"; +import { Trie } from "@wry/trie"; import { canUseWeakMap } from "../../utilities"; import { objToStr } from "./helpers"; @@ -71,7 +71,7 @@ export class ObjectCanon { private known = new (canUseWeakMap ? WeakSet : Set)(); // Efficient storage/lookup structure for canonical objects. - private pool = new KeyTrie<{ + private pool = new Trie<{ array?: any[]; object?: Record; keys?: SortedKeysInfo; @@ -98,7 +98,7 @@ export class ObjectCanon { case "[object Array]": { if (this.known.has(value)) return value; const array: any[] = value.map(this.admit, this); - // Arrays are looked up in the KeyTrie using their recursively + // Arrays are looked up in the Trie using their recursively // canonicalized elements, and the known version of the array is // preserved as node.array. const node = this.pool.lookupArray(array); @@ -123,9 +123,9 @@ export class ObjectCanon { keys.sorted.forEach(key => { array.push(this.admit(value[key])); }); - // Objects are looked up in the KeyTrie by their prototype - // (which is *not* recursively canonicalized), followed by a - // JSON representation of their (sorted) keys, followed by the + // Objects are looked up in the Trie by their prototype (which + // is *not* recursively canonicalized), followed by a JSON + // representation of their (sorted) keys, followed by the // sequence of recursively canonicalized values corresponding to // those keys. To keep the final results unambiguous with other // sequences (such as arrays that just happen to contain [proto, @@ -154,7 +154,7 @@ export class ObjectCanon { // It's worthwhile to cache the sorting of arrays of strings, since the // same initial unsorted arrays tend to be encountered many times. - // Fortunately, we can reuse the KeyTrie machinery to look up the sorted + // Fortunately, we can reuse the Trie machinery to look up the sorted // arrays in linear time (which is faster than sorting large arrays). private sortedKeys(obj: object) { const keys = Object.keys(obj); diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 58db0bdf96f..36dcb94caac 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -5,7 +5,7 @@ import { FieldNode, } from 'graphql'; -import { KeyTrie } from 'optimism'; +import { Trie } from '@wry/trie'; import { invariant, InvariantError } from 'ts-invariant'; import { @@ -915,7 +915,7 @@ function keyArgsFnFromSpecifier( function keyFieldsFnFromSpecifier( specifier: KeySpecifier, ): KeyFieldsFunction { - const trie = new KeyTrie<{ + const trie = new Trie<{ aliasMap?: AliasMap; }>(canUseWeakMap); From bdf34915d1de92073e5b58fb41d2ddacc380ab2f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 16 Dec 2020 13:42:10 -0500 Subject: [PATCH 10/12] Use firstValueIndex variable to make array indexing clearer. https://github.com/apollographql/apollo-client/pull/7439#discussion_r544246303 --- src/cache/inmemory/object-canon.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index b35a2ba03c1..599ad340f70 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -120,6 +120,7 @@ export class ObjectCanon { const array = [proto]; const keys = this.sortedKeys(value); array.push(keys.json); + const firstValueIndex = array.length; keys.sorted.forEach(key => { array.push(this.admit(value[key])); }); @@ -136,7 +137,7 @@ export class ObjectCanon { const obj = node.object = Object.create(proto); this.known.add(obj); keys.sorted.forEach((key, i) => { - obj[key] = array[i + 2]; + obj[key] = array[firstValueIndex + i]; }); // Since canonical objects may be shared widely between // unrelated consumers, it's important to regard them as From d6c58268feee5fed38eca1315d49081c0ee0a875 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 16 Dec 2020 14:05:31 -0500 Subject: [PATCH 11/12] Add more direct unit tests of ObjectCanon. https://github.com/apollographql/apollo-client/pull/7439#pullrequestreview-553616923 --- src/cache/inmemory/__tests__/object-canon.ts | 108 +++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 src/cache/inmemory/__tests__/object-canon.ts diff --git a/src/cache/inmemory/__tests__/object-canon.ts b/src/cache/inmemory/__tests__/object-canon.ts new file mode 100644 index 00000000000..d109d501d54 --- /dev/null +++ b/src/cache/inmemory/__tests__/object-canon.ts @@ -0,0 +1,108 @@ +import { ObjectCanon } from "../object-canon"; + +describe("ObjectCanon", () => { + it("can canonicalize objects and arrays", () => { + const canon = new ObjectCanon; + + const obj1 = { + a: [1, 2], + b: { + c: [{ + d: "dee", + e: "ee", + }, "f"], + g: "gee", + }, + }; + + const obj2 = { + b: { + g: "gee", + c: [{ + e: "ee", + d: "dee", + }, "f"], + }, + a: [1, 2], + }; + + expect(obj1).toEqual(obj2); + expect(obj1).not.toBe(obj2); + + const c1 = canon.admit(obj1); + const c2 = canon.admit(obj2); + + expect(c1).toBe(c2); + expect(c1).toEqual(obj1); + expect(c1).toEqual(obj2); + expect(c2).toEqual(obj1); + expect(c2).toEqual(obj2); + expect(c1).not.toBe(obj1); + expect(c1).not.toBe(obj2); + expect(c2).not.toBe(obj1); + expect(c2).not.toBe(obj2); + + expect(canon.admit(c1)).toBe(c1); + expect(canon.admit(c2)).toBe(c2); + }); + + it("preserves custom prototypes", () => { + const canon = new ObjectCanon; + + class Custom { + constructor(public value: any) {} + getValue() { return this.value } + } + + const customs = [ + new Custom("oyez"), + new Custom(1234), + new Custom(true), + ]; + + const admitted = canon.admit(customs); + expect(admitted).not.toBe(customs); + expect(admitted).toEqual(customs); + + function check(i: number) { + expect(admitted[i]).toEqual(customs[i]); + expect(admitted[i]).not.toBe(customs[i]); + expect(admitted[i].getValue()).toBe(customs[i].getValue()); + expect(Object.getPrototypeOf(admitted[i])).toBe(Custom.prototype); + expect(admitted[i]).toBeInstanceOf(Custom); + } + check(0); + check(1); + check(2); + + expect(canon.admit(customs)).toBe(admitted); + }); + + it("unwraps Pass wrappers as-is", () => { + const canon = new ObjectCanon; + + const cd = { + c: "see", + d: "dee", + }; + + const obj = { + a: cd, + b: canon.pass(cd), + e: cd, + }; + + function check() { + const admitted = canon.admit(obj); + expect(admitted).not.toBe(obj); + expect(admitted.b).toBe(cd); + expect(admitted.e).toEqual(cd); + expect(admitted.e).not.toBe(cd); + expect(admitted.e).toEqual(admitted.b); + expect(admitted.e).not.toBe(admitted.b); + expect(admitted.e).toBe(admitted.a); + } + check(); + check(); + }); +}); From 48bd0b1d0568f143eb9c8a29e191c9458f25c2b8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 16 Dec 2020 14:16:46 -0500 Subject: [PATCH 12/12] Mention PR #7439 in CHANGELOG.md. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bd817dc3b0..8d108429723 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ TBD ### Improvements +- `InMemoryCache` now _guarantees_ that any two result objects returned by the cache (from `readQuery`, `readFragment`, etc.) will be referentially equal (`===`) if they are deeply equal. Previously, `===` equality was often achievable for results for the same query, on a best-effort basis. Now, equivalent result objects will be automatically shared among the result trees of completely different queries. This guarantee is important for taking full advantage of optimistic updates that correctly guess the final data, and for "pure" UI components that can skip re-rendering when their input data are unchanged.
+ [@benjamn](https://github.com/benjamn) in [#7439](https://github.com/apollographql/apollo-client/pull/7439) + - Support `client.refetchQueries` as an imperative way to refetch queries, without having to pass `options.refetchQueries` to `client.mutate`.
[@dannycochran](https://github.com/dannycochran) in [#7431](https://github.com/apollographql/apollo-client/pull/7431)