Skip to content

Commit

Permalink
Tolerate merge overrides nested inside other merge overrides.
Browse files Browse the repository at this point in the history
Previously, defining a merge function for an ancestor field would prevent
the merge functions of any descendant fields from being called.
  • Loading branch information
benjamn committed Oct 15, 2019
1 parent 3e53b97 commit 88c8ed9
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 25 deletions.
170 changes: 170 additions & 0 deletions src/cache/inmemory/__tests__/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -740,5 +740,175 @@ describe("type policies", function () {
},
});
});

it("runs nested merge functions as well as ancestors", function () {
let eventMergeCount = 0;
let attendeeMergeCount = 0;

const cache = new InMemoryCache({
typePolicies: {
Event: {
fields: {
attendees: {
merge(existing: any[], incoming: any[]) {
++eventMergeCount;
expect(Array.isArray(incoming)).toBe(true);
return existing ? existing.concat(incoming) : incoming;
},
},
},
},

Attendee: {
fields: {
events: {
merge(existing: any[], incoming: any[]) {
++attendeeMergeCount;
expect(Array.isArray(incoming)).toBe(true);
return existing ? existing.concat(incoming) : incoming;
},
},
},
},
},
});

cache.writeQuery({
query: gql`
query {
eventsToday {
name
attendees {
name
events {
time
}
}
}
}
`,
data: {
eventsToday: [{
__typename: "Event",
id: 123,
name: "One-person party",
time: "noonish",
attendees: [{
__typename: "Attendee",
id: 234,
name: "Ben Newman",
events: [
{ __typename: "Event", id: 123 },
],
}],
}],
},
});

expect(eventMergeCount).toBe(1);
expect(attendeeMergeCount).toBe(1);

expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
eventsToday: [
{ __ref: "Event:123" },
],
},
"Event:123": {
__typename: "Event",
name: "One-person party",
attendees: [
{ __ref: "Attendee:234" },
],
},
"Attendee:234": {
__typename: "Attendee",
name: "Ben Newman",
events: [
{ __ref: "Event:123" },
],
},
});

cache.writeQuery({
query: gql`
query {
people {
name
events {
time
attendees {
name
}
}
}
}
`,
data: {
people: [{
__typename: "Attendee",
id: 234,
name: "Ben Newman",
events: [{
__typename: "Event",
id: 345,
name: "Rooftop dog party",
attendees: [{
__typename: "Attendee",
id: 456,
name: "Inspector Beckett",
}, {
__typename: "Attendee",
id: 234,
}],
}],
}],
},
});

expect(eventMergeCount).toBe(2);
expect(attendeeMergeCount).toBe(2);

expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
eventsToday: [
{ __ref: "Event:123" },
],
people: [
{ __ref: "Attendee:234" },
],
},
"Event:123": {
__typename: "Event",
name: "One-person party",
attendees: [
{ __ref: "Attendee:234" },
],
},
"Event:345": {
__typename: "Event",
attendees: [
{ __ref: "Attendee:456" },
{ __ref: "Attendee:234" },
],
},
"Attendee:234": {
__typename: "Attendee",
name: "Ben Newman",
events: [
{ __ref: "Event:123" },
{ __ref: "Event:345" },
],
},
"Attendee:456": {
__typename: "Attendee",
name: "Inspector Beckett",
},
});

expect(cache.gc()).toEqual([]);
});
});
});
45 changes: 20 additions & 25 deletions src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ import { NormalizedCache, StoreObject } from './types';
import { getTypenameFromStoreObject } from './helpers';
import { Policies, StoreValueMergeFunction } from './policies';

const { hasOwnProperty } = Object.prototype;

export type WriteContext = {
readonly store: NormalizedCache;
readonly written: {
Expand All @@ -54,11 +52,10 @@ type StoreObjectMergeFunction = (
overrides?: MergeOverrides,
) => StoreObject;

type MergeOverrides = Record<string | number,
| StoreValueMergeFunction
// TODO This should be replaced with MergeOverrides when TypeScript 3.7+
// starts supporting recursive type aliases.
| Record<string | number, any>>;
type MergeOverrides = Record<string, {
merge?: StoreValueMergeFunction;
child?: MergeOverrides;
}>;

export interface StoreWriterConfig {
policies: Policies;
Expand Down Expand Up @@ -240,10 +237,12 @@ export class StoreWriter {
context.variables,
);

const override = merge || processed.mergeOverrides;
if (override) {
if (merge || processed.mergeOverrides) {
mergeOverrides = mergeOverrides || Object.create(null);
mergeOverrides[storeFieldName] = override;
mergeOverrides[storeFieldName] = context.mergeFields(
mergeOverrides[storeFieldName],
{ merge, child: processed.mergeOverrides },
) as MergeOverrides[string];
}

mergedFields = context.mergeFields(mergedFields, {
Expand Down Expand Up @@ -364,15 +363,16 @@ function walkWithMergeOverrides(
overrides: MergeOverrides,
): void {
Object.keys(overrides).forEach(name => {
const override = overrides[name];
const { merge, child } = overrides[name];
const existingValue: any = existingObject && existingObject[name];
const incomingValue: any = incomingObject && incomingObject[name];
if (typeof override === "function") {
incomingObject[name] = override(existingValue, incomingValue, existingObject);
} else if (override && typeof override === "object") {
// StoreObjects can have multiple layers of nested objects/arrays,
// each layer with its own nested fields and override functions.
walkWithMergeOverrides(existingValue, incomingValue, override);
if (child) {
// StoreObjects can have multiple layers of child objects/arrays,
// each layer with its own child fields and override functions.
walkWithMergeOverrides(existingValue, incomingValue, child);
}
if (merge) {
incomingObject[name] = merge(existingValue, incomingValue, existingObject);
}
});
}
Expand All @@ -394,7 +394,8 @@ const storeObjectReconciler: ReconcilerFunction<[
const existing = existingObject[property];
const incoming = incomingObject[property];

if (mergeOverrides && hasOwnProperty.call(mergeOverrides, property)) {
const mergeChildObj = mergeOverrides && mergeOverrides[property];
if (mergeChildObj && typeof mergeChildObj.merge === "function") {
// If this property was overridden by a custom merge function, then
// its merged value has already been determined, so we should return
// incoming without recursively merging it into existing.
Expand All @@ -421,13 +422,7 @@ const storeObjectReconciler: ReconcilerFunction<[
return incoming;
}

let childMergeOverrides: MergeOverrides;
if (mergeOverrides) {
const child = mergeOverrides[property];
if (typeof child === "object") {
childMergeOverrides = child;
}
}
const childMergeOverrides = mergeChildObj && mergeChildObj.child;

if (isReference(incoming)) {
if (isReference(existing)) {
Expand Down

0 comments on commit 88c8ed9

Please sign in to comment.