diff --git a/CHANGELOG.md b/CHANGELOG.md index 13bcd0516cc..5d1263e2faf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,9 @@ These local variables are _reactive_ in the sense that updating their values invalidates any previously cached query results that depended on the old values.
[@benjamn](https://github.com/benjamn) in [#5799](https://github.com/apollographql/apollo-client/pull/5799) +- Various cache read and write performance optimizations, cutting read and write times by more than 50% in larger benchmarks.
+ [@benjamn](https://github.com/benjamn) in [#5948](https://github.com/apollographql/apollo-client/pull/5948) + - The `cache.readQuery` and `cache.writeQuery` methods now accept an `options.id` string, which eliminates most use cases for `cache.readFragment` and `cache.writeFragment`, and skips the implicit conversion of fragment documents to query documents performed by `cache.{read,write}Fragment`.
[@benjamn](https://github.com/benjamn) in [#5930](https://github.com/apollographql/apollo-client/pull/5930) diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index fdbd8c82f47..8e5a7d4a3e1 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -361,7 +361,10 @@ class CacheGroup { } function makeDepKey(dataId: string, storeFieldName: string) { - return JSON.stringify([dataId, fieldNameFromStoreName(storeFieldName)]); + // Since field names cannot have newline characters in them, this method + // of joining the field name and the ID should be unambiguous, and much + // cheaper than JSON.stringify([dataId, fieldName]). + return fieldNameFromStoreName(storeFieldName) + "#" + dataId; } export namespace EntityStore { diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 0b01c84716b..514a97335f4 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -212,12 +212,8 @@ export class Policies { }; } = Object.create(null); - public readonly rootTypenamesById: Readonly> = { - __proto__: null, // Equivalent to Object.create(null) - ROOT_QUERY: "Query", - ROOT_MUTATION: "Mutation", - ROOT_SUBSCRIPTION: "Subscription", - }; + public readonly rootIdsByTypename: Record = Object.create(null); + public readonly rootTypenamesById: Record = Object.create(null); public readonly usingPossibleTypes = false; @@ -231,6 +227,10 @@ export class Policies { ...config, }; + this.setRootTypename("Query"); + this.setRootTypename("Mutation"); + this.setRootTypename("Subscription"); + if (config.possibleTypes) { this.addPossibleTypes(config.possibleTypes); } @@ -346,13 +346,14 @@ export class Policies { private setRootTypename( which: "Query" | "Mutation" | "Subscription", - typename: string, + typename: string = which, ) { const rootId = "ROOT_" + which.toUpperCase(); const old = this.rootTypenamesById[rootId]; if (typename !== old) { - invariant(old === which, `Cannot change root ${which} __typename more than once`); - (this.rootTypenamesById as any)[rootId] = typename; + invariant(!old || old === which, `Cannot change root ${which} __typename more than once`); + this.rootIdsByTypename[typename] = rootId; + this.rootTypenamesById[rootId] = typename; } } diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 2041537c226..1688f4343e3 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -46,6 +46,8 @@ interface ExecContext { policies: Policies; fragmentMap: FragmentMap; variables: VariableMap; + // A JSON.stringify-serialized version of context.variables. + varString: string; }; export type ExecResult = { @@ -91,7 +93,7 @@ export class StoreReader { if (supportsResultCaching(context.store)) { return context.store.makeCacheKey( selectionSet, - JSON.stringify(context.variables), + context.varString, isReference(objectOrReference) ? objectOrReference.__ref : objectOrReference, @@ -108,7 +110,7 @@ export class StoreReader { return context.store.makeCacheKey( field, array, - JSON.stringify(context.variables), + context.varString, ); } } @@ -151,6 +153,11 @@ export class StoreReader { }: DiffQueryAgainstStoreOptions): Cache.DiffResult { const { policies } = this.config; + variables = { + ...getDefaultValues(getQueryDefinition(query)), + ...variables, + }; + const execResult = this.executeSelectionSet({ selectionSet: getMainDefinition(query).selectionSet, objectOrReference: makeReference(rootId), @@ -158,10 +165,8 @@ export class StoreReader { store, query, policies, - variables: { - ...getDefaultValues(getQueryDefinition(query)), - ...variables, - }, + variables, + varString: JSON.stringify(variables), fragmentMap: createFragmentMap(getFragmentDefinitions(query)), }, }); @@ -201,9 +206,7 @@ export class StoreReader { if (this.config.addTypename && typeof typename === "string" && - Object.values( - policies.rootTypenamesById - ).indexOf(typename) < 0) { + !policies.rootIdsByTypename[typename]) { // Ensure we always include a default value for the __typename // field, if we have one, and this.config.addTypename is true. Note // that this field can be overridden by other merged objects. @@ -219,7 +222,9 @@ export class StoreReader { return result.result; } - selectionSet.selections.forEach(selection => { + const workSet = new Set(selectionSet.selections); + + workSet.forEach(selection => { // Omit fields with directives @skip(if: ) or // @include(if: ). if (!shouldInclude(selection, variables)) return; @@ -296,13 +301,7 @@ export class StoreReader { } if (policies.fragmentMatches(fragment, typename)) { - objectsToMerge.push(handleMissing( - this.executeSelectionSet({ - selectionSet: fragment.selectionSet, - objectOrReference, - context, - }) - )); + fragment.selectionSet.selections.forEach(workSet.add, workSet); } } }); diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index f3051beb4a4..6087b58068e 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -27,7 +27,7 @@ import { cloneDeep } from '../../utilities/common/cloneDeep'; import { Policies } from './policies'; import { defaultNormalizedCacheFactory } from './entityStore'; import { NormalizedCache, StoreObject } from './types'; -import { makeProcessedFieldsMerger } from './helpers'; +import { makeProcessedFieldsMerger, FieldValueToBeMerged } from './helpers'; export type WriteContext = { readonly store: NormalizedCache; @@ -40,6 +40,16 @@ export type WriteContext = { merge(existing: T, incoming: T): T; }; +interface ProcessSelectionSetOptions { + result: Record; + selectionSet: SelectionSetNode; + context: WriteContext; + typename: string; + out: { + shouldApplyMerges?: boolean; + }; +} + export interface StoreWriterConfig { policies: Policies; }; @@ -134,20 +144,26 @@ export class StoreWriter { // fall back to that. store.get(dataId, "__typename") as string; - store.merge( - dataId, - policies.applyMerges( + const out: ProcessSelectionSetOptions["out"] = Object.create(null); + + let processed = this.processSelectionSet({ + result, + selectionSet, + context, + typename, + out, + }); + + if (out.shouldApplyMerges) { + processed = policies.applyMerges( makeReference(dataId), - this.processSelectionSet({ - result, - selectionSet, - context, - typename, - }), + processed, store.getFieldValue, context.variables, - ), - ); + ); + } + + store.merge(dataId, processed); return store; } @@ -157,23 +173,20 @@ export class StoreWriter { selectionSet, context, typename, - }: { - result: Record; - selectionSet: SelectionSetNode; - context: WriteContext; - typename: string; - }): StoreObject { + // This object allows processSelectionSet to report useful information + // to its callers without explicitly returning that information. + out, + }: ProcessSelectionSetOptions): StoreObject { let mergedFields: StoreObject = Object.create(null); if (typeof typename === "string") { mergedFields.__typename = typename; } - selectionSet.selections.forEach(selection => { - if (!shouldInclude(selection, context.variables)) { - return; - } + const { policies } = this; + const workSet = new Set(selectionSet.selections); - const { policies } = this; + workSet.forEach(selection => { + if (!shouldInclude(selection, context.variables)) return; if (isField(selection)) { const resultFieldKey = resultKeyNameFromField(selection); @@ -186,22 +199,27 @@ export class StoreWriter { context.variables, ); - const incomingValue = - this.processFieldValue(value, selection, context); + let incomingValue = + this.processFieldValue(value, selection, context, out); - mergedFields = context.merge(mergedFields, { + if (policies.hasMergeFunction(typename, selection.name.value)) { // If a custom merge function is defined for this field, store // a special FieldValueToBeMerged object, so that we can run // the merge function later, after all processSelectionSet // work is finished. - [storeFieldName]: policies.hasMergeFunction( - typename, - selection.name.value, - ) ? { + incomingValue = { __field: selection, __typename: typename, __value: incomingValue, - } : incomingValue, + } as FieldValueToBeMerged; + + // Communicate to the caller that mergedFields contains at + // least one FieldValueToBeMerged. + out.shouldApplyMerges = true; + } + + mergedFields = context.merge(mergedFields, { + [storeFieldName]: incomingValue, }); } else if ( @@ -233,15 +251,7 @@ export class StoreWriter { ); if (policies.fragmentMatches(fragment, typename)) { - mergedFields = context.merge( - mergedFields, - this.processSelectionSet({ - result, - selectionSet: fragment.selectionSet, - context, - typename, - }), - ); + fragment.selectionSet.selections.forEach(workSet.add, workSet); } } }); @@ -253,6 +263,7 @@ export class StoreWriter { value: any, field: FieldNode, context: WriteContext, + out: ProcessSelectionSetOptions["out"], ): StoreValue { if (!field.selectionSet || value === null) { // In development, we need to clone scalar values so that they can be @@ -262,7 +273,8 @@ export class StoreWriter { } if (Array.isArray(value)) { - return value.map((item, i) => this.processFieldValue(item, field, context)); + return value.map( + (item, i) => this.processFieldValue(item, field, context, out)); } if (value) { @@ -291,6 +303,7 @@ export class StoreWriter { context, typename: getTypenameFromResult( value, field.selectionSet, context.fragmentMap), + out, }); } } diff --git a/src/utilities/common/mergeDeep.ts b/src/utilities/common/mergeDeep.ts index 977a3c46ea9..c9074522ce5 100644 --- a/src/utilities/common/mergeDeep.ts +++ b/src/utilities/common/mergeDeep.ts @@ -64,8 +64,6 @@ const defaultReconciler: ReconcilerFunction = }; export class DeepMerger { - private pastCopies: any[] = []; - constructor( private reconciler: ReconcilerFunction = defaultReconciler, ) {} @@ -101,12 +99,10 @@ export class DeepMerger { public isObject = isObject; + private pastCopies = new Set(); + public shallowCopyForMerge(value: T): T { - if ( - value !== null && - typeof value === 'object' && - this.pastCopies.indexOf(value) < 0 - ) { + if (isObject(value) && !this.pastCopies.has(value)) { if (Array.isArray(value)) { value = (value as any).slice(0); } else { @@ -115,7 +111,7 @@ export class DeepMerger { ...value, }; } - this.pastCopies.push(value); + this.pastCopies.add(value); } return value; } diff --git a/src/utilities/graphql/directives.ts b/src/utilities/graphql/directives.ts index a7a20a01fb4..31658f1be16 100644 --- a/src/utilities/graphql/directives.ts +++ b/src/utilities/graphql/directives.ts @@ -19,15 +19,18 @@ export type DirectiveInfo = { }; export function shouldInclude( - selection: SelectionNode, - variables: { [name: string]: any } = {}, + { directives }: SelectionNode, + variables?: Record, ): boolean { + if (!directives || !directives.length) { + return true; + } return getInclusionDirectives( - selection.directives, + directives ).every(({ directive, ifArgument }) => { let evaledValue: boolean = false; if (ifArgument.value.kind === 'Variable') { - evaledValue = variables[(ifArgument.value as VariableNode).name.value]; + evaledValue = variables && variables[(ifArgument.value as VariableNode).name.value]; invariant( evaledValue !== void 0, `Invalid variable referenced in @${directive.name.value} directive.`, @@ -77,31 +80,39 @@ function isInclusionDirective({ name: { value } }: DirectiveNode): boolean { export function getInclusionDirectives( directives: ReadonlyArray, ): InclusionDirectives { - return directives ? directives.filter(isInclusionDirective).map(directive => { - const directiveArguments = directive.arguments; - const directiveName = directive.name.value; - - invariant( - directiveArguments && directiveArguments.length === 1, - `Incorrect number of arguments for the @${directiveName} directive.`, - ); - - const ifArgument = directiveArguments[0]; - invariant( - ifArgument.name && ifArgument.name.value === 'if', - `Invalid argument for the @${directiveName} directive.`, - ); - - const ifValue: ValueNode = ifArgument.value; - - // means it has to be a variable value if this is a valid @skip or @include directive - invariant( - ifValue && - (ifValue.kind === 'Variable' || ifValue.kind === 'BooleanValue'), - `Argument for the @${directiveName} directive must be a variable or a boolean value.`, - ); - - return { directive, ifArgument }; - }) : []; + const result: InclusionDirectives = []; + + if (directives && directives.length) { + directives.forEach(directive => { + if (!isInclusionDirective(directive)) return; + + const directiveArguments = directive.arguments; + const directiveName = directive.name.value; + + invariant( + directiveArguments && directiveArguments.length === 1, + `Incorrect number of arguments for the @${directiveName} directive.`, + ); + + const ifArgument = directiveArguments[0]; + invariant( + ifArgument.name && ifArgument.name.value === 'if', + `Invalid argument for the @${directiveName} directive.`, + ); + + const ifValue: ValueNode = ifArgument.value; + + // means it has to be a variable value if this is a valid @skip or @include directive + invariant( + ifValue && + (ifValue.kind === 'Variable' || ifValue.kind === 'BooleanValue'), + `Argument for the @${directiveName} directive must be a variable or a boolean value.`, + ); + + result.push({ directive, ifArgument }); + }); + } + + return result; }