Skip to content

Commit

Permalink
Partially backpeddle apollographql#5603
Browse files Browse the repository at this point in the history
Solves infinite loops when more than one query with different fields on an unidentified object

Fixes apollographql#6370
  • Loading branch information
Suckow, Thomas J authored and Suckow, Thomas J committed Jun 13, 2020
1 parent e68c5d4 commit d006ce1
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/ApolloClient.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ Object {
"a": 1,
"d": Object {
"__typename": "D",
"e": 4,
"h": Object {
"__typename": "H",
"i": 7,
Expand Down
1 change: 1 addition & 0 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,7 @@ describe('Cache', () => {
// The new value for d overwrites the old value, since there
// is no custom merge function defined for Query.d.
d: {
e: 4,
h: {
i: 7,
},
Expand Down
6 changes: 4 additions & 2 deletions src/cache/inmemory/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export abstract class EntityStore implements NormalizedCache {
}
}

protected lookup(dataId: string, dependOnExistence?: boolean): StoreObject | undefined {
public lookup = (dataId: string, dependOnExistence?: boolean): StoreObject | undefined => {
// The has method (above) calls lookup with dependOnExistence = true, so
// that it can later be invalidated when we add or remove a StoreObject for
// this dataId. Any consumer who cares about the contents of the StoreObject
Expand All @@ -81,7 +81,7 @@ export abstract class EntityStore implements NormalizedCache {
if (dependOnExistence) this.group.depend(dataId, "__exists");
return hasOwn.call(this.data, dataId) ? this.data[dataId] :
this instanceof Layer ? this.parent.lookup(dataId, dependOnExistence) : void 0;
}
};

public merge(dataId: string, incoming: StoreObject): void {
const existing = this.lookup(dataId);
Expand Down Expand Up @@ -139,6 +139,7 @@ export abstract class EntityStore implements NormalizedCache {
canRead: this.canRead,
toReference: this.toReference,
getFieldValue: this.getFieldValue,
lookup: this.lookup
},
);

Expand Down Expand Up @@ -380,6 +381,7 @@ export abstract class EntityStore implements NormalizedCache {
}

export type FieldValueGetter = EntityStore["getFieldValue"];
export type LookupFunction = EntityStore["lookup"];

// A single CacheGroup represents a set of one or more EntityStore objects,
// typically the Root store in a CacheGroup by itself, and all active Layer
Expand Down
22 changes: 16 additions & 6 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
isFieldValueToBeMerged,
storeValueIsStoreObject,
} from './helpers';
import { FieldValueGetter } from './entityStore';
import { FieldValueGetter, LookupFunction } from './entityStore';
import { InMemoryCache } from './inMemoryCache';
import {
SafeReadonly,
Expand Down Expand Up @@ -558,10 +558,10 @@ export class Policies {
if (isFieldValueToBeMerged(incoming)) {
const field = incoming.__field;
const fieldName = field.name.value;
// This policy and its merge function are guaranteed to exist
// because the incoming value is a FieldValueToBeMerged object.
const { merge } = this.getFieldPolicy(
incoming.__typename, fieldName, false)!;

const defaultMerge:FieldMergeFunction<any, any> = (existing, incoming, { mergeObjects }) => mergeObjects(existing, incoming)
const { merge = defaultMerge } = this.getFieldPolicy(
incoming.__typename, fieldName, false) || {};

// If storage ends up null, that just means no options.storage object
// has ever been created for a read function for this field before, so
Expand Down Expand Up @@ -646,8 +646,17 @@ export class Policies {
}
});

const eObject = isReference(e) ? context.lookup(e.__ref) : e;

if (newFields) {
return { ...i, ...newFields } as typeof incoming;
return { ...eObject, ...i, ...newFields } as typeof incoming;
} else if(eObject && eObject.__typename === i.__typename) {
// There is always the chance the data changed since the last
// time the object was queried but stale data is preferable to an
// infinite loop. If the type changes however it is probably safe
// to only keep the new object. Otherwise merge this object with
// the old one.
return { ...eObject, ...i } as typeof incoming;
}
}

Expand All @@ -660,6 +669,7 @@ export interface ReadMergeContext {
canRead: CanReadFunction;
toReference: ToReferenceFunction;
getFieldValue: FieldValueGetter;
lookup: LookupFunction;
}

function makeFieldFunctionOptions(
Expand Down
1 change: 1 addition & 0 deletions src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export class StoreReader {
toReference: store.toReference,
canRead: store.canRead,
getFieldValue: store.getFieldValue,
lookup: store.lookup,
path: [],
},
});
Expand Down
1 change: 1 addition & 0 deletions src/cache/inmemory/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export declare type IdGetter = (
export interface NormalizedCache {
has(dataId: string): boolean;
get(dataId: string, fieldName: string): StoreValue;
lookup(dataId: string, dependOnExistence?: boolean): StoreObject | undefined;
merge(dataId: string, incoming: StoreObject): void;
modify(dataId: string, fields: Modifiers | Modifier<any>): boolean;
delete(dataId: string, fieldName?: string): boolean;
Expand Down
7 changes: 6 additions & 1 deletion src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { cloneDeep } from '../../utilities/common/cloneDeep';

import { ReadMergeContext } from './policies';
import { NormalizedCache } from './types';
import { makeProcessedFieldsMerger, FieldValueToBeMerged, fieldNameFromStoreName } from './helpers';
import { makeProcessedFieldsMerger, FieldValueToBeMerged, fieldNameFromStoreName, storeValueIsStoreObject } from './helpers';
import { StoreReader } from './readFromStore';
import { InMemoryCache } from './inMemoryCache';

Expand Down Expand Up @@ -115,6 +115,7 @@ export class StoreWriter {
toReference: store.toReference,
canRead: store.canRead,
getFieldValue: store.getFieldValue,
lookup: store.lookup,
},
});

Expand Down Expand Up @@ -221,6 +222,10 @@ export class StoreWriter {
let incomingValue =
this.processFieldValue(value, selection, context, out);

if(storeValueIsStoreObject(incomingValue)) {
// Merge unidentified types
out.shouldApplyMerges = true;
}
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
Expand Down

0 comments on commit d006ce1

Please sign in to comment.