From e3205d7166edceac57ad0056b7eb63857a7924f8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 11:18:54 -0500 Subject: [PATCH 01/10] Call cache.transformForLink before each link execution. This is not a disruptive change for InMemoryCache, since it uses the default implementation of tranformForLink (inherited from ApolloCache), which simply returns the given document. If you're using a different ApolloCache implementation that has a custom transformForLink implementation, this new behavior should be more convenient and flexible, but you should probably double-check that your transformForLink method is suitably cached/idempotent, so multiple calls with the same input document return the same (===) transformed document. --- src/cache/core/cache.ts | 14 ++++++++------ src/core/QueryInfo.ts | 3 ++- src/core/QueryManager.ts | 31 ++++++++++++++++++++++--------- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 4aa8b372a11..b87f9da95eb 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -97,10 +97,18 @@ export abstract class ApolloCache implements DataProxy { // Optional API + // Called once per input document, allowing the cache to make static changes + // to the query, such as adding __typename fields. public transformDocument(document: DocumentNode): DocumentNode { return document; } + // Called before each ApolloLink request, allowing the cache to make dynamic + // changes to the query, such as filling in missing fragment definitions. + public transformForLink(document: DocumentNode): DocumentNode { + return document; + } + public identify(object: StoreObject | Reference): string | undefined { return; } @@ -113,12 +121,6 @@ export abstract class ApolloCache implements DataProxy { return false; } - // Experimental API - - public transformForLink(document: DocumentNode): DocumentNode { - return document; - } - // DataProxy API /** * diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index bc2a7bc9118..844ebe13cab 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -356,6 +356,7 @@ export class QueryInfo { public markResult( result: FetchResult, + document: DocumentNode, options: Pick { if (this.shouldWrite(result, options.variables)) { cache.writeQuery({ - query: this.document!, + query: document, data: result.data as T, variables: options.variables, overwrite: cacheWriteBehavior === CacheWriteBehavior.OVERWRITE, diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 04bec55c54e..b3b511206a9 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -190,11 +190,15 @@ export class QueryManager { ); const mutationId = this.generateMutationId(); - mutation = this.transform(mutation).document; - variables = this.getVariables(mutation, variables) as TVariables; + const { + document, + hasClientExports, + } = this.transform(mutation); + mutation = this.cache.transformForLink(document); - if (this.transform(mutation).hasClientExports) { + variables = this.getVariables(mutation, variables) as TVariables; + if (hasClientExports) { variables = await this.localState.addExportedVariables(mutation, variables, context) as TVariables; } @@ -558,11 +562,9 @@ export class QueryManager { if (!transformCache.has(document)) { const transformed = this.cache.transformDocument(document); - const forLink = removeConnectionDirectiveFromDocument( - this.cache.transformForLink(transformed)); - + const noConnection = removeConnectionDirectiveFromDocument(transformed); const clientQuery = this.localState.clientQuery(transformed); - const serverQuery = forLink && this.localState.serverQuery(forLink); + const serverQuery = noConnection && this.localState.serverQuery(noConnection); const cacheEntry: TransformCacheEntry = { document: transformed, @@ -1040,9 +1042,17 @@ export class QueryManager { // even though the input object may have been modified in the meantime. options = cloneDeep(options); + // Performing transformForLink here gives this.cache a chance to fill in + // missing fragment definitions (for example) before sending this document + // through the link chain. + const linkDocument = this.cache.transformForLink( + // Use same document originally produced by this.cache.transformDocument. + this.transform(queryInfo.document!).document + ); + return asyncMap( this.getObservableFromLink( - queryInfo.document!, + linkDocument, options.context, options.variables, ), @@ -1071,7 +1081,10 @@ export class QueryManager { graphQLErrors, })); } - queryInfo.markResult(result, options, cacheWriteBehavior); + // Use linkDocument rather than queryInfo.document so the + // operation/fragments used to write the result are the same as the + // ones used to obtain it from the link. + queryInfo.markResult(result, linkDocument, options, cacheWriteBehavior); queryInfo.markReady(); } From 570dbaaee1407865a1530ba1b7b315a62227de6e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 11:35:53 -0500 Subject: [PATCH 02/10] Allow getFragmentFromSelection to take a FragmentMapFunction. --- src/utilities/graphql/fragments.ts | 12 +++++++++--- src/utilities/index.ts | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/utilities/graphql/fragments.ts b/src/utilities/graphql/fragments.ts index 5c0ad42b8b9..6cf762fa07c 100644 --- a/src/utilities/graphql/fragments.ts +++ b/src/utilities/graphql/fragments.ts @@ -108,6 +108,9 @@ export interface FragmentMap { [fragmentName: string]: FragmentDefinitionNode; } +export type FragmentMapFunction = + (fragmentName: string) => FragmentDefinitionNode | null; + // Utility function that takes a list of fragment definitions and makes a hash out of them // that maps the name of the fragment to the fragment definition. export function createFragmentMap( @@ -122,14 +125,17 @@ export function createFragmentMap( export function getFragmentFromSelection( selection: SelectionNode, - fragmentMap?: FragmentMap, + fragmentMap?: FragmentMap | FragmentMapFunction, ): InlineFragmentNode | FragmentDefinitionNode | null { switch (selection.kind) { case 'InlineFragment': return selection; case 'FragmentSpread': { - const fragment = fragmentMap && fragmentMap[selection.name.value]; - invariant(fragment, `No fragment named ${selection.name.value}.`); + const fragmentName = selection.name.value; + const fragment = typeof fragmentMap === "function" + ? fragmentMap(fragmentName) + : fragmentMap && fragmentMap[fragmentName]; + invariant(fragment, `No fragment named ${fragmentName}.`); return fragment!; } default: diff --git a/src/utilities/index.ts b/src/utilities/index.ts index b810049aee6..4f6f4697897 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -14,6 +14,7 @@ export { export { FragmentMap, + FragmentMapFunction, createFragmentMap, getFragmentQueryDocument, getFragmentFromSelection, From c9559351826c36cf968eacaf877d0a09fc8241ee Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 11:37:39 -0500 Subject: [PATCH 03/10] Prevent isEmpty from recursively calling itself with falsy documents. --- src/utilities/graphql/transform.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/utilities/graphql/transform.ts b/src/utilities/graphql/transform.ts index 8ee698beafa..78f497764f7 100644 --- a/src/utilities/graphql/transform.ts +++ b/src/utilities/graphql/transform.ts @@ -66,12 +66,11 @@ const TYPENAME_FIELD: FieldNode = { function isEmpty( op: OperationDefinitionNode | FragmentDefinitionNode, - fragments: FragmentMap, + fragmentMap: FragmentMap, ): boolean { - return op.selectionSet.selections.every( - selection => - selection.kind === 'FragmentSpread' && - isEmpty(fragments[selection.name.value], fragments), + return !op || op.selectionSet.selections.every( + selection => selection.kind === 'FragmentSpread' && + isEmpty(fragmentMap[selection.name.value], fragmentMap) ); } From 01a22ba796647297d59f26452d5db505998e2986 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 11:44:59 -0500 Subject: [PATCH 04/10] Implement a FragmentRegistry for registering named fragments. This is just one implementation of a FragmentRegistry that could be provided to InMemoryCache to fill in missing fragment declarations in transformForLink. Learning from past mistakes, we will avoid referring directly to this particular implementation of FragmentRegistry within the InMemoryCache implementation, since that forces the full FragmentRegistry implementation to be bundled even when it is not used. --- src/cache/inmemory/fragmentRegistry.ts | 149 +++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 src/cache/inmemory/fragmentRegistry.ts diff --git a/src/cache/inmemory/fragmentRegistry.ts b/src/cache/inmemory/fragmentRegistry.ts new file mode 100644 index 00000000000..60f077d324e --- /dev/null +++ b/src/cache/inmemory/fragmentRegistry.ts @@ -0,0 +1,149 @@ +import { + DocumentNode, + ASTNode, + FragmentDefinitionNode, + FragmentSpreadNode, + visit, +} from "graphql"; + +import { wrap } from "optimism"; + +import { FragmentMap, getFragmentDefinitions } from "../../utilities"; + +export class FragmentRegistry { + private registry: FragmentMap = Object.create(null); + + static from(...fragments: DocumentNode[]): FragmentRegistry { + const registry = new this(); + return registry.register.apply(registry, fragments); + } + + // Call static method FragmentRegistry.from(...) instead of invoking the + // FragmentRegistry constructor directly. This reserves the constructor for + // future configuration of the FragmentRegistry. + protected constructor() { + this.resetCaches(); + } + + public register(...fragments: DocumentNode[]): this { + const definitions = new Map(); + fragments.forEach(doc => { + getFragmentDefinitions(doc).forEach(node => { + definitions.set(node.name.value, node); + }); + }); + + definitions.forEach((node, name) => { + if (node !== this.registry[name]) { + this.registry[name] = node; + this.invalidate(name); + } + }); + + return this; + } + + // Overridden in the resetCaches method below. + private invalidate(name: string) {} + + public resetCaches() { + this.invalidate = ( + this.lookup = this.cacheUnaryMethod("lookup") + ).dirty; // This dirty function is bound to the wrapped lookup method. + this.transform = this.cacheUnaryMethod("transform"); + this.findFragmentSpreads = this.cacheUnaryMethod("findFragmentSpreads"); + } + + private cacheUnaryMethod>(name: TName) { + const registry = this; + const originalMethod = FragmentRegistry.prototype[name]; + return wrap(function () { + return originalMethod.apply(registry, arguments); + }, { + makeCacheKey: arg => arg, + }); + } + + public lookup(fragmentName: string) { + return this.registry[fragmentName] || null; + } + + public transform(document: D): D { + const defined = new Map(); + getFragmentDefinitions(document).forEach(def => { + defined.set(def.name.value, def); + }); + + const unbound = new Set(); + const enqueue = (spreadName: string) => { + if (!defined.has(spreadName)) { + unbound.add(spreadName); + } + }; + + const enqueueChildSpreads = (node: ASTNode) => Object.keys( + this.findFragmentSpreads(node) + ).forEach(enqueue); + + enqueueChildSpreads(document); + + const missing: string[] = []; + const map: FragmentMap = Object.create(null); + + // This Set forEach loop can be extended during iteration by adding + // additional strings to the unbound set. + unbound.forEach(fragmentName => { + const knownFragmentDef = defined.get(fragmentName); + if (knownFragmentDef) { + enqueueChildSpreads(map[fragmentName] = knownFragmentDef); + } else { + missing.push(fragmentName); + const def = this.lookup(fragmentName); + if (def) { + enqueueChildSpreads(map[fragmentName] = def); + } + } + }); + + if (missing.length) { + const defsToAppend: FragmentDefinitionNode[] = []; + missing.forEach(name => { + const def = map[name]; + if (def) { + defsToAppend.push(def); + } else { + // TODO Warn? Error? + } + }); + + if (defsToAppend.length) { + document = { + ...document, + definitions: document.definitions.concat(defsToAppend), + }; + } + } + + return document; + } + + public findFragmentSpreads(root: ASTNode): FragmentSpreadMap { + const spreads: FragmentSpreadMap = Object.create(null); + + visit(root, { + FragmentSpread(node) { + spreads[node.name.value] = node; + }, + }); + + return spreads; + } +} + +interface FragmentSpreadMap { + [fragmentSpreadName: string]: FragmentSpreadNode; +} From 50f917a8490efd5f946ebcd7301625179b131e83 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 12:21:58 -0500 Subject: [PATCH 05/10] Provide abstract FragmentRegistryAPI TypeScript interface. As long as InMemoryCache uses only this TypeScript interface when handling FragmentRegistry configurations, any implementation can be swapped in without also paying for the bundle size of the default implementation (the FragmentRegistry class). --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/cache/index.ts | 5 +++ src/cache/inmemory/fragmentRegistry.ts | 38 +++++++++++++++------ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index 1496a099a0b..b16e03c1870 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -72,6 +72,7 @@ Array [ "Policies", "cacheSlot", "canonicalStringify", + "createFragmentRegistry", "defaultDataIdFromObject", "fieldNameFromStoreName", "isReference", diff --git a/src/cache/index.ts b/src/cache/index.ts index c3439c3b647..c42b5446df5 100644 --- a/src/cache/index.ts +++ b/src/cache/index.ts @@ -48,4 +48,9 @@ export { canonicalStringify, } from './inmemory/object-canon'; +export { + FragmentRegistryAPI, + createFragmentRegistry, +} from './inmemory/fragmentRegistry'; + export * from './inmemory/types'; diff --git a/src/cache/inmemory/fragmentRegistry.ts b/src/cache/inmemory/fragmentRegistry.ts index 60f077d324e..e0b1fb086b0 100644 --- a/src/cache/inmemory/fragmentRegistry.ts +++ b/src/cache/inmemory/fragmentRegistry.ts @@ -10,24 +10,42 @@ import { wrap } from "optimism"; import { FragmentMap, getFragmentDefinitions } from "../../utilities"; -export class FragmentRegistry { - private registry: FragmentMap = Object.create(null); +export interface FragmentRegistryAPI { + register(...fragments: DocumentNode[]): this; + lookup(fragmentName: string): FragmentDefinitionNode | null; + transform(document: D): D; +} - static from(...fragments: DocumentNode[]): FragmentRegistry { - const registry = new this(); - return registry.register.apply(registry, fragments); - } +// As long as createFragmentRegistry is not imported or used, the +// FragmentRegistry example implementation provided below should not be bundled +// (by tree-shaking bundlers like Rollup), because the implementation of +// InMemoryCache refers only to the TypeScript interface FragmentRegistryAPI, +// never the concrete implementation FragmentRegistry (which is deliberately not +// exported from this module). +export function createFragmentRegistry( + ...fragments: DocumentNode[] +): FragmentRegistryAPI { + return new FragmentRegistry(...fragments); +} + +const { forEach: arrayLikeForEach } = Array.prototype; + +class FragmentRegistry implements FragmentRegistryAPI { + private registry: FragmentMap = Object.create(null); // Call static method FragmentRegistry.from(...) instead of invoking the // FragmentRegistry constructor directly. This reserves the constructor for // future configuration of the FragmentRegistry. - protected constructor() { + constructor(...fragments: DocumentNode[]) { this.resetCaches(); + if (fragments.length) { + this.register.apply(this, fragments); + } } - public register(...fragments: DocumentNode[]): this { + public register(): this { const definitions = new Map(); - fragments.forEach(doc => { + arrayLikeForEach.call(arguments, (doc: DocumentNode) => { getFragmentDefinitions(doc).forEach(node => { definitions.set(node.name.value, node); }); @@ -68,7 +86,7 @@ export class FragmentRegistry { }); } - public lookup(fragmentName: string) { + public lookup(fragmentName: string): FragmentDefinitionNode | null { return this.registry[fragmentName] || null; } From abecb11601f286e414101a3bc5b53017e7eb1499 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 11:54:11 -0500 Subject: [PATCH 06/10] Use InMemoryCache#transformForLink to supply missing fragments. Since these changes do not include any calls to createFragmentRegistry, the FragmentRegistry class implementation remains optional and thus tree-shakable if unused. --- src/cache/inmemory/helpers.ts | 31 +++++++++++++++++++++++++++-- src/cache/inmemory/inMemoryCache.ts | 10 ++++++++++ src/cache/inmemory/readFromStore.ts | 13 +++++++----- src/cache/inmemory/types.ts | 3 +++ src/cache/inmemory/writeToStore.ts | 20 +++++++++++-------- 5 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/cache/inmemory/helpers.ts b/src/cache/inmemory/helpers.ts index 729f4347626..18e0bae2b97 100644 --- a/src/cache/inmemory/helpers.ts +++ b/src/cache/inmemory/helpers.ts @@ -1,4 +1,4 @@ -import { SelectionSetNode } from 'graphql'; +import { DocumentNode, FragmentDefinitionNode, SelectionSetNode } from 'graphql'; import { NormalizedCache, @@ -6,6 +6,7 @@ import { } from './types'; import { KeyFieldsContext } from './policies'; +import { FragmentRegistryAPI } from './fragmentRegistry'; import { Reference, @@ -18,6 +19,10 @@ import { shouldInclude, isNonNullObject, compact, + FragmentMap, + FragmentMapFunction, + createFragmentMap, + getFragmentDefinitions, } from '../../utilities'; export const { @@ -28,6 +33,8 @@ export function isNullish(value: any): value is null | undefined { return value === null || value === void 0; } +export const isArray: (a: any) => a is any[] | readonly any[] = Array.isArray; + export function defaultDataIdFromObject( { __typename, id, _id }: Readonly, context?: KeyFieldsContext, @@ -128,4 +135,24 @@ export function makeProcessedFieldsMerger() { return new DeepMerger; } -export const isArray = (a: any): a is any[] | readonly any[] => Array.isArray(a) +export function extractFragmentContext( + document: DocumentNode, + fragments?: FragmentRegistryAPI, +): { + fragmentMap: FragmentMap; + lookupFragment: FragmentMapFunction; +} { + // FragmentMap consisting only of fragments defined directly in document, not + // including other fragments registered in the FragmentRegistry. + const fragmentMap = createFragmentMap(getFragmentDefinitions(document)); + return { + fragmentMap, + lookupFragment(name) { + let def: FragmentDefinitionNode | null = fragmentMap[name]; + if (!def && fragments) { + def = fragments.lookup(name); + } + return def || null; + }, + }; +} diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 69a497f332a..8fd05f23dfb 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -91,6 +91,7 @@ export class InMemoryCache extends ApolloCache { private resetResultCache(resetResultIdentities?: boolean) { const previousReader = this.storeReader; + const { fragments } = this.config; // The StoreWriter is mostly stateless and so doesn't really need to be // reset, but it does need to have its writer.storeReader reference updated, @@ -105,7 +106,9 @@ export class InMemoryCache extends ApolloCache { canon: resetResultIdentities ? void 0 : previousReader && previousReader.canon, + fragments, }), + fragments, ); this.maybeBroadcastWatch = wrap(( @@ -514,6 +517,13 @@ export class InMemoryCache extends ApolloCache { return document; } + public transformForLink(document: DocumentNode): DocumentNode { + const { fragments } = this.config; + return fragments + ? fragments.transform(document) + : document; + } + protected broadcastWatches(options?: BroadcastOptions) { if (!this.txCount) { this.watches.forEach(c => this.maybeBroadcastWatch(c, options)); diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index a7fc0189c55..30bd2d3a2c2 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -14,12 +14,10 @@ import { isReference, makeReference, StoreObject, - createFragmentMap, FragmentMap, shouldInclude, addTypenameToDocument, getDefaultValues, - getFragmentDefinitions, getMainDefinition, getQueryDefinition, getFragmentFromSelection, @@ -29,15 +27,17 @@ import { isNonNullObject, canUseWeakMap, compact, + FragmentMapFunction, } from '../../utilities'; import { Cache } from '../core/types/Cache'; import { DiffQueryAgainstStoreOptions, + InMemoryCacheConfig, NormalizedCache, ReadMergeModifyContext, } from './types'; import { maybeDependOnExistenceOfEntity, supportsResultCaching } from './entityStore'; -import { getTypenameFromStoreObject, isArray, shouldCanonizeResults } from './helpers'; +import { isArray, extractFragmentContext, getTypenameFromStoreObject, shouldCanonizeResults } from './helpers'; import { Policies } from './policies'; import { InMemoryCache } from './inMemoryCache'; import { MissingFieldError, MissingTree } from '../core/types/common'; @@ -50,6 +50,7 @@ interface ReadContext extends ReadMergeModifyContext { policies: Policies; canonizeResults: boolean; fragmentMap: FragmentMap; + lookupFragment: FragmentMapFunction; }; export type ExecResult = { @@ -77,6 +78,7 @@ export interface StoreReaderConfig { resultCacheMaxSize?: number; canonizeResults?: boolean; canon?: ObjectCanon; + fragments?: InMemoryCacheConfig["fragments"]; } // Arguments type after keyArgs translation. @@ -119,6 +121,7 @@ export class StoreReader { addTypename: boolean; resultCacheMaxSize?: number; canonizeResults: boolean; + fragments?: InMemoryCacheConfig["fragments"]; }; private knownResults = new ( @@ -243,7 +246,7 @@ export class StoreReader { variables, varString: canonicalStringify(variables), canonizeResults, - fragmentMap: createFragmentMap(getFragmentDefinitions(query)), + ...extractFragmentContext(query, this.config.fragments), }, }); @@ -400,7 +403,7 @@ export class StoreReader { } else { const fragment = getFragmentFromSelection( selection, - context.fragmentMap, + context.lookupFragment, ); if (fragment && policies.fragmentMatches(fragment, typename)) { diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts index ace759a6650..d6a52ef809e 100644 --- a/src/cache/inmemory/types.ts +++ b/src/cache/inmemory/types.ts @@ -21,6 +21,8 @@ import { CanReadFunction, } from '../core/types/common'; +import { FragmentRegistryAPI } from './fragmentRegistry'; + export { StoreObject, StoreValue, Reference } export interface IdGetterObj extends Object { @@ -130,6 +132,7 @@ export interface InMemoryCacheConfig extends ApolloReducerConfig { typePolicies?: TypePolicies; resultCacheMaxSize?: number; canonizeResults?: boolean; + fragments?: FragmentRegistryAPI; } export interface MergeInfo { diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 44c14f79fcd..47697b03f9d 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -7,11 +7,10 @@ import { } from 'graphql'; import { - createFragmentMap, FragmentMap, + FragmentMapFunction, getFragmentFromSelection, getDefaultValues, - getFragmentDefinitions, getOperationDefinition, getTypenameFromResult, makeReference, @@ -28,8 +27,8 @@ import { argumentsObjectFromField, } from '../../utilities'; -import { NormalizedCache, ReadMergeModifyContext, MergeTree } from './types'; -import { makeProcessedFieldsMerger, fieldNameFromStoreName, storeValueIsStoreObject, isArray } from './helpers'; +import { NormalizedCache, ReadMergeModifyContext, MergeTree, InMemoryCacheConfig } from './types'; +import { isArray, makeProcessedFieldsMerger, fieldNameFromStoreName, storeValueIsStoreObject, extractFragmentContext } from './helpers'; import { StoreReader } from './readFromStore'; import { InMemoryCache } from './inMemoryCache'; import { EntityStore } from './entityStore'; @@ -42,7 +41,8 @@ export interface WriteContext extends ReadMergeModifyContext { readonly written: { [dataId: string]: SelectionSetNode[]; }; - readonly fragmentMap?: FragmentMap; + readonly fragmentMap: FragmentMap; + lookupFragment: FragmentMapFunction; // General-purpose deep-merge function for use during writes. merge(existing: T, incoming: T): T; // If true, merge functions will be called with undefined existing data. @@ -104,6 +104,7 @@ export class StoreWriter { constructor( public readonly cache: InMemoryCache, private reader?: StoreReader, + private fragments?: InMemoryCacheConfig["fragments"], ) {} public writeToStore(store: NormalizedCache, { @@ -129,7 +130,7 @@ export class StoreWriter { }, variables, varString: canonicalStringify(variables), - fragmentMap: createFragmentMap(getFragmentDefinitions(query)), + ...extractFragmentContext(query, this.fragments), overwrite: !!overwrite, incomingById: new Map, clientOnly: false, @@ -475,6 +476,7 @@ export class StoreWriter { | "deferred" | "flavors" | "fragmentMap" + | "lookupFragment" | "variables" >>( selectionSet: SelectionSetNode, @@ -559,8 +561,10 @@ export class StoreWriter { ); } else { - const fragment = - getFragmentFromSelection(selection, context.fragmentMap); + const fragment = getFragmentFromSelection( + selection, + context.lookupFragment, + ); if (fragment && policies.fragmentMatches( From 52c7f61f88531b2e502fdcad9fccd5ffa6e5319d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Feb 2022 11:55:15 -0500 Subject: [PATCH 07/10] Basic tests of configuring InMemoryCache with FragmentRegistryAPI. --- .../inmemory/__tests__/fragmentRegistry.ts | 180 ++++++++++++++++++ src/cache/inmemory/__tests__/writeToStore.ts | 6 +- 2 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 src/cache/inmemory/__tests__/fragmentRegistry.ts diff --git a/src/cache/inmemory/__tests__/fragmentRegistry.ts b/src/cache/inmemory/__tests__/fragmentRegistry.ts new file mode 100644 index 00000000000..44c57213eba --- /dev/null +++ b/src/cache/inmemory/__tests__/fragmentRegistry.ts @@ -0,0 +1,180 @@ +import { ApolloClient, ApolloLink, gql, NetworkStatus } from "../../../core"; +import { getFragmentDefinitions, Observable } from "../../../utilities"; +import { InMemoryCache, createFragmentRegistry } from "../../index"; +import { itAsync, subscribeAndCount } from "../../../testing"; + +describe("FragmentRegistry", () => { + it("can be passed to InMemoryCache", () => { + const cache = new InMemoryCache({ + fragments: createFragmentRegistry(gql` + fragment BasicFragment on Query { + basic + } + `), + }); + + // TODO Allow writeFragment to just use fragmentName:"BasicFragment"? + cache.writeQuery({ + query: gql` + query { + ...BasicFragment + } + `, + data: { + basic: true, + }, + }); + + const result = cache.readQuery({ + query: gql` + query { + ...BasicFragment + } + `, + }); + + expect(result).toEqual({ + basic: true, + }); + }); + + itAsync("influences ApolloClient and ApolloLink", (resolve, reject) => { + const cache = new InMemoryCache({ + fragments: createFragmentRegistry(gql` + fragment SourceFragment on Query { + source + } + `), + }); + + const client = new ApolloClient({ + cache, + link: new ApolloLink(operation => new Observable(observer => { + expect( + getFragmentDefinitions(operation.query).map(def => def.name.value).sort() + ).toEqual([ + // Proof that the missing SourceFragment definition was appended to + // operation.query before it was passed into the link. + "SourceFragment", + ]); + + observer.next({ + data: { + source: "link", + }, + }); + + observer.complete(); + })), + }); + + const query = gql` + query SourceQuery { + ...SourceFragment + } + `; + + cache.writeQuery({ + query, + data: { + source: "local", + }, + }); + + subscribeAndCount(reject, client.watchQuery({ + query, + fetchPolicy: "cache-and-network", + }), (count, result) => { + if (count === 1) { + expect(result).toEqual({ + loading: true, + networkStatus: NetworkStatus.loading, + data: { + source: "local", + }, + }); + + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + networkStatus: NetworkStatus.ready, + data: { + source: "link", + }, + }); + + expect(cache.readQuery({ query })).toEqual({ + source: "link", + }); + + setTimeout(resolve, 10); + } else { + reject(`Unexpectedly many results (${count})`); + } + }); + }); + + it("can register fragments with unbound ...spreads", () => { + const cache = new InMemoryCache({ + fragments: createFragmentRegistry(gql` + fragment NeedsExtra on Person { + __typename + id + # This fragment spread has a default definition below, but can be + # selectively overridden by queries. + ...ExtraFields + } + + fragment ExtraFields on Person { + __typename + } + `), + }); + + const query = gql` + query GetMe { + me { + ...NeedsExtra + } + } + + # This version of the ExtraFields fragment will be used instead of the one + # registered in the FragmentRegistry, because explicit definitions take + # precedence over registered fragments. + fragment ExtraFields on Person { + name + } + `; + + cache.writeQuery({ + query, + data: { + me: { + __typename: "Person", + id: 12345, + name: "Alice", + }, + }, + }); + + expect(cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + me: { __ref: "Person:12345" }, + }, + "Person:12345": { + __typename: "Person", + id: 12345, + name: "Alice", + }, + }); + + expect(cache.readQuery({ query })).toEqual({ + me: { + __typename: "Person", + id: 12345, + name: "Alice", + }, + }); + }); +}); diff --git a/src/cache/inmemory/__tests__/writeToStore.ts b/src/cache/inmemory/__tests__/writeToStore.ts index 5fc2a16b2de..86c24df986c 100644 --- a/src/cache/inmemory/__tests__/writeToStore.ts +++ b/src/cache/inmemory/__tests__/writeToStore.ts @@ -17,8 +17,6 @@ import { StoreObject, addTypenameToDocument, cloneDeep, - createFragmentMap, - getFragmentDefinitions, getMainDefinition, } from '../../../utilities'; import { itAsync } from '../../../testing/core'; @@ -27,6 +25,7 @@ import { defaultNormalizedCacheFactory, writeQueryToStore } from './helpers'; import { InMemoryCache } from '../inMemoryCache'; import { withErrorSpy, withWarningSpy } from '../../../testing'; import { TypedDocumentNode } from '../../../core' +import { extractFragmentContext } from '../helpers'; const getIdField = ({ id }: { id: string }) => id; @@ -3118,7 +3117,6 @@ describe('writing to the store', () => { }, ) { const { selectionSet } = getMainDefinition(query); - const fragmentMap = createFragmentMap(getFragmentDefinitions(query)); const flat = writer["flattenFields"](selectionSet, { __typename: "Query", @@ -3126,7 +3124,7 @@ describe('writing to the store', () => { bField: "b", rootField: "root", }, { - fragmentMap, + ...extractFragmentContext(query), clientOnly: false, deferred: false, flavors: new Map, From 32b4acd70a246c83fe2ea7a065a4e88902330c67 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 12 Sep 2022 16:45:48 -0400 Subject: [PATCH 08/10] When fragmentMap is a function, let it decide whether to throw. --- src/utilities/graphql/fragments.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/utilities/graphql/fragments.ts b/src/utilities/graphql/fragments.ts index 6cf762fa07c..0746f6b7b20 100644 --- a/src/utilities/graphql/fragments.ts +++ b/src/utilities/graphql/fragments.ts @@ -132,11 +132,12 @@ export function getFragmentFromSelection( return selection; case 'FragmentSpread': { const fragmentName = selection.name.value; - const fragment = typeof fragmentMap === "function" - ? fragmentMap(fragmentName) - : fragmentMap && fragmentMap[fragmentName]; - invariant(fragment, `No fragment named ${fragmentName}.`); - return fragment!; + if (typeof fragmentMap === "function") { + return fragmentMap(fragmentName); + } + const fragment = fragmentMap && fragmentMap[fragmentName]; + invariant(fragment, `No fragment named ${fragmentName}`); + return fragment || null; } default: return null; From 5c6ae248cfd7e5389d1b461ec51eb832d8d40b98 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 28 Jun 2022 11:58:28 -0400 Subject: [PATCH 09/10] Test error thrown when FragmentRegistry cannot resolve all fragments. --- .../inmemory/__tests__/fragmentRegistry.ts | 104 ++++++++++++++++++ src/cache/inmemory/fragmentRegistry.ts | 2 - src/cache/inmemory/inMemoryCache.ts | 10 +- src/cache/inmemory/readFromStore.ts | 7 +- src/cache/inmemory/writeToStore.ts | 5 + 5 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/cache/inmemory/__tests__/fragmentRegistry.ts b/src/cache/inmemory/__tests__/fragmentRegistry.ts index 44c57213eba..e098b5a7ef6 100644 --- a/src/cache/inmemory/__tests__/fragmentRegistry.ts +++ b/src/cache/inmemory/__tests__/fragmentRegistry.ts @@ -114,6 +114,110 @@ describe("FragmentRegistry", () => { }); }); + it("throws an error when not all used fragments are defined", () => { + const cache = new InMemoryCache({ + fragments: createFragmentRegistry(gql` + fragment IncompleteFragment on Person { + __typename + id + ...MustBeDefinedByQuery + } + `), + }); + + const queryWithoutFragment = gql` + query WithoutFragment { + me { + ...IncompleteFragment + } + } + `; + + const queryWithFragment = gql` + query WithFragment { + me { + ...IncompleteFragment + } + } + + fragment MustBeDefinedByQuery on Person { + name + } + `; + + expect(() => { + cache.writeQuery({ + query: queryWithoutFragment, + data: { + me: { + __typename: "Person", + id: 12345, + name: "Ben", + }, + }, + }); + }).toThrow( + /No fragment named MustBeDefinedByQuery/ + ); + + expect(cache.extract()).toEqual({ + // Nothing written because the cache.writeQuery failed above. + }); + + cache.writeQuery({ + query: queryWithFragment, + data: { + me: { + __typename: "Person", + id: 12345, + name: "Ben Newman", + }, + }, + }); + + expect(cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + me: { __ref: "Person:12345" }, + }, + "Person:12345": { + __typename: "Person", + id: 12345, + name: "Ben Newman", + }, + }); + + expect(() => { + cache.diff({ + query: queryWithoutFragment, + returnPartialData: true, + optimistic: true, + }); + }).toThrow( + /No fragment named MustBeDefinedByQuery/ + ); + + expect(() => { + cache.readQuery({ + query: queryWithoutFragment + }); + }).toThrow( + /No fragment named MustBeDefinedByQuery/ + ); + + expect( + cache.readQuery({ + query: queryWithFragment, + }), + ).toEqual({ + me: { + __typename: "Person", + id: 12345, + name: "Ben Newman", + }, + }); + }); + it("can register fragments with unbound ...spreads", () => { const cache = new InMemoryCache({ fragments: createFragmentRegistry(gql` diff --git a/src/cache/inmemory/fragmentRegistry.ts b/src/cache/inmemory/fragmentRegistry.ts index e0b1fb086b0..3756b9022ca 100644 --- a/src/cache/inmemory/fragmentRegistry.ts +++ b/src/cache/inmemory/fragmentRegistry.ts @@ -133,8 +133,6 @@ class FragmentRegistry implements FragmentRegistryAPI { const def = map[name]; if (def) { defsToAppend.push(def); - } else { - // TODO Warn? Error? } }); diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 8fd05f23dfb..fdd0a3b44d3 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -181,11 +181,11 @@ export class InMemoryCache extends ApolloCache { }).result || null; } catch (e) { if (e instanceof MissingFieldError) { - // Swallow MissingFieldError and return null, so callers do not - // need to worry about catching "normal" exceptions resulting from - // incomplete cache data. Unexpected errors will be re-thrown. If - // you need more information about which fields were missing, use - // cache.diff instead, and examine diffResult.missing. + // Swallow MissingFieldError and return null, so callers do not need to + // worry about catching "normal" exceptions resulting from incomplete + // cache data. Unexpected errors will be re-thrown. If you need more + // information about which fields were missing, use cache.diff instead, + // and examine diffResult.missing. return null; } throw e; diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 30bd2d3a2c2..20e5a6bfe4d 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -1,8 +1,9 @@ -import { invariant } from '../../utilities/globals'; +import { invariant, InvariantError } from '../../utilities/globals'; import { DocumentNode, FieldNode, + Kind, SelectionSetNode, } from 'graphql'; import { wrap, OptimisticWrapperFunction } from 'optimism'; @@ -406,6 +407,10 @@ export class StoreReader { context.lookupFragment, ); + if (!fragment && selection.kind === Kind.FRAGMENT_SPREAD) { + throw new InvariantError(`No fragment named ${selection.name.value}`); + } + if (fragment && policies.fragmentMatches(fragment, typename)) { fragment.selectionSet.selections.forEach(workSet.add, workSet); } diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 47697b03f9d..6d511a79ef1 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -4,6 +4,7 @@ import { Trie } from '@wry/trie'; import { SelectionSetNode, FieldNode, + Kind, } from 'graphql'; import { @@ -566,6 +567,10 @@ export class StoreWriter { context.lookupFragment, ); + if (!fragment && selection.kind === Kind.FRAGMENT_SPREAD) { + throw new InvariantError(`No fragment named ${selection.name.value}`); + } + if (fragment && policies.fragmentMatches( fragment, typename, result, context.variables)) { From dc8b8bacd9f52ac1cc18249a190016631bd83bf0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 15 Sep 2022 12:58:09 -0400 Subject: [PATCH 10/10] Bump bundlesize limit to 31.65kB (now 31.63kB). --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a159d17fdc3..12924c81eb6 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ { "name": "apollo-client", "path": "./dist/apollo-client.min.cjs", - "maxSize": "31.46kB" + "maxSize": "31.65kB" } ], "engines": {