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) 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 e9d5ba125e9..38f1a8225c4 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": { @@ -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/__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__/__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__/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__/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(); + }); +}); 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/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index d46c79b89ca..bec47f710eb 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,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(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(); + }); + + 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); + }); }); 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/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/object-canon.ts b/src/cache/inmemory/object-canon.ts new file mode 100644 index 00000000000..599ad340f70 --- /dev/null +++ b/src/cache/inmemory/object-canon.ts @@ -0,0 +1,180 @@ +import { Trie } from "@wry/trie"; +import { canUseWeakMap } from "../../utilities"; +import { objToStr } from "./helpers"; + +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: +// +// 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 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 +// 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 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)(); + + // Efficient storage/lookup structure for canonical objects. + private pool = new Trie<{ + array?: any[]; + object?: Record; + keys?: SortedKeysInfo; + }>(canUseWeakMap); + + // Make the ObjectCanon assume this value has already been + // canonicalized. + public pass(value: T): T extends object ? Pass : T; + public pass(value: any) { + return isObjectOrArray(value) ? new Pass(value) : value; + } + + // Returns the canonical version of value. + public admit(value: T): T; + public admit(value: any) { + 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; + } + + 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 Trie 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); + // 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); + } + } + 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); + const firstValueIndex = array.length; + keys.sorted.forEach(key => { + array.push(this.admit(value[key])); + }); + // 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, + // 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[firstValueIndex + i]; + }); + // 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); + } + } + 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 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); + 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/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); diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 1abafb40f30..7d487bb890c 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'; @@ -36,6 +35,7 @@ import { getTypenameFromStoreObject } from './helpers'; import { Policies } from './policies'; import { InMemoryCache } from './inMemoryCache'; import { MissingFieldError } from '../core/types/common'; +import { ObjectCanon } from './object-canon'; export type VariableMap = { [name: string]: any }; @@ -279,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, @@ -324,11 +317,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 +326,8 @@ export class StoreReader { return finalResult; } + private canon = new ObjectCanon; + private knownResults = new WeakMap, SelectionSetNode>(); // Cached version of execSubSelectedArrayImpl. @@ -377,7 +368,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 +401,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: