Skip to content

Commit

Permalink
Remove FragmentMatcher abstraction from apollo-cache-inmemory.
Browse files Browse the repository at this point in the history
Correctly matching fragments against abstract types like unions or
interfaces requires knowledge of the schema, which can be obtained in
multiple ways.

Previously, we provided two different FragmentMatcher implementations: the
HeuristicFragmentMatcher, which would attempt to match fragments without
any knowledge of the schema; and the IntrospectionFragmentMatcher, which
would consume an introspection query result to obtain information about
possible subtypes.

This commit removes the FragmentMatcher abstraction and both of its
implementations. Instead, you can skip the introspection query step and
simply pass a possibleTypes map to the InMemoryCache constructor:

  new InMemoryCache({
    addTypename: true,
    freezeResults: true,
    possibleTypes: {
      Animal: ["Cat", "Dog", "MiniatureHorse", "Frog"],
      Test: ["PassingTest", "FailingTest", "SkippedTest"],
    },
  })

When you provide this information, assuming it is correct and complete,
the cache does not need to make any heuristic guesses, so there's really
no point in ever having other FragmentMatcher implementations.

More importantly, to maintain backwards compability with the existing
FragmentMatcher interface, the cache was forced to construct temporary
data stores to trick the FragmentMatcher into doing the right thing. This
commit is a breaking change that removes all that awkward logic.

Of course, you have to get the possibleTypes map from somewhere, and
schema introspection is still a fine way of doing that. However, the
possibleTypes map is relatively easy to write by hand, if you're just
prototyping/experimenting. Once you're ready to generate it automatically
from your schema, it can be easily imported from a generated module.

If the possibleTypes option is not specified, the cache will behave like
the old HeuristicFragmentMatcher, which is adequate as long as you're not
using fragments with abstract constraints.
  • Loading branch information
benjamn committed Jul 18, 2019
1 parent c2a67db commit 450107a
Show file tree
Hide file tree
Showing 19 changed files with 276 additions and 677 deletions.
15 changes: 6 additions & 9 deletions packages/apollo-cache-inmemory/src/__tests__/diffAgainstStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ import { toIdValue } from 'apollo-utilities';
import { defaultNormalizedCacheFactory } from '../objectCache';
import { StoreReader } from '../readFromStore';
import { StoreWriter } from '../writeToStore';
import { HeuristicFragmentMatcher } from '../fragmentMatcher';
import { defaultDataIdFromObject } from '../inMemoryCache';
import { NormalizedCache } from '../types';

const fragmentMatcherFunction = new HeuristicFragmentMatcher().match;

disableFragmentWarnings();
export function withError(func: Function, regex: RegExp) {

export function withError(func: Function, regex?: RegExp) {
let message: string = null as never;
const { error } = console;
console.error = (m: any) => {
Expand All @@ -20,7 +18,9 @@ export function withError(func: Function, regex: RegExp) {

try {
const result = func();
expect(message).toMatch(regex);
if (regex) {
expect(message).toMatch(regex);
}
return result;
} finally {
console.error = error;
Expand Down Expand Up @@ -53,7 +53,6 @@ describe('diffing queries against the store', () => {
}
}
`,
fragmentMatcherFunction,
config: {
dataIdFromObject: defaultDataIdFromObject,
},
Expand Down Expand Up @@ -99,7 +98,6 @@ describe('diffing queries against the store', () => {
}
}
`,
fragmentMatcherFunction,
config: {
dataIdFromObject: defaultDataIdFromObject,
},
Expand Down Expand Up @@ -253,11 +251,10 @@ describe('diffing queries against the store', () => {
store,
query: unionQuery,
returnPartialData: false,
fragmentMatcherFunction,
});

expect(complete).toBe(false);
}, /IntrospectionFragmentMatcher/);
});
});

