From ff4bb9f4b807de4e46434bc4ecbae88039f5ec6c Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 12:31:59 -0400 Subject: [PATCH 01/18] Move stub implementation of canonicalStringify to utilities/common. --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/cache/index.ts | 8 +++-- src/cache/inmemory/__tests__/key-extractor.ts | 2 +- src/cache/inmemory/inMemoryCache.ts | 2 +- src/cache/inmemory/object-canon.ts | 32 ------------------- src/cache/inmemory/policies.ts | 5 +-- src/cache/inmemory/readFromStore.ts | 3 +- src/cache/inmemory/writeToStore.ts | 2 +- src/utilities/common/canonicalStringify.ts | 11 +++++++ src/utilities/index.ts | 1 + 10 files changed, 24 insertions(+), 43 deletions(-) create mode 100644 src/utilities/common/canonicalStringify.ts diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index 6c06e405fd9..1c48ea242e3 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -395,6 +395,7 @@ Array [ "canUseSymbol", "canUseWeakMap", "canUseWeakSet", + "canonicalStringify", "checkDocument", "cloneDeep", "compact", diff --git a/src/cache/index.ts b/src/cache/index.ts index bed4ad849fd..d57341ff2ac 100644 --- a/src/cache/index.ts +++ b/src/cache/index.ts @@ -14,7 +14,11 @@ export type { export { MissingFieldError } from "./core/types/common.js"; export type { Reference } from "../utilities/index.js"; -export { isReference, makeReference } from "../utilities/index.js"; +export { + isReference, + makeReference, + canonicalStringify, +} from "../utilities/index.js"; export { EntityStore } from "./inmemory/entityStore.js"; export { @@ -38,8 +42,6 @@ export type { } from "./inmemory/policies.js"; export { Policies } from "./inmemory/policies.js"; -export { canonicalStringify } from "./inmemory/object-canon.js"; - export type { FragmentRegistryAPI } from "./inmemory/fragmentRegistry.js"; export { createFragmentRegistry } from "./inmemory/fragmentRegistry.js"; diff --git a/src/cache/inmemory/__tests__/key-extractor.ts b/src/cache/inmemory/__tests__/key-extractor.ts index 3636f6490e6..d525263d010 100644 --- a/src/cache/inmemory/__tests__/key-extractor.ts +++ b/src/cache/inmemory/__tests__/key-extractor.ts @@ -1,5 +1,5 @@ import { KeySpecifier } from "../policies"; -import { canonicalStringify } from "../object-canon"; +import { canonicalStringify } from "../../../utilities"; import { getSpecifierPaths, collectSpecifierPaths, diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 2dac460bf54..3fd230b94e0 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -16,6 +16,7 @@ import { addTypenameToDocument, isReference, DocumentTransform, + canonicalStringify, } from "../../utilities/index.js"; import type { InMemoryCacheConfig, NormalizedCacheObject } from "./types.js"; import { StoreReader } from "./readFromStore.js"; @@ -24,7 +25,6 @@ import { EntityStore, supportsResultCaching } from "./entityStore.js"; import { makeVar, forgetCache, recallCache } from "./reactiveVars.js"; import { Policies } from "./policies.js"; import { hasOwn, normalizeConfig, shouldCanonizeResults } from "./helpers.js"; -import { canonicalStringify } from "./object-canon.js"; import type { OperationVariables } from "../../core/index.js"; type BroadcastOptions = Pick< diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index 376b474f282..05067b74d46 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -195,35 +195,3 @@ type SortedKeysInfo = { sorted: string[]; json: string; }; - -// 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 const canonicalStringify = Object.assign( - function (value: any): string { - if (isObjectOrArray(value)) { - if (stringifyCanon === void 0) { - resetCanonicalStringify(); - } - 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); - }, - { - reset: resetCanonicalStringify, - } -); - -// Can be reset by calling canonicalStringify.reset(). -let stringifyCanon: ObjectCanon; -let stringifyCache: WeakMap; - -function resetCanonicalStringify() { - stringifyCanon = new ObjectCanon(); - stringifyCache = new (canUseWeakMap ? WeakMap : Map)(); -} diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 2b2caec7e9c..d44c896cc83 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -20,6 +20,7 @@ import { getStoreKeyName, isNonNullObject, stringifyForDisplay, + canonicalStringify, } from "../../utilities/index.js"; import type { IdGetter, @@ -48,10 +49,6 @@ import type { } from "../core/types/common.js"; import type { WriteContext } from "./writeToStore.js"; -// 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.js"; import { keyArgsFnFromSpecifier, keyFieldsFnFromSpecifier, diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 9a9ddc1498c..0643233b947 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -28,6 +28,7 @@ import { isNonNullObject, canUseWeakMap, compact, + canonicalStringify, } from "../../utilities/index.js"; import type { Cache } from "../core/types/Cache.js"; import type { @@ -50,7 +51,7 @@ import type { Policies } from "./policies.js"; import type { InMemoryCache } from "./inMemoryCache.js"; import type { MissingTree } from "../core/types/common.js"; import { MissingFieldError } from "../core/types/common.js"; -import { canonicalStringify, ObjectCanon } from "./object-canon.js"; +import { ObjectCanon } from "./object-canon.js"; export type VariableMap = { [name: string]: any }; diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 1d3f3fb01d4..95057861b2f 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -25,6 +25,7 @@ import { addTypenameToDocument, isNonEmptyArray, argumentsObjectFromField, + canonicalStringify, } from "../../utilities/index.js"; import type { @@ -44,7 +45,6 @@ import type { StoreReader } from "./readFromStore.js"; import type { InMemoryCache } from "./inMemoryCache.js"; import type { EntityStore } from "./entityStore.js"; import type { Cache } from "../../core/index.js"; -import { canonicalStringify } from "./object-canon.js"; import { normalizeReadFieldOptions } from "./policies.js"; import type { ReadFieldFunction } from "../core/types/common.js"; diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts new file mode 100644 index 00000000000..96ed40eca08 --- /dev/null +++ b/src/utilities/common/canonicalStringify.ts @@ -0,0 +1,11 @@ +export const canonicalStringify = Object.assign( + function canonicalStringify(value: any): string { + // TODO + return JSON.stringify(value); + }, + { + reset() { + // TODO + }, + } +); diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 6462f639fea..adbd5e26c1e 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -120,6 +120,7 @@ export * from "./common/makeUniqueId.js"; export * from "./common/stringifyForDisplay.js"; export * from "./common/mergeOptions.js"; export * from "./common/incrementalResult.js"; +export * from "./common/canonicalStringify.js"; export { omitDeep } from "./common/omitDeep.js"; export { stripTypename } from "./common/stripTypename.js"; From 729b48e2735c76c78f6715e6e1940f2bb7bf0a11 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 12:52:15 -0400 Subject: [PATCH 02/18] Trivial implementation of canonicalStringify that makes tests pass. --- src/utilities/common/canonicalStringify.ts | 33 ++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 96ed40eca08..7791a899874 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -1,7 +1,7 @@ +// Like JSON.stringify, but with object keys always sorted in the same order. export const canonicalStringify = Object.assign( function canonicalStringify(value: any): string { - // TODO - return JSON.stringify(value); + return JSON.stringify(value, stableObjectReplacer); }, { reset() { @@ -9,3 +9,32 @@ export const canonicalStringify = Object.assign( }, } ); + +// The JSON.stringify function takes an optional second argument called a +// replacer function. This function is called for each key-value pair in the +// object being stringified, and its return value is used instead of the +// original value. If the replacer function returns a new value, that value is +// stringified as JSON instead of the original value of the property. +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#the_replacer_parameter +function stableObjectReplacer(key: string, value: any) { + if (value && typeof value === "object") { + const proto = Object.getPrototypeOf(value); + // We don't want to mess with objects that are not "plain" objects, which + // means their prototype is either Object.prototype or null. + if (proto === Object.prototype || proto === null) { + const sorted = Object.create(null); + // TODO This sorting step can be sped up in two ways: + // 1. If the keys are already sorted, we can skip the sorting step. + // 2. If we have sorted this sequence of keys before, we can look up the + // previously sorted sequence in linear time rather than O(n log n) + // time, which is the typical cost of sorting. + Object.keys(value) + .sort() + .forEach((key) => { + sorted[key] = value[key]; + }); + return sorted; + } + } + return value; +} From aeaf611b8c6dddcf90e4d42e7b9553d62f95b48d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 12:57:42 -0400 Subject: [PATCH 03/18] Avoid hack of calling getStoreKeyName.setStringify(canonicalStringify). --- src/cache/inmemory/policies.ts | 3 - src/utilities/graphql/storeUtils.ts | 121 +++++++++++----------------- 2 files changed, 47 insertions(+), 77 deletions(-) diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index d44c896cc83..6918c7dd6aa 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -20,7 +20,6 @@ import { getStoreKeyName, isNonNullObject, stringifyForDisplay, - canonicalStringify, } from "../../utilities/index.js"; import type { IdGetter, @@ -54,8 +53,6 @@ import { keyFieldsFnFromSpecifier, } from "./key-extractor.js"; -getStoreKeyName.setStringify(canonicalStringify); - export type TypePolicies = { [__typename: string]: TypePolicy; }; diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 606c3667f00..5624432ebe1 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -24,6 +24,7 @@ import type { import { isNonNullObject } from "../common/objects.js"; import type { FragmentMap } from "./fragments.js"; import { getFragmentFromSelection } from "./fragments.js"; +import { canonicalStringify } from "../common/canonicalStringify.js"; export interface Reference { readonly __ref: string; @@ -194,89 +195,61 @@ const KNOWN_DIRECTIVES: string[] = [ "nonreactive", ]; -export const getStoreKeyName = Object.assign( - function ( - fieldName: string, - args?: Record | null, - directives?: Directives - ): string { +export function getStoreKeyName( + fieldName: string, + args?: Record | null, + directives?: Directives +): string { + if ( + args && + directives && + directives["connection"] && + directives["connection"]["key"] + ) { if ( - args && - directives && - directives["connection"] && - directives["connection"]["key"] + directives["connection"]["filter"] && + (directives["connection"]["filter"] as string[]).length > 0 ) { - if ( - directives["connection"]["filter"] && - (directives["connection"]["filter"] as string[]).length > 0 - ) { - const filterKeys = directives["connection"]["filter"] - ? (directives["connection"]["filter"] as string[]) - : []; - filterKeys.sort(); - - const filteredArgs = {} as { [key: string]: any }; - filterKeys.forEach((key) => { - filteredArgs[key] = args[key]; - }); - - return `${directives["connection"]["key"]}(${stringify(filteredArgs)})`; - } else { - return directives["connection"]["key"]; - } - } - - let completeFieldName: string = fieldName; + const filterKeys = directives["connection"]["filter"] + ? (directives["connection"]["filter"] as string[]) + : []; + filterKeys.sort(); + + const filteredArgs = {} as { [key: string]: any }; + filterKeys.forEach((key) => { + filteredArgs[key] = args[key]; + }); - if (args) { - // We can't use `JSON.stringify` here since it's non-deterministic, - // and can lead to different store key names being created even though - // the `args` object used during creation has the same properties/values. - const stringifiedArgs: string = stringify(args); - completeFieldName += `(${stringifiedArgs})`; + return `${directives["connection"]["key"]}(${canonicalStringify( + filteredArgs + )})`; + } else { + return directives["connection"]["key"]; } + } - if (directives) { - Object.keys(directives).forEach((key) => { - if (KNOWN_DIRECTIVES.indexOf(key) !== -1) return; - if (directives[key] && Object.keys(directives[key]).length) { - completeFieldName += `@${key}(${stringify(directives[key])})`; - } else { - completeFieldName += `@${key}`; - } - }); - } + let completeFieldName: string = fieldName; - return completeFieldName; - }, - { - setStringify(s: typeof stringify) { - const previous = stringify; - stringify = s; - return previous; - }, + if (args) { + // We can't use `JSON.stringify` here since it's non-deterministic, + // and can lead to different store key names being created even though + // the `args` object used during creation has the same properties/values. + const stringifiedArgs: string = canonicalStringify(args); + completeFieldName += `(${stringifiedArgs})`; } -); - -// 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); -}; -function stringifyReplacer(_key: string, value: any): any { - if (isNonNullObject(value) && !Array.isArray(value)) { - value = Object.keys(value) - .sort() - .reduce( - (copy, key) => { - copy[key] = value[key]; - return copy; - }, - {} as Record - ); + if (directives) { + Object.keys(directives).forEach((key) => { + if (KNOWN_DIRECTIVES.indexOf(key) !== -1) return; + if (directives[key] && Object.keys(directives[key]).length) { + completeFieldName += `@${key}(${canonicalStringify(directives[key])})`; + } else { + completeFieldName += `@${key}`; + } + }); } - return value; + + return completeFieldName; } export function argumentsObjectFromField( From 5d57cdaaac29668c4bb9fc60101d307cb6e4e9bb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 13:35:57 -0400 Subject: [PATCH 04/18] Improve canonicalStringify implementation using SortingTrie. --- src/utilities/common/canonicalStringify.ts | 60 ++++++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 7791a899874..6627c827cd1 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -5,7 +5,10 @@ export const canonicalStringify = Object.assign( }, { reset() { - // TODO + // Blowing away the root-level trie map will reclaim all memory stored in + // the trie, without affecting the logical results of canonicalStringify, + // but potentially sacrificing performance until the trie is refilled. + sortingTrieRoot.next = Object.create(null); }, } ); @@ -22,19 +25,54 @@ function stableObjectReplacer(key: string, value: any) { // We don't want to mess with objects that are not "plain" objects, which // means their prototype is either Object.prototype or null. if (proto === Object.prototype || proto === null) { - const sorted = Object.create(null); - // TODO This sorting step can be sped up in two ways: - // 1. If the keys are already sorted, we can skip the sorting step. - // 2. If we have sorted this sequence of keys before, we can look up the - // previously sorted sequence in linear time rather than O(n log n) - // time, which is the typical cost of sorting. - Object.keys(value) - .sort() - .forEach((key) => { + const keys = Object.keys(value); + const sortedKeys = sortKeys(keys); + if (sortedKeys !== keys) { + const sorted = Object.create(null); + // Reassigning the keys in sorted order will cause JSON.stringify to + // serialize them in sorted order. + sortedKeys.forEach((key) => { sorted[key] = value[key]; }); - return sorted; + return sorted; + } } } return value; } + +interface SortingTrie { + sorted?: readonly string[]; + next: Record; +} + +const sortingTrieRoot: SortingTrie = { + sorted: [], + next: Object.create(null), +}; + +// Sort the given keys using a lookup trie, and return the same (===) array in +// case it was already sorted, so we can avoid always creating a new object in +// the replacer function above. +function sortKeys(keys: readonly string[]): readonly string[] { + let node = sortingTrieRoot; + let alreadySorted = true; + for (let k = 0, len = keys.length; k < len; ++k) { + const key = keys[k]; + if (k > 0 && keys[k - 1] > key) { + alreadySorted = false; + } + const next = node.next; + node = next[key] || (next[key] = { next: Object.create(null) }); + } + if (alreadySorted) { + // There may already be a node.sorted array that's equivalent to the + // already-sorted keys array, but if keys was already sorted, we always want + // to return that array, not node.sorted. + return node.sorted ? keys : (node.sorted = keys); + } + // The .slice(0) is necessary so that we do not modify the original keys array + // by calling keys.sort(), and also so that we always return a new (!==) + // sorted array when keys was not already sorted. + return node.sorted || (node.sorted = keys.slice(0).sort()); +} From be458d38539012d6be0a0dc73818abf5896d340d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 14:34:06 -0400 Subject: [PATCH 05/18] Bump .size-limit.cjs limits slightly. --- .size-limit.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index 090bc4c9dc4..286c4f7ba0d 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -1,7 +1,7 @@ const checks = [ { path: "dist/apollo-client.min.cjs", - limit: "38000", + limit: "38060", }, { path: "dist/main.cjs", @@ -10,7 +10,7 @@ const checks = [ { path: "dist/index.js", import: "{ ApolloClient, InMemoryCache, HttpLink }", - limit: "32052", + limit: "32095", }, ...[ "ApolloProvider", From fa4ff5f35240ddb71792286dc4bf46dcd7f2e01c Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 14:39:05 -0400 Subject: [PATCH 06/18] Add a changeset file. --- .changeset/beige-geese-wink.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/beige-geese-wink.md diff --git a/.changeset/beige-geese-wink.md b/.changeset/beige-geese-wink.md new file mode 100644 index 00000000000..d92e77ccb9d --- /dev/null +++ b/.changeset/beige-geese-wink.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Decouple `canonicalStringify` from `ObjectCanon` for better time and memory performance. From 50e485a47f3d702ac1e24188d527031f2f243a1b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 26 Sep 2023 14:42:25 -0400 Subject: [PATCH 07/18] Make API extractor happy about canonicalStringify move. --- .api-reports/api-report-cache.md | 12 ++++-------- .api-reports/api-report-core.md | 6 +++--- .api-reports/api-report-utilities.md | 23 +++++++++++------------ .api-reports/api-report.md | 6 +++--- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/.api-reports/api-report-cache.md b/.api-reports/api-report-cache.md index e2135907143..0af8cb6cff1 100644 --- a/.api-reports/api-report-cache.md +++ b/.api-reports/api-report-cache.md @@ -192,7 +192,7 @@ export const cacheSlot: { // @public (undocumented) export const canonicalStringify: ((value: any) => string) & { - reset: typeof resetCanonicalStringify; + reset(): void; }; // @public (undocumented) @@ -858,9 +858,6 @@ export interface Reference { readonly __ref: string; } -// @public (undocumented) -function resetCanonicalStringify(): void; - // @public (undocumented) type SafeReadonly = T extends object ? Readonly : T; @@ -947,10 +944,9 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // -// src/cache/inmemory/object-canon.ts:203:32 - (ae-forgotten-export) The symbol "resetCanonicalStringify" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:98:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:92:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:126:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/.api-reports/api-report-core.md b/.api-reports/api-report-core.md index e3138c81d29..a2acd522195 100644 --- a/.api-reports/api-report-core.md +++ b/.api-reports/api-report-core.md @@ -2179,9 +2179,9 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // -// src/cache/inmemory/policies.ts:98:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:92:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:126:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:112:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:113:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts diff --git a/.api-reports/api-report-utilities.md b/.api-reports/api-report-utilities.md index cc97fb9dbe6..2114304b3d6 100644 --- a/.api-reports/api-report-utilities.md +++ b/.api-reports/api-report-utilities.md @@ -462,6 +462,11 @@ const enum CacheWriteBehavior { OVERWRITE = 1 } +// @public (undocumented) +export const canonicalStringify: ((value: any) => string) & { + reset(): void; +}; + // @public (undocumented) type CanReadFunction = (value: StoreValue) => boolean; @@ -1072,9 +1077,7 @@ export function getOperationName(doc: DocumentNode): string | null; export function getQueryDefinition(doc: DocumentNode): OperationDefinitionNode; // @public (undocumented) -export const getStoreKeyName: ((fieldName: string, args?: Record | null, directives?: Directives) => string) & { - setStringify(s: typeof stringify): (value: any) => string; -}; +export function getStoreKeyName(fieldName: string, args?: Record | null, directives?: Directives): string; // @public (undocumented) export function getTypenameFromResult(result: Record, selectionSet: SelectionSetNode, fragmentMap?: FragmentMap): string | undefined; @@ -2298,9 +2301,6 @@ type StoreObjectValueMaybeReference = StoreVal extends Record string; - // @public (undocumented) export function stringifyForDisplay(value: any, space?: number): string; @@ -2498,11 +2498,11 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // // src/cache/core/types/DataProxy.ts:141:5 - (ae-forgotten-export) The symbol "MissingFieldError" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:63:3 - (ae-forgotten-export) The symbol "TypePolicy" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:168:3 - (ae-forgotten-export) The symbol "FieldReadFunction" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:169:3 - (ae-forgotten-export) The symbol "FieldMergeFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:57:3 - (ae-forgotten-export) The symbol "TypePolicy" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "FieldReadFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:163:3 - (ae-forgotten-export) The symbol "FieldMergeFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:126:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/writeToStore.ts:65:7 - (ae-forgotten-export) The symbol "MergeTree" needs to be exported by the entry point index.d.ts // src/core/ApolloClient.ts:47:3 - (ae-forgotten-export) The symbol "UriFunction" needs to be exported by the entry point index.d.ts @@ -2517,7 +2517,6 @@ interface WriteContext extends ReadMergeModifyContext { // src/core/types.ts:178:3 - (ae-forgotten-export) The symbol "MutationQueryReducer" needs to be exported by the entry point index.d.ts // src/core/types.ts:205:5 - (ae-forgotten-export) The symbol "Resolver" needs to be exported by the entry point index.d.ts // src/core/watchQueryOptions.ts:191:3 - (ae-forgotten-export) The symbol "UpdateQueryFn" needs to be exported by the entry point index.d.ts -// src/utilities/graphql/storeUtils.ts:202:12 - (ae-forgotten-export) The symbol "stringify" needs to be exported by the entry point index.d.ts // src/utilities/policies/pagination.ts:76:3 - (ae-forgotten-export) The symbol "TRelayEdge" needs to be exported by the entry point index.d.ts // src/utilities/policies/pagination.ts:77:3 - (ae-forgotten-export) The symbol "TRelayPageInfo" needs to be exported by the entry point index.d.ts diff --git a/.api-reports/api-report.md b/.api-reports/api-report.md index 1fac5b82bf4..bbf1f3f2898 100644 --- a/.api-reports/api-report.md +++ b/.api-reports/api-report.md @@ -2858,9 +2858,9 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // -// src/cache/inmemory/policies.ts:98:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:92:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:126:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:112:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:113:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts From 9211fd44a9c714657702fb076e1e15257ce30675 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 27 Sep 2023 13:02:20 -0400 Subject: [PATCH 08/18] Keep getStoreKeyName.setStringify but use canonicalStringify by default. https://github.com/apollographql/apollo-client/pull/11254#discussion_r1338588020 --- .api-reports/api-report-utilities.md | 12 ++- .size-limit.cjs | 4 +- src/utilities/graphql/storeUtils.ts | 114 +++++++++++++++------------ 3 files changed, 78 insertions(+), 52 deletions(-) diff --git a/.api-reports/api-report-utilities.md b/.api-reports/api-report-utilities.md index 2114304b3d6..4301c465dc5 100644 --- a/.api-reports/api-report-utilities.md +++ b/.api-reports/api-report-utilities.md @@ -1077,7 +1077,11 @@ export function getOperationName(doc: DocumentNode): string | null; export function getQueryDefinition(doc: DocumentNode): OperationDefinitionNode; // @public (undocumented) -export function getStoreKeyName(fieldName: string, args?: Record | null, directives?: Directives): string; +export const getStoreKeyName: ((fieldName: string, args?: Record | null, directives?: Directives) => string) & { + setStringify(s: typeof storeKeyNameStringify): ((value: any) => string) & { + reset(): void; + }; +}; // @public (undocumented) export function getTypenameFromResult(result: Record, selectionSet: SelectionSetNode, fragmentMap?: FragmentMap): string | undefined; @@ -2287,6 +2291,11 @@ type StorageType = Record; // @public (undocumented) export function storeKeyNameFromField(field: FieldNode, variables?: Object): string; +// @public (undocumented) +let storeKeyNameStringify: ((value: any) => string) & { + reset(): void; +}; + // @public (undocumented) export interface StoreObject { // (undocumented) @@ -2517,6 +2526,7 @@ interface WriteContext extends ReadMergeModifyContext { // src/core/types.ts:178:3 - (ae-forgotten-export) The symbol "MutationQueryReducer" needs to be exported by the entry point index.d.ts // src/core/types.ts:205:5 - (ae-forgotten-export) The symbol "Resolver" needs to be exported by the entry point index.d.ts // src/core/watchQueryOptions.ts:191:3 - (ae-forgotten-export) The symbol "UpdateQueryFn" needs to be exported by the entry point index.d.ts +// src/utilities/graphql/storeUtils.ts:208:12 - (ae-forgotten-export) The symbol "storeKeyNameStringify" needs to be exported by the entry point index.d.ts // src/utilities/policies/pagination.ts:76:3 - (ae-forgotten-export) The symbol "TRelayEdge" needs to be exported by the entry point index.d.ts // src/utilities/policies/pagination.ts:77:3 - (ae-forgotten-export) The symbol "TRelayPageInfo" needs to be exported by the entry point index.d.ts diff --git a/.size-limit.cjs b/.size-limit.cjs index 286c4f7ba0d..bc0d452dd60 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -1,7 +1,7 @@ const checks = [ { path: "dist/apollo-client.min.cjs", - limit: "38060", + limit: "38083", }, { path: "dist/main.cjs", @@ -10,7 +10,7 @@ const checks = [ { path: "dist/index.js", import: "{ ApolloClient, InMemoryCache, HttpLink }", - limit: "32095", + limit: "32123", }, ...[ "ApolloProvider", diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 5624432ebe1..c5b166065a6 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -195,62 +195,78 @@ const KNOWN_DIRECTIVES: string[] = [ "nonreactive", ]; -export function getStoreKeyName( - fieldName: string, - args?: Record | null, - directives?: Directives -): string { - if ( - args && - directives && - directives["connection"] && - directives["connection"]["key"] - ) { +// Default stable JSON.stringify implementation used by getStoreKeyName. Can be +// updated/replaced with something better by calling +// getStoreKeyName.setStringify(newStringifyFunction). +let storeKeyNameStringify = canonicalStringify; + +export const getStoreKeyName = Object.assign( + function ( + fieldName: string, + args?: Record | null, + directives?: Directives + ): string { if ( - directives["connection"]["filter"] && - (directives["connection"]["filter"] as string[]).length > 0 + args && + directives && + directives["connection"] && + directives["connection"]["key"] ) { - const filterKeys = directives["connection"]["filter"] - ? (directives["connection"]["filter"] as string[]) - : []; - filterKeys.sort(); - - const filteredArgs = {} as { [key: string]: any }; - filterKeys.forEach((key) => { - filteredArgs[key] = args[key]; - }); - - return `${directives["connection"]["key"]}(${canonicalStringify( - filteredArgs - )})`; - } else { - return directives["connection"]["key"]; + if ( + directives["connection"]["filter"] && + (directives["connection"]["filter"] as string[]).length > 0 + ) { + const filterKeys = directives["connection"]["filter"] + ? (directives["connection"]["filter"] as string[]) + : []; + filterKeys.sort(); + + const filteredArgs = {} as { [key: string]: any }; + filterKeys.forEach((key) => { + filteredArgs[key] = args[key]; + }); + + return `${directives["connection"]["key"]}(${storeKeyNameStringify( + filteredArgs + )})`; + } else { + return directives["connection"]["key"]; + } } - } - let completeFieldName: string = fieldName; + let completeFieldName: string = fieldName; - if (args) { - // We can't use `JSON.stringify` here since it's non-deterministic, - // and can lead to different store key names being created even though - // the `args` object used during creation has the same properties/values. - const stringifiedArgs: string = canonicalStringify(args); - completeFieldName += `(${stringifiedArgs})`; - } + if (args) { + // We can't use `JSON.stringify` here since it's non-deterministic, + // and can lead to different store key names being created even though + // the `args` object used during creation has the same properties/values. + const stringifiedArgs: string = storeKeyNameStringify(args); + completeFieldName += `(${stringifiedArgs})`; + } - if (directives) { - Object.keys(directives).forEach((key) => { - if (KNOWN_DIRECTIVES.indexOf(key) !== -1) return; - if (directives[key] && Object.keys(directives[key]).length) { - completeFieldName += `@${key}(${canonicalStringify(directives[key])})`; - } else { - completeFieldName += `@${key}`; - } - }); - } + if (directives) { + Object.keys(directives).forEach((key) => { + if (KNOWN_DIRECTIVES.indexOf(key) !== -1) return; + if (directives[key] && Object.keys(directives[key]).length) { + completeFieldName += `@${key}(${storeKeyNameStringify( + directives[key] + )})`; + } else { + completeFieldName += `@${key}`; + } + }); + } - return completeFieldName; -} + return completeFieldName; + }, + { + setStringify(s: typeof storeKeyNameStringify) { + const previous = storeKeyNameStringify; + storeKeyNameStringify = s; + return previous; + }, + } +); export function argumentsObjectFromField( field: FieldNode | DirectiveNode, From badac8b1a4c30d0977bac56e6e301e4065b8ea86 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 11:09:37 -0400 Subject: [PATCH 09/18] Give `storeKeyNameStringify` an explicit type signature. Co-authored-by: Lenz Weber-Tronic --- .api-reports/api-report-utilities.md | 8 ++------ src/utilities/graphql/storeUtils.ts | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/.api-reports/api-report-utilities.md b/.api-reports/api-report-utilities.md index 4301c465dc5..a90cd6f0b36 100644 --- a/.api-reports/api-report-utilities.md +++ b/.api-reports/api-report-utilities.md @@ -1078,9 +1078,7 @@ export function getQueryDefinition(doc: DocumentNode): OperationDefinitionNode; // @public (undocumented) export const getStoreKeyName: ((fieldName: string, args?: Record | null, directives?: Directives) => string) & { - setStringify(s: typeof storeKeyNameStringify): ((value: any) => string) & { - reset(): void; - }; + setStringify(s: typeof storeKeyNameStringify): (value: any) => string; }; // @public (undocumented) @@ -2292,9 +2290,7 @@ type StorageType = Record; export function storeKeyNameFromField(field: FieldNode, variables?: Object): string; // @public (undocumented) -let storeKeyNameStringify: ((value: any) => string) & { - reset(): void; -}; +let storeKeyNameStringify: (value: any) => string; // @public (undocumented) export interface StoreObject { diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index c5b166065a6..c119a447c26 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -198,7 +198,7 @@ const KNOWN_DIRECTIVES: string[] = [ // Default stable JSON.stringify implementation used by getStoreKeyName. Can be // updated/replaced with something better by calling // getStoreKeyName.setStringify(newStringifyFunction). -let storeKeyNameStringify = canonicalStringify; +let storeKeyNameStringify: (value: any) => string = canonicalStringify; export const getStoreKeyName = Object.assign( function ( From cbfdb9b265cd4cef7829537bf6a50b81dd238193 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 11:11:01 -0400 Subject: [PATCH 10/18] Better comment for `canonicalStringify` function. Co-authored-by: Lenz Weber-Tronic --- src/utilities/common/canonicalStringify.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 6627c827cd1..9fa634eecc0 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -1,4 +1,20 @@ -// Like JSON.stringify, but with object keys always sorted in the same order. +/** + * Like JSON.stringify, but with object keys always sorted in the same order. + * + * To achieve performant sorting, this function uses a trie to store the sorted + * key names of objects that have already been encountered, bringing "memoized" + * sort actions down to O(n) from O(n log n). + * + * As a drawback, this function will add a little bit more memory for every object + * is called with that has different (more, less, a different order) keys than + * in the past. + * + * In a typical application, this should not play a significant role, as + * `canonicalStringify` will be called for only a limited number of object shapes, + * and the cache will not grow beyond a certain point. + * But in some edge cases, this could be a problem, so we provide a `reset` method + * that will clear the cache and could be called at intervals from userland. + * */ export const canonicalStringify = Object.assign( function canonicalStringify(value: any): string { return JSON.stringify(value, stableObjectReplacer); From c8cab398fd6d0ba59e66e944c6460ab8e051ea79 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 11:55:37 -0400 Subject: [PATCH 11/18] Conserve total number of sorted arrays retained by canonicalStringify. --- src/utilities/common/canonicalStringify.ts | 56 ++++++++++++++++------ 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 9fa634eecc0..964b3063356 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -42,7 +42,7 @@ function stableObjectReplacer(key: string, value: any) { // means their prototype is either Object.prototype or null. if (proto === Object.prototype || proto === null) { const keys = Object.keys(value); - const sortedKeys = sortKeys(keys); + const sortedKeys = lookupSortedKeys(keys, true); if (sortedKeys !== keys) { const sorted = Object.create(null); // Reassigning the keys in sorted order will cause JSON.stringify to @@ -64,13 +64,19 @@ interface SortingTrie { const sortingTrieRoot: SortingTrie = { sorted: [], + // Using Object.create(null) is actually important here, since we could + // theoretically encounter strings like "__proto__" or "hasOwnProperty", which + // would be problematic if Object.prototype is in the prototype chain. next: Object.create(null), }; -// Sort the given keys using a lookup trie, and return the same (===) array in -// case it was already sorted, so we can avoid always creating a new object in -// the replacer function above. -function sortKeys(keys: readonly string[]): readonly string[] { +// Sort the given keys using a lookup trie, with an option to return the same +// (===) array in case it was already sorted, so we can avoid always creating a +// new object in the replacer function above. +function lookupSortedKeys( + keys: readonly string[], + returnKeysIfAlreadySorted: boolean, +): readonly string[] { let node = sortingTrieRoot; let alreadySorted = true; for (let k = 0, len = keys.length; k < len; ++k) { @@ -78,17 +84,37 @@ function sortKeys(keys: readonly string[]): readonly string[] { if (k > 0 && keys[k - 1] > key) { alreadySorted = false; } - const next = node.next; - node = next[key] || (next[key] = { next: Object.create(null) }); + node = node.next[key] || ( + node.next[key] = { next: Object.create(null) } + ); } + if (alreadySorted) { - // There may already be a node.sorted array that's equivalent to the - // already-sorted keys array, but if keys was already sorted, we always want - // to return that array, not node.sorted. - return node.sorted ? keys : (node.sorted = keys); + return node.sorted + // There may already be a node.sorted array that's equivalent to the + // already-sorted keys array, but if keys was already sorted, we want to + // return the keys reference as-is when returnKeysIfAlreadySorted is true. + // This behavior helps us decide whether we need to create a new object in + // the stableObjectReplacer function above. + ? (returnKeysIfAlreadySorted ? keys : node.sorted) + : (node.sorted = keys); } - // The .slice(0) is necessary so that we do not modify the original keys array - // by calling keys.sort(), and also so that we always return a new (!==) - // sorted array when keys was not already sorted. - return node.sorted || (node.sorted = keys.slice(0).sort()); + + // To conserve the total number of sorted arrays we store in the trie, we + // always use the same sorted array reference for a given set of strings, + // regardless of which permutation of the strings led to this SortingTrie + // node. To obtain this one true array, we do a little extra work to look up + // the sorted array associated with the sorted permutation, since there will + // be one unique path through the trie for the sorted permutation (even if + // there were duplicate keys). We can reuse the lookupSortedKeys function to + // perform this lookup, but we pass false for returnKeysIfAlreadySorted so it + // will return the existing array (if any) rather than the new sorted array we + // use to perform the lookup. If there is no existing array associated with + // the sorted permutation, the new array produced by keys.slice(0).sort() will + // be stored as the one true array and returned here. Since we are passing in + // an array that is definitely already sorted, this call to lookupSortedKeys + // will never actually have to call .sort(), so this lookup is always linear. + return node.sorted || ( + node.sorted = lookupSortedKeys(keys.slice(0).sort(), false) + ); } From 2311b8dbac4b1cd0bd42f493bd2e0610bb13badb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 12:02:53 -0400 Subject: [PATCH 12/18] Use Map to avoid two-level trie nodes and Object.create(null). --- src/utilities/common/canonicalStringify.ts | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 964b3063356..2b36648cf7b 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -24,7 +24,7 @@ export const canonicalStringify = Object.assign( // Blowing away the root-level trie map will reclaim all memory stored in // the trie, without affecting the logical results of canonicalStringify, // but potentially sacrificing performance until the trie is refilled. - sortingTrieRoot.next = Object.create(null); + sortingTrieRoot.clear(); }, } ); @@ -57,18 +57,15 @@ function stableObjectReplacer(key: string, value: any) { return value; } -interface SortingTrie { +type SortingTrie = Map & { + // If there is an entry in the trie for the sequence of keys leading up to + // this node, the node.sorted array will contain those keys in sorted order. + // The contents of the Map represent the next level(s) of the trie, branching + // out for each possible next key. sorted?: readonly string[]; - next: Record; } -const sortingTrieRoot: SortingTrie = { - sorted: [], - // Using Object.create(null) is actually important here, since we could - // theoretically encounter strings like "__proto__" or "hasOwnProperty", which - // would be problematic if Object.prototype is in the prototype chain. - next: Object.create(null), -}; +const sortingTrieRoot: SortingTrie = new Map; // Sort the given keys using a lookup trie, with an option to return the same // (===) array in case it was already sorted, so we can avoid always creating a @@ -84,9 +81,7 @@ function lookupSortedKeys( if (k > 0 && keys[k - 1] > key) { alreadySorted = false; } - node = node.next[key] || ( - node.next[key] = { next: Object.create(null) } - ); + node = node.get(key) || node.set(key, new Map).get(key)!; } if (alreadySorted) { From 1b4aad3537bf450f892e96c6c4a480a9eb141e44 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 13:16:08 -0400 Subject: [PATCH 13/18] Add tests of canonicalStringify and helper lookupSortedKeys. The lookupSortedKeys function is not intended to be used directly, but seemed worth unit testing nevertheless. --- .../common/__tests__/canonicalStringify.ts | 132 ++++++++++++++++++ src/utilities/common/canonicalStringify.ts | 2 +- src/utilities/index.ts | 2 +- 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 src/utilities/common/__tests__/canonicalStringify.ts diff --git a/src/utilities/common/__tests__/canonicalStringify.ts b/src/utilities/common/__tests__/canonicalStringify.ts new file mode 100644 index 00000000000..4322bdb5bca --- /dev/null +++ b/src/utilities/common/__tests__/canonicalStringify.ts @@ -0,0 +1,132 @@ +import { + canonicalStringify, + lookupSortedKeys, +} from "../canonicalStringify"; + +function forEachPermutation( + keys: string[], + callback: (permutation: string[]) => void, +) { + if (keys.length <= 1) { + callback(keys); + return; + } + const first = keys[0]; + const rest = keys.slice(1); + forEachPermutation(rest, (permutation) => { + for (let i = 0; i <= permutation.length; ++i) { + callback([ + ...permutation.slice(0, i), + first, + ...permutation.slice(i), + ]); + } + }); +} + +function allObjectPermutations>(obj: T) { + const keys = Object.keys(obj); + const permutations: T[] = []; + forEachPermutation(keys, permutation => { + const permutationObj = + Object.create(Object.getPrototypeOf(obj)); + permutation.forEach(key => { + permutationObj[key] = obj[key]; + }); + permutations.push(permutationObj); + }); + return permutations; +} + +describe("canonicalStringify", () => { + beforeEach(() => { + canonicalStringify.reset(); + }); + + it("should not modify original object", () => { + const obj = { c: 3, a: 1, b: 2 }; + expect(canonicalStringify(obj)).toBe('{"a":1,"b":2,"c":3}'); + expect(Object.keys(obj)).toEqual(["c", "a", "b"]); + }); + + it("forEachPermutation should work", () => { + const permutations: string[][] = []; + forEachPermutation(["a", "b", "c"], (permutation) => { + permutations.push(permutation); + }); + expect(permutations).toEqual([ + ["a", "b", "c"], + ["b", "a", "c"], + ["b", "c", "a"], + ["a", "c", "b"], + ["c", "a", "b"], + ["c", "b", "a"], + ]); + }); + + it("canonicalStringify should stably stringify all permutations of an object", () => { + const unstableStrings = new Set(); + const stableStrings = new Set(); + + allObjectPermutations({ + c: 3, + a: 1, + b: 2, + }).forEach(obj => { + unstableStrings.add(JSON.stringify(obj)); + stableStrings.add(canonicalStringify(obj)); + + expect(canonicalStringify(obj)).toBe('{"a":1,"b":2,"c":3}'); + + allObjectPermutations({ + z: "z", + y: ["y", obj, "why"], + x: "x", + }).forEach(parent => { + expect(canonicalStringify(parent)).toBe( + '{"x":"x","y":["y",{"a":1,"b":2,"c":3},"why"],"z":"z"}', + ); + }); + }); + + expect(unstableStrings.size).toBe(6); + expect(stableStrings.size).toBe(1); + }); + + it("lookupSortedKeys(keys, false) should reuse same sorted array for all permutations", () => { + const keys = ["z", "a", "c", "b"]; + const sorted = lookupSortedKeys(["z", "a", "b", "c"], false); + expect(sorted).toEqual(["a", "b", "c", "z"]); + forEachPermutation(keys, permutation => { + expect(lookupSortedKeys(permutation, false)).toBe(sorted); + }); + }); + + it("lookupSortedKeys(keys, true) should return same array if already sorted", () => { + const keys = ["a", "b", "c", "x", "y", "z"].sort(); + const sorted = lookupSortedKeys(keys, true); + expect(sorted).toBe(keys); + + forEachPermutation(keys, permutation => { + const sortedTrue = lookupSortedKeys(permutation, true); + const sortedFalse = lookupSortedKeys(permutation, false); + + expect(sortedTrue).toEqual(sorted); + expect(sortedFalse).toEqual(sorted); + + const wasPermutationSorted = + permutation.every((key, i) => key === keys[i]); + + if (wasPermutationSorted) { + expect(sortedTrue).toBe(permutation); + expect(sortedTrue).not.toBe(sorted); + } else { + expect(sortedTrue).not.toBe(permutation); + expect(sortedTrue).toBe(sorted); + } + + expect(sortedFalse).not.toBe(permutation); + expect(sortedFalse).toBe(sorted); + }); + }); +}); diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 2b36648cf7b..566dc7988e5 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -70,7 +70,7 @@ const sortingTrieRoot: SortingTrie = new Map; // Sort the given keys using a lookup trie, with an option to return the same // (===) array in case it was already sorted, so we can avoid always creating a // new object in the replacer function above. -function lookupSortedKeys( +export function lookupSortedKeys( keys: readonly string[], returnKeysIfAlreadySorted: boolean, ): readonly string[] { diff --git a/src/utilities/index.ts b/src/utilities/index.ts index adbd5e26c1e..ea660b1cafe 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -120,8 +120,8 @@ export * from "./common/makeUniqueId.js"; export * from "./common/stringifyForDisplay.js"; export * from "./common/mergeOptions.js"; export * from "./common/incrementalResult.js"; -export * from "./common/canonicalStringify.js"; +export { canonicalStringify } from "./common/canonicalStringify.js"; export { omitDeep } from "./common/omitDeep.js"; export { stripTypename } from "./common/stripTypename.js"; From dad8607bfd10662b767b779d63cf6a95ae0ec1c9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 14:13:08 -0400 Subject: [PATCH 14/18] Run prettier. --- .../common/__tests__/canonicalStringify.ts | 35 ++++++++----------- src/utilities/common/canonicalStringify.ts | 26 +++++++------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/utilities/common/__tests__/canonicalStringify.ts b/src/utilities/common/__tests__/canonicalStringify.ts index 4322bdb5bca..f8f5788a1dc 100644 --- a/src/utilities/common/__tests__/canonicalStringify.ts +++ b/src/utilities/common/__tests__/canonicalStringify.ts @@ -1,11 +1,8 @@ -import { - canonicalStringify, - lookupSortedKeys, -} from "../canonicalStringify"; +import { canonicalStringify, lookupSortedKeys } from "../canonicalStringify"; function forEachPermutation( keys: string[], - callback: (permutation: string[]) => void, + callback: (permutation: string[]) => void ) { if (keys.length <= 1) { callback(keys); @@ -15,11 +12,7 @@ function forEachPermutation( const rest = keys.slice(1); forEachPermutation(rest, (permutation) => { for (let i = 0; i <= permutation.length; ++i) { - callback([ - ...permutation.slice(0, i), - first, - ...permutation.slice(i), - ]); + callback([...permutation.slice(0, i), first, ...permutation.slice(i)]); } }); } @@ -27,10 +20,9 @@ function forEachPermutation( function allObjectPermutations>(obj: T) { const keys = Object.keys(obj); const permutations: T[] = []; - forEachPermutation(keys, permutation => { - const permutationObj = - Object.create(Object.getPrototypeOf(obj)); - permutation.forEach(key => { + forEachPermutation(keys, (permutation) => { + const permutationObj = Object.create(Object.getPrototypeOf(obj)); + permutation.forEach((key) => { permutationObj[key] = obj[key]; }); permutations.push(permutationObj); @@ -72,7 +64,7 @@ describe("canonicalStringify", () => { c: 3, a: 1, b: 2, - }).forEach(obj => { + }).forEach((obj) => { unstableStrings.add(JSON.stringify(obj)); stableStrings.add(canonicalStringify(obj)); @@ -82,9 +74,9 @@ describe("canonicalStringify", () => { z: "z", y: ["y", obj, "why"], x: "x", - }).forEach(parent => { + }).forEach((parent) => { expect(canonicalStringify(parent)).toBe( - '{"x":"x","y":["y",{"a":1,"b":2,"c":3},"why"],"z":"z"}', + '{"x":"x","y":["y",{"a":1,"b":2,"c":3},"why"],"z":"z"}' ); }); }); @@ -97,7 +89,7 @@ describe("canonicalStringify", () => { const keys = ["z", "a", "c", "b"]; const sorted = lookupSortedKeys(["z", "a", "b", "c"], false); expect(sorted).toEqual(["a", "b", "c", "z"]); - forEachPermutation(keys, permutation => { + forEachPermutation(keys, (permutation) => { expect(lookupSortedKeys(permutation, false)).toBe(sorted); }); }); @@ -107,15 +99,16 @@ describe("canonicalStringify", () => { const sorted = lookupSortedKeys(keys, true); expect(sorted).toBe(keys); - forEachPermutation(keys, permutation => { + forEachPermutation(keys, (permutation) => { const sortedTrue = lookupSortedKeys(permutation, true); const sortedFalse = lookupSortedKeys(permutation, false); expect(sortedTrue).toEqual(sorted); expect(sortedFalse).toEqual(sorted); - const wasPermutationSorted = - permutation.every((key, i) => key === keys[i]); + const wasPermutationSorted = permutation.every( + (key, i) => key === keys[i] + ); if (wasPermutationSorted) { expect(sortedTrue).toBe(permutation); diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 566dc7988e5..0ec56190edf 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -63,16 +63,16 @@ type SortingTrie = Map & { // The contents of the Map represent the next level(s) of the trie, branching // out for each possible next key. sorted?: readonly string[]; -} +}; -const sortingTrieRoot: SortingTrie = new Map; +const sortingTrieRoot: SortingTrie = new Map(); // Sort the given keys using a lookup trie, with an option to return the same // (===) array in case it was already sorted, so we can avoid always creating a // new object in the replacer function above. export function lookupSortedKeys( keys: readonly string[], - returnKeysIfAlreadySorted: boolean, + returnKeysIfAlreadySorted: boolean ): readonly string[] { let node = sortingTrieRoot; let alreadySorted = true; @@ -81,17 +81,19 @@ export function lookupSortedKeys( if (k > 0 && keys[k - 1] > key) { alreadySorted = false; } - node = node.get(key) || node.set(key, new Map).get(key)!; + node = node.get(key) || node.set(key, new Map()).get(key)!; } if (alreadySorted) { return node.sorted - // There may already be a node.sorted array that's equivalent to the - // already-sorted keys array, but if keys was already sorted, we want to - // return the keys reference as-is when returnKeysIfAlreadySorted is true. - // This behavior helps us decide whether we need to create a new object in - // the stableObjectReplacer function above. - ? (returnKeysIfAlreadySorted ? keys : node.sorted) + ? // There may already be a node.sorted array that's equivalent to the + // already-sorted keys array, but if keys was already sorted, we want to + // return the keys reference as-is when returnKeysIfAlreadySorted is true. + // This behavior helps us decide whether we need to create a new object in + // the stableObjectReplacer function above. + returnKeysIfAlreadySorted + ? keys + : node.sorted : (node.sorted = keys); } @@ -109,7 +111,7 @@ export function lookupSortedKeys( // be stored as the one true array and returned here. Since we are passing in // an array that is definitely already sorted, this call to lookupSortedKeys // will never actually have to call .sort(), so this lookup is always linear. - return node.sorted || ( - node.sorted = lookupSortedKeys(keys.slice(0).sort(), false) + return ( + node.sorted || (node.sorted = lookupSortedKeys(keys.slice(0).sort(), false)) ); } From c5446ac0030e141d3fe468b0bdbe60ef0847a8d9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 16:03:37 -0400 Subject: [PATCH 15/18] Switch to a simpler lookup strategy not involving a trie. --- .../common/__tests__/canonicalStringify.ts | 53 +++----- src/utilities/common/canonicalStringify.ts | 122 ++++++------------ 2 files changed, 61 insertions(+), 114 deletions(-) diff --git a/src/utilities/common/__tests__/canonicalStringify.ts b/src/utilities/common/__tests__/canonicalStringify.ts index f8f5788a1dc..a82f18e3863 100644 --- a/src/utilities/common/__tests__/canonicalStringify.ts +++ b/src/utilities/common/__tests__/canonicalStringify.ts @@ -1,4 +1,4 @@ -import { canonicalStringify, lookupSortedKeys } from "../canonicalStringify"; +import { canonicalStringify } from "../canonicalStringify"; function forEachPermutation( keys: string[], @@ -85,41 +85,26 @@ describe("canonicalStringify", () => { expect(stableStrings.size).toBe(1); }); - it("lookupSortedKeys(keys, false) should reuse same sorted array for all permutations", () => { - const keys = ["z", "a", "c", "b"]; - const sorted = lookupSortedKeys(["z", "a", "b", "c"], false); - expect(sorted).toEqual(["a", "b", "c", "z"]); - forEachPermutation(keys, (permutation) => { - expect(lookupSortedKeys(permutation, false)).toBe(sorted); - }); - }); - - it("lookupSortedKeys(keys, true) should return same array if already sorted", () => { - const keys = ["a", "b", "c", "x", "y", "z"].sort(); - const sorted = lookupSortedKeys(keys, true); - expect(sorted).toBe(keys); - - forEachPermutation(keys, (permutation) => { - const sortedTrue = lookupSortedKeys(permutation, true); - const sortedFalse = lookupSortedKeys(permutation, false); - - expect(sortedTrue).toEqual(sorted); - expect(sortedFalse).toEqual(sorted); + it("should not modify keys of custom-prototype objects", () => { + class Custom { + z = "z"; + y = "y"; + x = "x"; + b = "b"; + a = "a"; + c = "c"; + } - const wasPermutationSorted = permutation.every( - (key, i) => key === keys[i] - ); + const obj = { + z: "z", + x: "x", + y: new Custom(), + }; - if (wasPermutationSorted) { - expect(sortedTrue).toBe(permutation); - expect(sortedTrue).not.toBe(sorted); - } else { - expect(sortedTrue).not.toBe(permutation); - expect(sortedTrue).toBe(sorted); - } + expect(Object.keys(obj.y)).toEqual(["z", "y", "x", "b", "a", "c"]); - expect(sortedFalse).not.toBe(permutation); - expect(sortedFalse).toBe(sorted); - }); + expect(canonicalStringify(obj)).toBe( + '{"x":"x","y":{"z":"z","y":"y","x":"x","b":"b","a":"a","c":"c"},"z":"z"}' + ); }); }); diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 0ec56190edf..631b9f3b4b4 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -1,19 +1,19 @@ /** * Like JSON.stringify, but with object keys always sorted in the same order. * - * To achieve performant sorting, this function uses a trie to store the sorted - * key names of objects that have already been encountered, bringing "memoized" - * sort actions down to O(n) from O(n log n). + * To achieve performant sorting, this function uses a Map from JSON-serialized + * arrays of keys (in any order) to sorted arrays of the same keys, with a + * single sorted array reference shared by all permutations of the keys. * - * As a drawback, this function will add a little bit more memory for every object - * is called with that has different (more, less, a different order) keys than - * in the past. + * As a drawback, this function will add a little bit more memory for every + * object encountered that has different (more, less, a different order of) keys + * than in the past. * - * In a typical application, this should not play a significant role, as - * `canonicalStringify` will be called for only a limited number of object shapes, - * and the cache will not grow beyond a certain point. - * But in some edge cases, this could be a problem, so we provide a `reset` method - * that will clear the cache and could be called at intervals from userland. + * In a typical application, this extra memory usage should not play a + * significant role, as `canonicalStringify` will be called for only a limited + * number of object shapes, and the cache will not grow beyond a certain point. + * But in some edge cases, this could be a problem, so we provide + * canonicalStringify.reset() as a way of clearing the cache. * */ export const canonicalStringify = Object.assign( function canonicalStringify(value: any): string { @@ -21,14 +21,16 @@ export const canonicalStringify = Object.assign( }, { reset() { - // Blowing away the root-level trie map will reclaim all memory stored in - // the trie, without affecting the logical results of canonicalStringify, - // but potentially sacrificing performance until the trie is refilled. - sortingTrieRoot.clear(); + // Clearing the sortingMap will reclaim all cached memory, without + // affecting the logical results of canonicalStringify, but potentially + // sacrificing performance until the cache is refilled. + sortingMap.clear(); }, } ); +const sortingMap = new Map(); + // The JSON.stringify function takes an optional second argument called a // replacer function. This function is called for each key-value pair in the // object being stringified, and its return value is used instead of the @@ -39,79 +41,39 @@ function stableObjectReplacer(key: string, value: any) { if (value && typeof value === "object") { const proto = Object.getPrototypeOf(value); // We don't want to mess with objects that are not "plain" objects, which - // means their prototype is either Object.prototype or null. + // means their prototype is either Object.prototype or null. This check also + // prevents needlessly rearranging the indices of arrays. if (proto === Object.prototype || proto === null) { const keys = Object.keys(value); - const sortedKeys = lookupSortedKeys(keys, true); - if (sortedKeys !== keys) { - const sorted = Object.create(null); - // Reassigning the keys in sorted order will cause JSON.stringify to - // serialize them in sorted order. - sortedKeys.forEach((key) => { - sorted[key] = value[key]; - }); - return sorted; + if (isArraySorted(keys)) return value; + const unsortedKey = JSON.stringify(keys); + let sortedKeys = sortingMap.get(unsortedKey); + if (!sortedKeys) { + keys.sort(); + const sortedKey = JSON.stringify(keys); + // Checking for sortedKey in the sortingMap allows us to share the same + // sorted array reference for all permutations of the same set of keys. + sortedKeys = sortingMap.get(sortedKey) || keys; + sortingMap.set(unsortedKey, sortedKeys); + sortingMap.set(sortedKey, sortedKeys); } + const sortedObject = Object.create(Object.getPrototypeOf(value)); + // Reassigning the keys in sorted order will cause JSON.stringify to + // serialize them in sorted order. + sortedKeys.forEach((key) => { + sortedObject[key] = value[key]; + }); + return sortedObject; } } return value; } -type SortingTrie = Map & { - // If there is an entry in the trie for the sequence of keys leading up to - // this node, the node.sorted array will contain those keys in sorted order. - // The contents of the Map represent the next level(s) of the trie, branching - // out for each possible next key. - sorted?: readonly string[]; -}; - -const sortingTrieRoot: SortingTrie = new Map(); - -// Sort the given keys using a lookup trie, with an option to return the same -// (===) array in case it was already sorted, so we can avoid always creating a -// new object in the replacer function above. -export function lookupSortedKeys( - keys: readonly string[], - returnKeysIfAlreadySorted: boolean -): readonly string[] { - let node = sortingTrieRoot; - let alreadySorted = true; - for (let k = 0, len = keys.length; k < len; ++k) { - const key = keys[k]; - if (k > 0 && keys[k - 1] > key) { - alreadySorted = false; +function isArraySorted(keys: readonly string[]): boolean { + for (let k = 1, len = keys.length; k < len; ++k) { + if (keys[k - 1] > keys[k]) { + return false; } - node = node.get(key) || node.set(key, new Map()).get(key)!; } - - if (alreadySorted) { - return node.sorted - ? // There may already be a node.sorted array that's equivalent to the - // already-sorted keys array, but if keys was already sorted, we want to - // return the keys reference as-is when returnKeysIfAlreadySorted is true. - // This behavior helps us decide whether we need to create a new object in - // the stableObjectReplacer function above. - returnKeysIfAlreadySorted - ? keys - : node.sorted - : (node.sorted = keys); - } - - // To conserve the total number of sorted arrays we store in the trie, we - // always use the same sorted array reference for a given set of strings, - // regardless of which permutation of the strings led to this SortingTrie - // node. To obtain this one true array, we do a little extra work to look up - // the sorted array associated with the sorted permutation, since there will - // be one unique path through the trie for the sorted permutation (even if - // there were duplicate keys). We can reuse the lookupSortedKeys function to - // perform this lookup, but we pass false for returnKeysIfAlreadySorted so it - // will return the existing array (if any) rather than the new sorted array we - // use to perform the lookup. If there is no existing array associated with - // the sorted permutation, the new array produced by keys.slice(0).sort() will - // be stored as the one true array and returned here. Since we are passing in - // an array that is definitely already sorted, this call to lookupSortedKeys - // will never actually have to call .sort(), so this lookup is always linear. - return ( - node.sorted || (node.sorted = lookupSortedKeys(keys.slice(0).sort(), false)) - ); + return true; } From 5636a345608ec7b5e46db7ac4d6f37baf5f1a9bb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 16:08:50 -0400 Subject: [PATCH 16/18] Reduce size limits after simplifying canonicalStringify. --- .size-limit.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index bc0d452dd60..37f130e22f1 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -1,7 +1,7 @@ const checks = [ { path: "dist/apollo-client.min.cjs", - limit: "38083", + limit: "38049", }, { path: "dist/main.cjs", @@ -10,7 +10,7 @@ const checks = [ { path: "dist/index.js", import: "{ ApolloClient, InMemoryCache, HttpLink }", - limit: "32123", + limit: "32082", }, ...[ "ApolloProvider", From c2a8e34c5558c6d1fd5a33d36c237e55e5064b8e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 28 Sep 2023 17:16:05 -0400 Subject: [PATCH 17/18] Another comment, and one less call to Object.getPrototypeOf. --- src/utilities/common/canonicalStringify.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 631b9f3b4b4..7aed5ac4990 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -29,6 +29,8 @@ export const canonicalStringify = Object.assign( } ); +// Values are JSON-serialized arrays of object keys (in any order), and values +// are sorted arrays of the same keys. const sortingMap = new Map(); // The JSON.stringify function takes an optional second argument called a @@ -57,7 +59,7 @@ function stableObjectReplacer(key: string, value: any) { sortingMap.set(unsortedKey, sortedKeys); sortingMap.set(sortedKey, sortedKeys); } - const sortedObject = Object.create(Object.getPrototypeOf(value)); + const sortedObject = Object.create(proto); // Reassigning the keys in sorted order will cause JSON.stringify to // serialize them in sorted order. sortedKeys.forEach((key) => { From d0aa0301ebfaf5b6894386382a4a2c2c7a350fb6 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 2 Oct 2023 11:24:30 -0400 Subject: [PATCH 18/18] Replace isArraySorted with keys.every(everyKeyInOrder). https://github.com/apollographql/apollo-client/pull/11254#discussion_r1342825654 --- src/utilities/common/canonicalStringify.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/utilities/common/canonicalStringify.ts b/src/utilities/common/canonicalStringify.ts index 7aed5ac4990..021f23430e2 100644 --- a/src/utilities/common/canonicalStringify.ts +++ b/src/utilities/common/canonicalStringify.ts @@ -47,7 +47,9 @@ function stableObjectReplacer(key: string, value: any) { // prevents needlessly rearranging the indices of arrays. if (proto === Object.prototype || proto === null) { const keys = Object.keys(value); - if (isArraySorted(keys)) return value; + // If keys is already sorted, let JSON.stringify serialize the original + // value instead of creating a new object with keys in the same order. + if (keys.every(everyKeyInOrder)) return value; const unsortedKey = JSON.stringify(keys); let sortedKeys = sortingMap.get(unsortedKey); if (!sortedKeys) { @@ -71,11 +73,14 @@ function stableObjectReplacer(key: string, value: any) { return value; } -function isArraySorted(keys: readonly string[]): boolean { - for (let k = 1, len = keys.length; k < len; ++k) { - if (keys[k - 1] > keys[k]) { - return false; - } - } - return true; +// Since everything that happens in stableObjectReplacer benefits from being as +// efficient as possible, we use a static function as the callback for +// keys.every in order to test if the provided keys are already sorted without +// allocating extra memory for a callback. +function everyKeyInOrder( + key: string, + i: number, + keys: readonly string[] +): boolean { + return i === 0 || keys[i - 1] <= key; }