From 27c7096316023cdc127c59a2a3b2a908b2ff3bd0 Mon Sep 17 00:00:00 2001 From: Beau Collins Date: Fri, 23 Oct 2020 12:14:38 -0700 Subject: [PATCH 1/5] test for cursors with empty edges --- .../__tests__/relayStylePagination.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 src/utilities/policies/__tests__/relayStylePagination.test.ts diff --git a/src/utilities/policies/__tests__/relayStylePagination.test.ts b/src/utilities/policies/__tests__/relayStylePagination.test.ts new file mode 100644 index 00000000000..ef1a3e7f20e --- /dev/null +++ b/src/utilities/policies/__tests__/relayStylePagination.test.ts @@ -0,0 +1,50 @@ +import { FieldFunctionOptions, InMemoryCache, isReference } from '../../../core'; +import { relayStylePagination } from '../pagination'; + +describe('relayStylePagination', () => { + const policy = relayStylePagination(); + + describe('merge', () => { + + const merge = policy.merge; + // The merge function should exist, make TS aware + if (merge == null || typeof merge === 'boolean') { + throw new Error('Expecting merge function'); + } + + const options: FieldFunctionOptions = { + args: null, + fieldName: 'fake', + storeFieldName: 'fake', + field: null, + isReference: isReference, + toReference: () => undefined, + storage: {}, + cache: new InMemoryCache(), + readField: () => undefined, + canRead: () => false, + mergeObjects: (existing, _incoming) => existing, + } + it('should maintain endCursor and startCursor with empty edges', () => { + const incoming: Parameters[1] = { + pageInfo: { + hasPreviousPage: false, + hasNextPage: true, + startCursor: 'abc', + endCursor: 'xyz', + } + } + const result = merge(undefined, incoming, options); + expect(result).toEqual({ + edges: [], + pageInfo: { + hasPreviousPage: false, + hasNextPage: true, + startCursor: 'abc', + endCursor: 'xyz' + } + }) + }) + + }) +}); From 092a1390ebe3144d8f493045ae05caf4d619f265 Mon Sep 17 00:00:00 2001 From: Beau Collins Date: Thu, 22 Oct 2020 18:36:00 -0700 Subject: [PATCH 2/5] Use the pageInfo cursors in the case the edges are missing --- src/utilities/policies/pagination.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index 8b7d8a23959..ab51db011a0 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -184,8 +184,8 @@ export function relayStylePagination( const pageInfo: TPageInfo = { ...incoming.pageInfo, ...existing.pageInfo, - startCursor: firstEdge && firstEdge.cursor || "", - endCursor: lastEdge && lastEdge.cursor || "", + startCursor: firstEdge?.cursor ?? incoming.pageInfo?.startCursor ?? '', + endCursor: lastEdge?.cursor ?? incoming.pageInfo?.endCursor ?? '', }; if (incoming.pageInfo) { From bb41d0f44904025756d331eafecfcc38f1fccbbc Mon Sep 17 00:00:00 2001 From: Beau Collins Date: Fri, 23 Oct 2020 12:30:17 -0700 Subject: [PATCH 3/5] code style changes --- .../policies/__tests__/relayStylePagination.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/utilities/policies/__tests__/relayStylePagination.test.ts b/src/utilities/policies/__tests__/relayStylePagination.test.ts index ef1a3e7f20e..fe583f9de29 100644 --- a/src/utilities/policies/__tests__/relayStylePagination.test.ts +++ b/src/utilities/policies/__tests__/relayStylePagination.test.ts @@ -8,7 +8,7 @@ describe('relayStylePagination', () => { const merge = policy.merge; // The merge function should exist, make TS aware - if (merge == null || typeof merge === 'boolean') { + if (typeof merge !== 'function') { throw new Error('Expecting merge function'); } @@ -24,7 +24,7 @@ describe('relayStylePagination', () => { readField: () => undefined, canRead: () => false, mergeObjects: (existing, _incoming) => existing, - } + }; it('should maintain endCursor and startCursor with empty edges', () => { const incoming: Parameters[1] = { pageInfo: { @@ -33,7 +33,7 @@ describe('relayStylePagination', () => { startCursor: 'abc', endCursor: 'xyz', } - } + }; const result = merge(undefined, incoming, options); expect(result).toEqual({ edges: [], @@ -43,8 +43,7 @@ describe('relayStylePagination', () => { startCursor: 'abc', endCursor: 'xyz' } - }) - }) - + }); + }); }) }); From 75ae477f9b1c3cbfefebf1f0bbed95b9824405b3 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 27 Oct 2020 16:42:36 -0400 Subject: [PATCH 4/5] Ensure incoming.pageInfo.{start,end}Cursor defined, if possible. --- src/utilities/policies/pagination.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index ab51db011a0..fb8b823597b 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -136,17 +136,28 @@ export function relayStylePagination( }) : []; if (incoming.pageInfo) { - // In case we did not request the cursor field for edges in this - // query, we can still infer some of those cursors from pageInfo. - const { startCursor, endCursor } = incoming.pageInfo; + const { pageInfo } = incoming; + const { startCursor, endCursor } = pageInfo; const firstEdge = incomingEdges[0]; + const lastEdge = incomingEdges[incomingEdges.length - 1]; + // In case we did not request the cursor field for edges in this + // query, we can still infer cursors from pageInfo. if (firstEdge && startCursor) { firstEdge.cursor = startCursor; } - const lastEdge = incomingEdges[incomingEdges.length - 1]; if (lastEdge && endCursor) { lastEdge.cursor = endCursor; } + // Cursors can also come from edges, so we default + // pageInfo.{start,end}Cursor to {first,last}Edge.cursor. + const firstCursor = firstEdge && firstEdge.cursor; + if (firstCursor && !startCursor) { + pageInfo.startCursor = firstCursor; + } + const lastCursor = lastEdge && lastEdge.cursor; + if (lastCursor && !endCursor) { + pageInfo.endCursor = lastCursor; + } } let prefix = existing.edges; From dd2a54dc0c681cd6ec15cc1ea415639d4d8db235 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 27 Oct 2020 15:56:49 -0400 Subject: [PATCH 5/5] Update pageInfo.{start,end}Cursor more like has{Previous,Next}Page. --- .../__tests__/relayStylePagination.test.ts | 54 ++++++++++++++++++- src/utilities/policies/pagination.ts | 30 +++++++---- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/utilities/policies/__tests__/relayStylePagination.test.ts b/src/utilities/policies/__tests__/relayStylePagination.test.ts index fe583f9de29..b8a27dde9a8 100644 --- a/src/utilities/policies/__tests__/relayStylePagination.test.ts +++ b/src/utilities/policies/__tests__/relayStylePagination.test.ts @@ -1,11 +1,10 @@ -import { FieldFunctionOptions, InMemoryCache, isReference } from '../../../core'; +import { FieldFunctionOptions, InMemoryCache, isReference, makeReference } from '../../../core'; import { relayStylePagination } from '../pagination'; describe('relayStylePagination', () => { const policy = relayStylePagination(); describe('merge', () => { - const merge = policy.merge; // The merge function should exist, make TS aware if (typeof merge !== 'function') { @@ -25,6 +24,7 @@ describe('relayStylePagination', () => { canRead: () => false, mergeObjects: (existing, _incoming) => existing, }; + it('should maintain endCursor and startCursor with empty edges', () => { const incoming: Parameters[1] = { pageInfo: { @@ -45,5 +45,55 @@ describe('relayStylePagination', () => { } }); }); + + it('should maintain existing PageInfo when adding a page', () => { + const existingEdges = [ + { cursor: 'alpha', node: makeReference("fakeAlpha") }, + ]; + + const incomingEdges = [ + { cursor: 'omega', node: makeReference("fakeOmega") }, + ]; + + const result = merge( + { + edges: existingEdges, + pageInfo: { + hasPreviousPage: false, + hasNextPage: true, + startCursor: 'alpha', + endCursor: 'alpha' + }, + }, + { + edges: incomingEdges, + pageInfo: { + hasPreviousPage: true, + hasNextPage: true, + startCursor: incomingEdges[0].cursor, + endCursor: incomingEdges[incomingEdges.length - 1].cursor, + }, + }, + { + ...options, + args: { + after: 'alpha', + }, + }, + ); + + expect(result).toEqual({ + edges: [ + ...existingEdges, + ...incomingEdges, + ], + pageInfo: { + hasPreviousPage: false, + hasNextPage: true, + startCursor: 'alpha', + endCursor: 'omega', + } + }); + }); }) }); diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index fb8b823597b..380e8efa57a 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -189,27 +189,35 @@ export function relayStylePagination( ...suffix, ]; - const firstEdge = edges[0]; - const lastEdge = edges[edges.length - 1]; - const pageInfo: TPageInfo = { + // The ordering of these two ...spreads may be surprising, but it + // makes sense because we want to combine PageInfo properties with a + // preference for existing values, *unless* the existing values are + // overridden by the logic below, which is permitted only when the + // incoming page falls at the beginning or end of the data. ...incoming.pageInfo, ...existing.pageInfo, - startCursor: firstEdge?.cursor ?? incoming.pageInfo?.startCursor ?? '', - endCursor: lastEdge?.cursor ?? incoming.pageInfo?.endCursor ?? '', }; if (incoming.pageInfo) { - const { hasPreviousPage, hasNextPage } = incoming.pageInfo; + const { + hasPreviousPage, hasNextPage, + startCursor, endCursor, + } = incoming.pageInfo; // Keep existing.pageInfo.has{Previous,Next}Page unless the // placement of the incoming edges means incoming.hasPreviousPage // or incoming.hasNextPage should become the new values for those - // properties in existing.pageInfo. - if (!prefix.length && hasPreviousPage !== void 0) { - pageInfo.hasPreviousPage = hasPreviousPage; + // properties in existing.pageInfo. Note that these updates are + // only permitted when the beginning or end of the incoming page + // coincides with the beginning or end of the existing data, as + // determined using prefix.length and suffix.length. + if (!prefix.length) { + if (void 0 !== hasPreviousPage) pageInfo.hasPreviousPage = hasPreviousPage; + if (void 0 !== startCursor) pageInfo.startCursor = startCursor; } - if (!suffix.length && hasNextPage !== void 0) { - pageInfo.hasNextPage = hasNextPage; + if (!suffix.length) { + if (void 0 !== hasNextPage) pageInfo.hasNextPage = hasNextPage; + if (void 0 !== endCursor) pageInfo.endCursor = endCursor; } }