it('does not error on a query with fields missing from all but one named fragment', () => {
Expand Down
191 changes: 133 additions & 58 deletions packages/apollo-cache-inmemory/src/__tests__/fragmentMatcher.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,139 @@
import { IntrospectionFragmentMatcher } from '../fragmentMatcher';
import { defaultNormalizedCacheFactory } from '../objectCache';
import { ReadStoreContext } from '../types';
import { InMemoryCache } from '../inMemoryCache';
import gql from 'graphql-tag';

describe('FragmentMatcher', () => {
describe('fragment matching', () => {
it('can match exact types with or without possibleTypes', () => {
const cacheWithoutPossibleTypes = new InMemoryCache({
addTypename: true,
});

const cacheWithPossibleTypes = new InMemoryCache({
addTypename: true,
possibleTypes: {
Animal: ['Cat', 'Dog'],
},
});

const query = gql`
query AnimalNames {
animals {
id
name
...CatDetails
}
}
fragment CatDetails on Cat {
livesLeft
killsToday
}
`;

const data = {
animals: [
{
__typename: 'Cat',
id: 1,
name: 'Felix',
livesLeft: 8,
killsToday: 2,
},
{
__typename: 'Dog',
id: 2,
name: 'Baxter',
},
],
};

cacheWithoutPossibleTypes.writeQuery({ query, data });
expect(cacheWithoutPossibleTypes.readQuery({ query })).toEqual(data);

cacheWithPossibleTypes.writeQuery({ query, data });
expect(cacheWithPossibleTypes.readQuery({ query })).toEqual(data);
});

it('can match interface subtypes', () => {
const cache = new InMemoryCache({
addTypename: true,
possibleTypes: {
Animal: ['Cat', 'Dog'],
},
});

const query = gql`
query BestFriend {
bestFriend {
id
...AnimalName
}
}
fragment AnimalName on Animal {
name
}
`;

const data = {
bestFriend: {
__typename: 'Dog',
id: 2,
name: 'Beckett',
},
};

cache.writeQuery({ query, data });
expect(cache.readQuery({ query })).toEqual(data);
});

it('can match union member types', () => {
const cache = new InMemoryCache({
addTypename: true,
possibleTypes: {
Status: ['PASSING', 'FAILING', 'SKIPPED'],
},
});

const query = gql`
query {
testResults {
id
output {
... on Status {
stdout
}
... on FAILING {
stderr
}
}
}
}
`;

const data = {
testResults: [
{
__typename: 'TestResult',
id: 123,
output: {
__typename: 'PASSING',
stdout: 'ok!',
},
},
{
__typename: 'TestResult',
id: 456,
output: {
__typename: 'FAILING',
stdout: '',
stderr: 'oh no',
},
},
],
};

cache.writeQuery({ query, data });
expect(cache.readQuery({ query })).toEqual(data);
});

it('can match against the root Query', () => {
const cache = new InMemoryCache({
addTypename: true,
Expand Down Expand Up @@ -45,57 +174,3 @@ describe('FragmentMatcher', () => {
expect(cache.readQuery({ query })).toEqual(data);
});
});

describe('IntrospectionFragmentMatcher', () => {
it('will throw an error if match is called if it is not ready', () => {
const ifm = new IntrospectionFragmentMatcher();
expect(() => (ifm.match as any)()).toThrowError(/called before/);
});

it('can be seeded with an introspection query result', () => {
const ifm = new IntrospectionFragmentMatcher({
introspectionQueryResultData: {
__schema: {
types: [
{
kind: 'UNION',
name: 'Item',
possibleTypes: [
{
name: 'ItemA',
},
{
name: 'ItemB',
},
],
},
],
},
},
});

const store = defaultNormalizedCacheFactory({
a: {
__typename: 'ItemB',
},
});

const idValue = {
type: 'id',
id: 'a',
generated: false,
};

const readStoreContext = {
store,
returnPartialData: false,
hasMissingField: false,
cacheRedirects: {},
} as ReadStoreContext;

expect(ifm.match(idValue as any, 'Item', readStoreContext)).toBe(true);
expect(ifm.match(idValue as any, 'NotAnItem', readStoreContext)).toBe(
false,
);
});
});
7 changes: 2 additions & 5 deletions packages/apollo-cache-inmemory/src/__tests__/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ import { IdValue, JsonValue } from 'apollo-utilities';
import gql from 'graphql-tag';
import { stripSymbols } from 'apollo-utilities';

import { StoreObject, HeuristicFragmentMatcher } from '../';
import { StoreObject } from '../';
import { StoreReader } from '../readFromStore';
import { defaultNormalizedCacheFactory } from '../objectCache';

const fragmentMatcherFunction = new HeuristicFragmentMatcher().match;
import { withError } from './diffAgainstStore';

describe('reading from the store', () => {
Expand Down Expand Up @@ -62,15 +60,14 @@ describe('reading from the store', () => {
}
}
`,
fragmentMatcherFunction,
});

expect(stripSymbols(queryResult)).toEqual({
nestedObj: {
innerArray: [{ id: 'abcdef', someField: 3 }],
},
});
}, /queries contain union or interface types/);
});
});

it('rejects malformed queries', () => {
Expand Down
13 changes: 5 additions & 8 deletions packages/apollo-cache-inmemory/src/__tests__/roundtrip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import { withWarning } from './writeToStore';

import { DepTrackingCache } from '../depTrackingCache';

import { HeuristicFragmentMatcher, StoreReader, StoreWriter } from '../';

const fragmentMatcherFunction = new HeuristicFragmentMatcher().match;
import { StoreReader, StoreWriter } from '../';

function assertDeeplyFrozen(value: any, stack: any[] = []) {
if (value !== null && typeof value === 'object' && stack.indexOf(value) < 0) {
Expand Down Expand Up @@ -37,7 +35,6 @@ function storeRoundtrip(query: DocumentNode, result: any, variables = {}) {
store,
query,
variables,
fragmentMatcherFunction,
};

const reconstructedResult = reader.readQueryFromStore<any>(readOptions);
Expand Down Expand Up @@ -335,7 +332,7 @@ describe('roundtrip', () => {
],
},
);
}, /using fragments/);
});
});

// XXX this test is weird because it assumes the server returned an incorrect result
Expand Down Expand Up @@ -398,7 +395,7 @@ describe('roundtrip', () => {
],
},
);
}, /IntrospectionFragmentMatcher/);
});
});

it('should resolve on union types with spread fragments', () => {
Expand Down Expand Up @@ -437,7 +434,7 @@ describe('roundtrip', () => {
],
},
);
}, /IntrospectionFragmentMatcher/);
});
});

it('should work with a fragment on the actual interface or union', () => {
Expand Down Expand Up @@ -476,7 +473,7 @@ describe('roundtrip', () => {
],
},
);
}, /IntrospectionFragmentMatcher/);
});
});

it('should throw on error on two of the same spread fragment types', () => {
Expand Down
Loading

0 comments on commit 450107a

Please sign in to comment.