Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(graphcache) - Change Store constructor to accept options object #622

Merged
merged 4 commits into from
Mar 16, 2020
Merged

(graphcache) - Change Store constructor to accept options object #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
72d2b54
Update Store constructor to accept options
kitten Mar 16, 2020
fe2abcd
Adjust tests for change
kitten Mar 16, 2020
f449515
Move null-ish check for opts in cacheExchange
kitten Mar 16, 2020
17d88df
Add changeset
kitten Mar 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fresh-cars-sip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/exchange-graphcache': patch
---

Improve Store constructor to accept an options object instead of separate arguments, identical to the cacheExchange options. (This is a patch, not a minor, since we consider Store part of the private API)
12 changes: 2 additions & 10 deletions exchanges/graphcache/src/cacheExchange.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,10 @@ export const cacheExchange = (opts?: CacheExchangeOpts): Exchange => ({
forward,
client,
}) => {
if (!opts) opts = {};
Copy link
Contributor

@andyrichardson andyrichardson Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout on removing this!

Default params also save us some conditional logic:

opts?: CacheExchangeOpts = {}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, I've just moved it 😆 but typically transpilers will jump through more hoops in the generated code with default params rather than reassigning the argument. It's a minor details though, since we do get the type safety from TypeScript in either cases 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default params are very expensive size wise, see this REPL


const store = new Store(
opts.schema,
opts.resolvers,
opts.updates,
opts.optimistic,
opts.keys
);
const store = new Store(opts);

let hydration: void | Promise<void>;
if (opts.storage) {
if (opts && opts.storage) {
hydration = opts.storage.read().then(entries => {
hydrateData(store.data, opts!.storage!, entries);
});
Expand Down
112 changes: 65 additions & 47 deletions exchanges/graphcache/src/extras/relayPagination.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ it('works with forward pagination', () => {
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down Expand Up @@ -102,9 +104,11 @@ it('works with backwards pagination', () => {
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down Expand Up @@ -170,9 +174,11 @@ it('handles duplicate edges', () => {
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down Expand Up @@ -244,9 +250,11 @@ it('works with simultaneous forward and backward pagination (outwards merging)',
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination({ mergeMode: 'outwards' }),
const store = new Store({
resolvers: {
Query: {
items: relayPagination({ mergeMode: 'outwards' }),
},
},
});

Expand Down Expand Up @@ -360,9 +368,11 @@ it('works with simultaneous forward and backward pagination (inwards merging)',
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination({ mergeMode: 'inwards' }),
const store = new Store({
resolvers: {
Query: {
items: relayPagination({ mergeMode: 'inwards' }),
},
},
});

Expand Down Expand Up @@ -474,9 +484,11 @@ it('prevents overlapping of pagination on different arguments', () => {
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down Expand Up @@ -544,9 +556,11 @@ it('returns an empty array of edges when the cache has zero edges stored', () =>
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down Expand Up @@ -581,9 +595,11 @@ it('returns other fields on the same level as the edges', () => {
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down Expand Up @@ -632,14 +648,14 @@ it('returns a subset of the cached items if the query requests less items than t
}
`;

const store = new Store(
require('../test-utils/relayPagination_schema.json'),
{
const store = new Store({
schema: require('../test-utils/relayPagination_schema.json'),
resolvers: {
Query: {
items: relayPagination({ mergeMode: 'outwards' }),
},
}
);
},
});

const results = {
__typename: 'Query',
Expand Down Expand Up @@ -690,14 +706,14 @@ it("returns the cached items even if they don't fullfil the query", () => {
}
`;

const store = new Store(
require('../test-utils/relayPagination_schema.json'),
{
const store = new Store({
schema: require('../test-utils/relayPagination_schema.json'),
resolvers: {
Query: {
items: relayPagination(),
},
}
);
},
});

const results = {
__typename: 'Query',
Expand Down Expand Up @@ -752,14 +768,14 @@ it('returns the cached items even when they come from a different query', () =>
}
`;

const store = new Store(
require('../test-utils/relayPagination_schema.json'),
{
const store = new Store({
schema: require('../test-utils/relayPagination_schema.json'),
resolvers: {
Query: {
items: relayPagination(),
},
}
);
},
});

const results = {
__typename: 'Query',
Expand Down Expand Up @@ -810,14 +826,14 @@ it('caches and retrieves correctly queries with inwards pagination', () => {
}
`;

const store = new Store(
require('../test-utils/relayPagination_schema.json'),
{
const store = new Store({
schema: require('../test-utils/relayPagination_schema.json'),
resolvers: {
Query: {
items: relayPagination(),
},
}
);
},
});

const results = {
__typename: 'Query',
Expand Down Expand Up @@ -872,9 +888,11 @@ it('does not include a previous result when adding parameters', () => {
}
`;

const store = new Store(undefined, {
Query: {
items: relayPagination(),
const store = new Store({
resolvers: {
Query: {
items: relayPagination(),
},
},
});

Expand Down
40 changes: 25 additions & 15 deletions exchanges/graphcache/src/extras/simplePagination.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ it('works with simple pagination', () => {
}
`;

const store = new Store(undefined, {
Query: {
persons: simplePagination(),
const store = new Store({
resolvers: {
Query: {
persons: simplePagination(),
},
},
});

Expand Down Expand Up @@ -82,9 +84,11 @@ it('handles duplicates', () => {
}
`;

const store = new Store(undefined, {
Query: {
persons: simplePagination(),
const store = new Store({
resolvers: {
Query: {
persons: simplePagination(),
},
},
});

Expand Down Expand Up @@ -138,9 +142,11 @@ it('should not return previous result when adding a parameter', () => {
}
`;

const store = new Store(undefined, {
Query: {
persons: simplePagination(),
const store = new Store({
resolvers: {
Query: {
persons: simplePagination(),
},
},
});

Expand Down Expand Up @@ -187,9 +193,11 @@ it('should preserve the correct order', () => {
}
`;

const store = new Store(undefined, {
Query: {
persons: simplePagination(),
const store = new Store({
resolvers: {
Query: {
persons: simplePagination(),
},
},
});

Expand Down Expand Up @@ -243,9 +251,11 @@ it('prevents overlapping of pagination on different arguments', () => {
}
`;

const store = new Store(undefined, {
Query: {
persons: simplePagination(),
const store = new Store({
resolvers: {
Query: {
persons: simplePagination(),
},
},
});

Expand Down
2 changes: 1 addition & 1 deletion exchanges/graphcache/src/operations/invalidate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Query', () => {
});

beforeEach(() => {
store = new Store(schema);
store = new Store({ schema });
write(
store,
{ query: TODO_QUERY },
Expand Down
6 changes: 3 additions & 3 deletions exchanges/graphcache/src/operations/query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Query', () => {
});

beforeEach(() => {
store = new Store(schema);
store = new Store({ schema });
write(
store,
{ query: TODO_QUERY },
Expand Down Expand Up @@ -120,7 +120,7 @@ describe('Query', () => {
}
`;
// Use new store to ensure bug reproduction
const store = new Store(schema);
const store = new Store({ schema });

let { data } = query(store, { query: VALID_QUERY });
expect(data).toEqual(null);
Expand Down Expand Up @@ -151,7 +151,7 @@ describe('Query', () => {
}
`;

const store = new Store(alteredRoot);
const store = new Store({ schema: alteredRoot });

let { data } = query(store, { query: QUERY });
expect(data).toEqual(null);
Expand Down
2 changes: 1 addition & 1 deletion exchanges/graphcache/src/operations/write.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Query', () => {
});

beforeEach(() => {
store = new Store(schema);
store = new Store({ schema });
write(
store,
{ query: TODO_QUERY },
Expand Down
20 changes: 12 additions & 8 deletions exchanges/graphcache/src/store/store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ const Todos = gql`

describe('Store with KeyingConfig', () => {
it('generates keys from custom keying function', () => {
const store = new Store(undefined, undefined, undefined, undefined, {
User: () => 'me',
None: () => null,
const store = new Store({
keys: {
User: () => 'me',
None: () => null,
},
});

expect(store.keyOfEntity({ __typename: 'Any', id: '123' })).toBe('Any:123');
Expand All @@ -53,11 +55,13 @@ describe('Store with OptimisticMutationConfig', () => {
let store, todosData;

beforeEach(() => {
store = new Store(undefined, undefined, undefined, {
addTodo: variables => {
return {
...variables,
} as Data;
store = new Store({
optimistic: {
addTodo: variables => {
return {
...variables,
} as Data;
},
},
});
todosData = {
Expand Down
Loading