From 897e36eeb225ef530fd514f000b35be770e08427 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 14 May 2021 13:32:39 -0400 Subject: [PATCH 1/8] Remove dependency on fast-json-stable-stringify. > Note: I am expecting tests to fail for this commit, demonstrating the importance of using a stable serialization strategy for field arguments. Although fast-json-stable-stringify has done its job well, it only provides a "main" field in its package.json file, pointing to a CommonJS entry point, and does not appear to export any ECMAScript modules. Thanks to our conversion/abandonment of fast-json-stable-stringify and other CommonJS-only npm dependencies (zen-observable in #5961 and graphql-tag in #6074), it should now (after this PR is merged) be possible to load @apollo/client/core from an ESM-aware CDN like jsdelivr.net or jspm.io: If you put that script tag in an HTML file, or inject it into the DOM of any webpage, you will currently see this error: Uncaught SyntaxError: The requested module '/npm/fast-json-stable-stringify@2.1.0/+esm' does not provide an export named 'default' This list of errors used to be longer, but now the only package left is fast-json-stable-stringify. Note that we're loading @apollo/client/core@beta here, not @apollo/client@beta. The reason @apollo/client itself is not yet ESM-ready is that react and react-dom are the two remaining CommonJS-only dependencies, and @apollo/client currently/regrettably re-exports utilities from @apollo/client/react. If importing from @apollo/client/core is a burden or feels weird to you, please know that we are planning to make @apollo/client synonymous with @apollo/client/core in Apollo Client 4.0, along with making @apollo/client/react synonymous with the v3 API of @apollo/client, though of course those will be major breaking changes: https://github.com/apollographql/apollo-client/issues/8190 --- package-lock.json | 3 ++- package.json | 1 - src/utilities/graphql/storeUtils.ts | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index d1f9f3a2a62..11b7c112b95 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6654,7 +6654,8 @@ "fast-json-stable-stringify": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz", - "integrity": "sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==" + "integrity": "sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==", + "dev": true }, "fast-levenshtein": { "version": "2.0.6", diff --git a/package.json b/package.json index 29da62edc3e..8b1fc04951d 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "@wry/context": "^0.6.0", "@wry/equality": "^0.4.0", "@wry/trie": "^0.3.0", - "fast-json-stable-stringify": "^2.0.0", "graphql-tag": "^2.12.3", "hoist-non-react-statics": "^3.3.2", "optimism": "^0.16.1", diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 31c9513fb8d..6ef388baf03 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -17,7 +17,6 @@ import { SelectionSetNode, } from 'graphql'; -import stringify from 'fast-json-stable-stringify'; import { InvariantError } from 'ts-invariant'; import { FragmentMap, getFragmentFromSelection } from './fragments'; @@ -181,6 +180,7 @@ export function getStoreKeyName( fieldName: string, args?: Record | null, directives?: Directives, + stringify: (value: any) => string = JSON.stringify, ): string { if ( args && @@ -202,7 +202,7 @@ export function getStoreKeyName( filteredArgs[key] = args[key]; }); - return `${directives['connection']['key']}(${JSON.stringify( + return `${directives['connection']['key']}(${stringify( filteredArgs, )})`; } else { @@ -224,7 +224,7 @@ export function getStoreKeyName( Object.keys(directives).forEach(key => { if (KNOWN_DIRECTIVES.indexOf(key) !== -1) return; if (directives[key] && Object.keys(directives[key]).length) { - completeFieldName += `@${key}(${JSON.stringify(directives[key])})`; + completeFieldName += `@${key}(${stringify(directives[key])})`; } else { completeFieldName += `@${key}`; } From c82e9395d597d67f68d5a101a81ef67702f8e340 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 14 May 2021 17:05:39 -0400 Subject: [PATCH 2/8] Simple but slow(er) replacement for fast-json-stable-stringify. This should fix the failing tests, though I'm planning to replace this default implementation with a more clever one in the next commit. --- src/utilities/graphql/storeUtils.ts | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 6ef388baf03..26937a34139 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -180,7 +180,6 @@ export function getStoreKeyName( fieldName: string, args?: Record | null, directives?: Directives, - stringify: (value: any) => string = JSON.stringify, ): string { if ( args && @@ -234,6 +233,28 @@ export function getStoreKeyName( return completeFieldName; } +export namespace getStoreKeyName { + export function setStringify(s: typeof stringify) { + const previous = stringify; + stringify = s; + return previous; + } +} + +let stringify = function defaultStringify(value: any): string { + return JSON.stringify(value, stringifyReplacer); +}; + +function stringifyReplacer(_key: string, value: any): any { + if (value && typeof value === "object" && !Array.isArray(value)) { + value = Object.keys(value).sort().reduce((copy, key) => { + copy[key] = value[key]; + return copy; + }, Object.create(Object.getPrototypeOf(value))); + } + return value; +} + export function argumentsObjectFromField( field: FieldNode | DirectiveNode, variables?: Record, From 842eb5e2b820f5d0aec4746c229acaeb69306d1b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 14 May 2021 17:21:52 -0400 Subject: [PATCH 3/8] Use ObjectCanon to speed up stable stringification. This reimplementation of the stable `stringify` function used by `getStoreKeyName` builds on the `ObjectCanon` introduced by my PR #7439, which guarantees canonical objects keys are always in sorted order. --- src/cache/inmemory/object-canon.ts | 27 ++++++++++++++++++++++++--- src/cache/inmemory/policies.ts | 6 ++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index 0be454bc0d7..84c7c008cd6 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -2,7 +2,7 @@ import { Trie } from "@wry/trie"; import { canUseWeakMap } from "../../utilities"; import { objToStr } from "./helpers"; -function isObjectOrArray(value: any): boolean { +function isObjectOrArray(value: any): value is object { return !!value && typeof value === "object"; } @@ -109,7 +109,7 @@ export class ObjectCanon { switch (objToStr.call(value)) { case "[object Array]": { if (this.known.has(value)) return value; - const array: any[] = value.map(this.admit, this); + const array: any[] = (value as any[]).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. @@ -134,7 +134,7 @@ export class ObjectCanon { array.push(keys.json); const firstValueIndex = array.length; keys.sorted.forEach(key => { - array.push(this.admit(value[key])); + array.push(this.admit((value as any)[key])); }); // Objects are looked up in the Trie by their prototype (which // is *not* recursively canonicalized), followed by a JSON @@ -193,3 +193,24 @@ type SortedKeysInfo = { sorted: string[]; json: string; }; + +const stringifyCanon = new ObjectCanon; +const stringifyCache = new WeakMap(); + +// Since the keys of canonical objects are always created in lexicographically +// sorted order, we can use the ObjectCanon to implement a fast and stable +// version of JSON.stringify, which automatically sorts object keys. +export function canonicalStringify(value: any): string { + if (isObjectOrArray(value)) { + const canonical = stringifyCanon.admit(value); + let json = stringifyCache.get(canonical); + if (json === void 0) { + stringifyCache.set( + canonical, + json = JSON.stringify(canonical), + ); + } + return json; + } + return JSON.stringify(value); +} diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index f0a92829522..749b8af893f 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -48,6 +48,12 @@ import { } from '../core/types/common'; import { WriteContext } from './writeToStore'; +// Upgrade to a faster version of the default stable JSON.stringify function +// used by getStoreKeyName. This function is used when computing storeFieldName +// strings (when no keyArgs has been configured for a field). +import { canonicalStringify } from './object-canon'; +getStoreKeyName.setStringify(canonicalStringify); + export type TypePolicies = { [__typename: string]: TypePolicy; } From ac74d8e7729b6532ca7922b86e8c5eececf38ec4 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 17 May 2021 15:05:00 -0400 Subject: [PATCH 4/8] Allow resetting stringifyCanon used by canonicalStringify. This reset will happen whenever cache.gc() is called, which makes sense because it frees all memory associated with the replaced stringifyCanon, at the cost of temporarily slowing down canonicalStringify (but without any logical changes in the behavior/output of canonicalStringify). --- src/cache/inmemory/inMemoryCache.ts | 3 +++ src/cache/inmemory/object-canon.ts | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index fe7eb284bdd..896847bb6d6 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -28,6 +28,7 @@ import { TypePolicies, } from './policies'; import { hasOwn } from './helpers'; +import { resetStringifyCanon } from './object-canon'; export interface InMemoryCacheConfig extends ApolloReducerConfig { resultCaching?: boolean; @@ -262,6 +263,7 @@ export class InMemoryCache extends ApolloCache { // Request garbage collection of unreachable normalized entities. public gc() { + resetStringifyCanon(); return this.optimisticData.gc(); } @@ -322,6 +324,7 @@ export class InMemoryCache extends ApolloCache { public reset(): Promise { this.init(); this.broadcastWatches(); + resetStringifyCanon(); return Promise.resolve(); } diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index 84c7c008cd6..a933b36a280 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -194,7 +194,10 @@ type SortedKeysInfo = { json: string; }; -const stringifyCanon = new ObjectCanon; +let stringifyCanon = new ObjectCanon; +export function resetStringifyCanon() { + stringifyCanon = new ObjectCanon; +} const stringifyCache = new WeakMap(); // Since the keys of canonical objects are always created in lexicographically From 5a17ec1f1724ec436b5107ed9112e830f4c3ff80 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 14 May 2021 18:34:17 -0400 Subject: [PATCH 5/8] Don't bother preserving prototypes of objects to be JSON-serialized. https://github.com/apollographql/apollo-client/pull/8222#discussion_r632827175 --- src/utilities/graphql/storeUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 26937a34139..6d6729d2d23 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -250,7 +250,7 @@ function stringifyReplacer(_key: string, value: any): any { value = Object.keys(value).sort().reduce((copy, key) => { copy[key] = value[key]; return copy; - }, Object.create(Object.getPrototypeOf(value))); + }, {} as Record); } return value; } From ef2d3ad4cf4ea2e64e57dd26f5c803f1013eba02 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 17 May 2021 17:42:54 -0400 Subject: [PATCH 6/8] Support canonicalStringify.reset() instead of a separate function. --- src/cache/inmemory/inMemoryCache.ts | 6 +++--- src/cache/inmemory/object-canon.ts | 20 ++++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 896847bb6d6..0f059478c0d 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -28,7 +28,7 @@ import { TypePolicies, } from './policies'; import { hasOwn } from './helpers'; -import { resetStringifyCanon } from './object-canon'; +import { canonicalStringify } from './object-canon'; export interface InMemoryCacheConfig extends ApolloReducerConfig { resultCaching?: boolean; @@ -263,7 +263,7 @@ export class InMemoryCache extends ApolloCache { // Request garbage collection of unreachable normalized entities. public gc() { - resetStringifyCanon(); + canonicalStringify.reset(); return this.optimisticData.gc(); } @@ -324,7 +324,7 @@ export class InMemoryCache extends ApolloCache { public reset(): Promise { this.init(); this.broadcastWatches(); - resetStringifyCanon(); + canonicalStringify.reset(); return Promise.resolve(); } diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index a933b36a280..d9a2747e2a7 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -194,16 +194,10 @@ type SortedKeysInfo = { json: string; }; -let stringifyCanon = new ObjectCanon; -export function resetStringifyCanon() { - stringifyCanon = new ObjectCanon; -} -const stringifyCache = new WeakMap(); - // Since the keys of canonical objects are always created in lexicographically // sorted order, we can use the ObjectCanon to implement a fast and stable // version of JSON.stringify, which automatically sorts object keys. -export function canonicalStringify(value: any): string { +export const canonicalStringify = Object.assign(function (value: any): string { if (isObjectOrArray(value)) { const canonical = stringifyCanon.admit(value); let json = stringifyCache.get(canonical); @@ -216,4 +210,14 @@ export function canonicalStringify(value: any): string { return json; } return JSON.stringify(value); -} +}, { + reset() { + stringifyCanon = new ObjectCanon; + }, +}); + +// Can be reset by calling canonicalStringify.reset(). +let stringifyCanon = new ObjectCanon; + +// Needs no resetting, thanks to weakness. +const stringifyCache = new WeakMap(); From a2d679ed058289a560c5e0901cb22fc25f00c954 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 17 May 2021 17:43:56 -0400 Subject: [PATCH 7/8] Use Object.assign instead of namespace for getStoreKeyName.setStringify. https://github.com/apollographql/apollo-client/pull/8222#discussion_r633847490 --- src/utilities/graphql/storeUtils.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 6d6729d2d23..70e0bc0eb5b 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -176,7 +176,7 @@ const KNOWN_DIRECTIVES: string[] = [ 'export', ]; -export function getStoreKeyName( +export const getStoreKeyName = Object.assign(function ( fieldName: string, args?: Record | null, directives?: Directives, @@ -231,16 +231,16 @@ export function getStoreKeyName( } return completeFieldName; -} - -export namespace getStoreKeyName { - export function setStringify(s: typeof stringify) { +}, { + setStringify(s: typeof stringify) { const previous = stringify; stringify = s; return previous; - } -} + }, +}); +// Default stable JSON.stringify implementation. Can be updated/replaced with +// something better by calling getStoreKeyName.setStringify. let stringify = function defaultStringify(value: any): string { return JSON.stringify(value, stringifyReplacer); }; From cd202fc307de804d721bc2e8a09df4bcf2c16c7f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 17 May 2021 17:45:50 -0400 Subject: [PATCH 8/8] Export canonicalStringify from @apollo/client/cache. https://github.com/apollographql/apollo-client/pull/8222#discussion_r633851693 --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/cache/index.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index d41a41b752a..cf064e3d32a 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -68,6 +68,7 @@ Array [ "MissingFieldError", "Policies", "cacheSlot", + "canonicalStringify", "defaultDataIdFromObject", "fieldNameFromStoreName", "isReference", diff --git a/src/cache/index.ts b/src/cache/index.ts index d57d526ef8b..5e3f221a44f 100644 --- a/src/cache/index.ts +++ b/src/cache/index.ts @@ -38,4 +38,8 @@ export { Policies, } from './inmemory/policies'; +export { + canonicalStringify, +} from './inmemory/object-canon'; + export * from './inmemory/types';