From 94453071d2ac47ae2db4aa1adbded1464bba46d3 Mon Sep 17 00:00:00 2001 From: Andrew Narkewicz Date: Fri, 28 Aug 2020 19:38:13 -0700 Subject: [PATCH 1/4] allow querying of relayConnection metadata without arguments or edges/pageInfo --- .../__tests__/__snapshots__/policies.ts.snap | 32 ++++ src/cache/inmemory/__tests__/policies.ts | 140 ++++++++++++++++++ src/utilities/policies/pagination.ts | 6 +- 3 files changed, 175 insertions(+), 3 deletions(-) diff --git a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap index 216de02d4d7..86747741be2 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap @@ -765,3 +765,35 @@ Object { }, } `; + +exports[`type policies field policies can handle Relay-style pagination without args 1`] = ` +Object { + "ROOT_QUERY": Object { + "__typename": "Query", + "todos": Object { + "edges": Array [ + Object { + "__typename": "TodoEdge", + "cursor": "YXJyYXljb25uZWN0aW9uOjI=", + "node": Object { + "__ref": "Todo:1", + }, + }, + ], + "pageInfo": Object { + "__typename": "PageInfo", + "endCursor": "YXJyYXljb25uZWN0aW9uOjI=", + "hasNextPage": true, + "hasPreviousPage": false, + "startCursor": "YXJyYXljb25uZWN0aW9uOjI=", + }, + "totalCount": 1292, + }, + }, + "Todo:1": Object { + "__typename": "Todo", + "id": "1", + "title": "Fix the tests", + }, +} +`; diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 4ced435b09f..6a20c9608b0 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -2177,6 +2177,146 @@ describe("type policies", function () { }); }); + itAsync("can handle Relay-style pagination without args", (resolve, reject) => { + const cache = new InMemoryCache({ + addTypename: false, + typePolicies: { + Query: { + fields: { + todos: relayStylePagination(), + }, + }, + }, + }); + + const initialQuery = gql` + query TodoQuery { + todos { + totalCount + } + } + ` + + const query = gql` + query TodoQuery { + todos(after: $after, first: $first) { + pageInfo { + __typename + hasNextPage + endCursor + } + totalCount + edges { + __typename + node { + __typename + id + title + } + } + } + } + ` + + const firstVariables = { + first: 1, + }; + + const firstEdges = [ + { + __typename: "TodoEdge", + node: { + __typename: "Todo", + id: '1', + title: 'Fix the tests' + } + }, + ]; + + const firstPageInfo = { + __typename: "PageInfo", + endCursor: "YXJyYXljb25uZWN0aW9uOjI=", + hasNextPage: true, + }; + + const link = new MockLink([ + { + request: { + query: initialQuery, + }, + result: { + data: { + todos: { + totalCount: 1292 + } + } + } + }, + { + request: { + query, + variables: firstVariables, + }, + result: { + data: { + todos: { + edges: firstEdges, + pageInfo: firstPageInfo, + totalCount: 1292, + } + } + }, + }, + ]).setOnError(reject); + + const client = new ApolloClient({ link, cache }); + + client.query({query: initialQuery}).then(result => { + expect(result).toEqual({ + loading: false, + networkStatus: NetworkStatus.ready, + data: { + todos: { + totalCount: 1292 + } + } + }) + + expect(cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + todos: { + edges: [], + pageInfo: { + "endCursor": "", + "hasNextPage": true, + "hasPreviousPage": false, + "startCursor": "", + }, + totalCount: 1292 + }, + } + }); + + client.query({query, variables: firstVariables}).then(result => { + expect(result).toEqual({ + loading: false, + networkStatus: NetworkStatus.ready, + data: { + todos: { + edges: firstEdges, + pageInfo: firstPageInfo, + totalCount: 1292, + } + } + }) + + expect(cache.extract()).toMatchSnapshot() + resolve() + }) + }) + }) + itAsync("can handle Relay-style pagination", (resolve, reject) => { const cache = new InMemoryCache({ addTypename: false, diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index b077f48e564..8f4acb3d2f0 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -79,9 +79,9 @@ export function relayStylePagination( }, merge(existing = makeEmptyData(), incoming, { args }) { - if (!args) return existing; // TODO Maybe throw? + if (!args) return {...existing, ...incoming}; // TODO Maybe throw? - const incomingEdges = incoming.edges.slice(0); + const incomingEdges = (incoming.edges || []).slice(0); if (incoming.pageInfo) { updateCursor(incomingEdges, 0, incoming.pageInfo.startCursor); updateCursor(incomingEdges, -1, incoming.pageInfo.endCursor); @@ -121,7 +121,7 @@ export function relayStylePagination( }; const updatePageInfo = (name: keyof TInternalRelay["pageInfo"]) => { - const value = incoming.pageInfo[name]; + const value = incoming.pageInfo && incoming.pageInfo[name]; if (value !== void 0) { (pageInfo as any)[name] = value; } From 19877d56d3e0e2bea38578ed033e010e4b13ee56 Mon Sep 17 00:00:00 2001 From: Andrew Narkewicz Date: Sat, 29 Aug 2020 07:51:54 -0700 Subject: [PATCH 2/4] do not throw without no args on relay style pagination as it is fine to query a connection without args --- src/utilities/policies/pagination.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index 8f4acb3d2f0..ec86e7f9e3f 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -79,7 +79,7 @@ export function relayStylePagination( }, merge(existing = makeEmptyData(), incoming, { args }) { - if (!args) return {...existing, ...incoming}; // TODO Maybe throw? + if (!args) return {...existing, ...incoming}; const incomingEdges = (incoming.edges || []).slice(0); if (incoming.pageInfo) { From 608e09cf64604dc3f072ac5c657ae1dfb7bbf7eb Mon Sep 17 00:00:00 2001 From: Andrew Narkewicz Date: Mon, 31 Aug 2020 11:17:04 -0700 Subject: [PATCH 3/4] dont overwrite page info when args are not passed to relayStylePagination --- .../__tests__/__snapshots__/policies.ts.snap | 33 ++++++++++ src/cache/inmemory/__tests__/policies.ts | 66 ++++++++++++++----- src/utilities/policies/pagination.ts | 19 +++--- 3 files changed, 93 insertions(+), 25 deletions(-) diff --git a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap index 86747741be2..afba80cb302 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap @@ -797,3 +797,36 @@ Object { }, } `; + +exports[`type policies field policies can handle Relay-style pagination without args 2`] = ` +Object { + "ROOT_QUERY": Object { + "__typename": "Query", + "todos": Object { + "edges": Array [ + Object { + "__typename": "TodoEdge", + "cursor": "YXJyYXljb25uZWN0aW9uOjI=", + "node": Object { + "__ref": "Todo:1", + }, + }, + ], + "extraMetaData": "extra", + "pageInfo": Object { + "__typename": "PageInfo", + "endCursor": "YXJyYXljb25uZWN0aW9uOjI=", + "hasNextPage": true, + "hasPreviousPage": false, + "startCursor": "YXJyYXljb25uZWN0aW9uOjI=", + }, + "totalCount": 1293, + }, + }, + "Todo:1": Object { + "__typename": "Todo", + "id": "1", + "title": "Fix the tests", + }, +} +`; diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 6a20c9608b0..1c75e637635 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -2189,7 +2189,7 @@ describe("type policies", function () { }, }); - const initialQuery = gql` + const firstQuery = gql` query TodoQuery { todos { totalCount @@ -2197,7 +2197,7 @@ describe("type policies", function () { } ` - const query = gql` + const secondQuery = gql` query TodoQuery { todos(after: $after, first: $first) { pageInfo { @@ -2218,11 +2218,20 @@ describe("type policies", function () { } ` - const firstVariables = { + const thirdQuery = gql` + query TodoQuery { + todos { + totalCount + extraMetaData + } + } + ` + + const secondVariables = { first: 1, }; - const firstEdges = [ + const secondEdges = [ { __typename: "TodoEdge", node: { @@ -2233,7 +2242,7 @@ describe("type policies", function () { }, ]; - const firstPageInfo = { + const secondPageInfo = { __typename: "PageInfo", endCursor: "YXJyYXljb25uZWN0aW9uOjI=", hasNextPage: true, @@ -2242,7 +2251,7 @@ describe("type policies", function () { const link = new MockLink([ { request: { - query: initialQuery, + query: firstQuery, }, result: { data: { @@ -2254,24 +2263,37 @@ describe("type policies", function () { }, { request: { - query, - variables: firstVariables, + query: secondQuery, + variables: secondVariables, }, result: { data: { todos: { - edges: firstEdges, - pageInfo: firstPageInfo, + edges: secondEdges, + pageInfo: secondPageInfo, totalCount: 1292, } } }, }, + { + request: { + query: thirdQuery, + }, + result: { + data: { + todos: { + totalCount: 1293, + extraMetaData: 'extra', + } + } + }, + } ]).setOnError(reject); const client = new ApolloClient({ link, cache }); - client.query({query: initialQuery}).then(result => { + client.query({query: firstQuery}).then(result => { expect(result).toEqual({ loading: false, networkStatus: NetworkStatus.ready, @@ -2298,21 +2320,35 @@ describe("type policies", function () { } }); - client.query({query, variables: firstVariables}).then(result => { + client.query({query: secondQuery, variables: secondVariables}).then(result => { expect(result).toEqual({ loading: false, networkStatus: NetworkStatus.ready, data: { todos: { - edges: firstEdges, - pageInfo: firstPageInfo, + edges: secondEdges, + pageInfo: secondPageInfo, totalCount: 1292, } } }) expect(cache.extract()).toMatchSnapshot() - resolve() + + client.query({query: thirdQuery}).then(result => { + expect(result).toEqual({ + loading: false, + networkStatus: NetworkStatus.ready, + data: { + todos: { + totalCount: 1293, + extraMetaData: 'extra', + } + } + }) + expect(cache.extract()).toMatchSnapshot() + resolve() + }) }) }) }) diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index ec86e7f9e3f..51772baf935 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -57,7 +57,7 @@ type TInternalRelay = Readonly<{ // anything about connections, edges, cursors, or pageInfo objects. export function relayStylePagination( keyArgs: KeyArgs = false, -): FieldPolicy> { +): FieldPolicy, Partial>> { return { keyArgs, @@ -79,10 +79,8 @@ export function relayStylePagination( }, merge(existing = makeEmptyData(), incoming, { args }) { - if (!args) return {...existing, ...incoming}; - - const incomingEdges = (incoming.edges || []).slice(0); - if (incoming.pageInfo) { + const incomingEdges = incoming.edges?.slice(0); + if (incoming.pageInfo && incomingEdges) { updateCursor(incomingEdges, 0, incoming.pageInfo.startCursor); updateCursor(incomingEdges, -1, incoming.pageInfo.endCursor); } @@ -90,13 +88,13 @@ export function relayStylePagination( let prefix = existing.edges; let suffix: typeof prefix = []; - if (args.after) { + if (args?.after) { const index = prefix.findIndex(edge => edge.cursor === args.after); if (index >= 0) { prefix = prefix.slice(0, index + 1); // suffix = []; // already true } - } else if (args.before) { + } else if (args?.before) { const index = prefix.findIndex(edge => edge.cursor === args.before); suffix = index < 0 ? prefix : prefix.slice(index); prefix = []; @@ -104,17 +102,18 @@ export function relayStylePagination( // If we have neither args.after nor args.before, the incoming // edges cannot be spliced into the existing edges, so they must // replace the existing edges. See #6592 for a motivating example. - prefix = []; + if(incomingEdges) + prefix = []; } const edges = [ ...prefix, - ...incomingEdges, + ...(incomingEdges || []), ...suffix, ]; const pageInfo = { - ...incoming.pageInfo, + ...(incoming.pageInfo || {}), ...existing.pageInfo, startCursor: cursorFromEdge(edges, 0), endCursor: cursorFromEdge(edges, -1), From 7a6feaeaf0999f540a0007329ced8d1e9da22790 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 9 Sep 2020 15:49:57 -0400 Subject: [PATCH 4/4] Simplify relayStylePagination to avoid optional chaining syntax. --- src/utilities/policies/pagination.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index 51772baf935..951959f3669 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -79,8 +79,8 @@ export function relayStylePagination( }, merge(existing = makeEmptyData(), incoming, { args }) { - const incomingEdges = incoming.edges?.slice(0); - if (incoming.pageInfo && incomingEdges) { + const incomingEdges = incoming.edges ? incoming.edges.slice(0) : []; + if (incoming.pageInfo) { updateCursor(incomingEdges, 0, incoming.pageInfo.startCursor); updateCursor(incomingEdges, -1, incoming.pageInfo.endCursor); } @@ -88,27 +88,26 @@ export function relayStylePagination( let prefix = existing.edges; let suffix: typeof prefix = []; - if (args?.after) { + if (args && args.after) { const index = prefix.findIndex(edge => edge.cursor === args.after); if (index >= 0) { prefix = prefix.slice(0, index + 1); // suffix = []; // already true } - } else if (args?.before) { + } else if (args && args.before) { const index = prefix.findIndex(edge => edge.cursor === args.before); suffix = index < 0 ? prefix : prefix.slice(index); prefix = []; - } else { + } else if (incoming.edges) { // If we have neither args.after nor args.before, the incoming // edges cannot be spliced into the existing edges, so they must // replace the existing edges. See #6592 for a motivating example. - if(incomingEdges) - prefix = []; + prefix = []; } const edges = [ ...prefix, - ...(incomingEdges || []), + ...incomingEdges, ...suffix, ];