From f816070036b2ee5064fb190be22759e12bee21b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Lach=C3=A8ze?= Date: Tue, 14 Dec 2021 22:27:54 -0500 Subject: [PATCH 1/3] Fix relayStylePagination for existing and incoming without edges property --- .../__tests__/relayStylePagination.test.ts | 100 +++++++++- src/utilities/policies/pagination.ts | 184 +++++++++--------- 2 files changed, 193 insertions(+), 91 deletions(-) diff --git a/src/utilities/policies/__tests__/relayStylePagination.test.ts b/src/utilities/policies/__tests__/relayStylePagination.test.ts index a2e19321bd4..b9021eaf250 100644 --- a/src/utilities/policies/__tests__/relayStylePagination.test.ts +++ b/src/utilities/policies/__tests__/relayStylePagination.test.ts @@ -109,8 +109,21 @@ describe('relayStylePagination', () => { hasNextPage: true, }); }); + + it("should not return empty edges if none existing", () => { + const resultWithTotalCount = policy.read!({ + totalCount: 10 + }, fakeReadOptions); + + expect( + resultWithTotalCount, + ).toEqual({ + totalCount: 10, + }); + }); }); + describe('merge', () => { const merge = policy.merge; // The merge function should exist, make TS aware @@ -143,7 +156,6 @@ describe('relayStylePagination', () => { }; const result = merge(undefined, incoming, options); expect(result).toEqual({ - edges: [], pageInfo: { hasPreviousPage: false, hasNextPage: true, @@ -236,6 +248,92 @@ describe('relayStylePagination', () => { expect(result).toEqual(fakeExisting); }) + describe('when incoming has no edges', () => { + it('should not replace existing null with empty edges', () => { + const fakeExisting = null; + + const fakeIncoming = { + totalCount: 10 + }; + + const fakeOptions = { + ...options, + }; + + const result = merge( + fakeExisting, + fakeIncoming, + fakeOptions, + ); + + expect(result).toEqual({ + totalCount: 10 + }); + }) + + it('should not merge existing with empty edges', () => { + const fakeExisting = { + totalCount: 10 + }; + + const fakeIncoming = { + totalCount: 11 + }; + + const fakeOptions = { + ...options, + args: { + after: 'alpha', + }, + }; + + const result = merge( + fakeExisting, + fakeIncoming, + fakeOptions, + ); + + expect(result).toEqual({ + totalCount: 11 + }); + }) + }) + + describe('when existing has no edges', () => { + it('should add incoming edges', () => { + const fakeExisting = { + totalCount: 10 + }; + + const incomingEdges = [ + { cursor: 'alpha', node: makeReference("fakeAlpha") }, + ]; + const incoming = { + edges: incomingEdges, + pageInfo: { + hasPreviousPage: false, + hasNextPage: true, + startCursor: 'alpha', + endCursor: 'alpha' + }, + }; + const fakeOptions = { + ...options, + }; + + const result = merge( + fakeExisting, + incoming, + fakeOptions, + ); + + expect(result).toEqual({ + ...incoming, + totalCount: 10 + }); + }) + }) + it('should replace existing null with incoming', () => { const incomingEdges = [ { cursor: 'alpha', node: makeReference("fakeAlpha") }, diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index 27a9b97bea4..e55fc9c056a 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -1,6 +1,7 @@ import { __rest } from "tslib"; import { FieldPolicy, Reference } from '../../cache'; +import { SafeReadonly } from "../../cache/core/types/common"; import { mergeDeep } from '../common/mergeDeep'; type KeyArgs = FieldPolicy["keyArgs"]; @@ -69,91 +70,106 @@ export type TRelayPageInfo = { endCursor: string; }; -export type TExistingRelay = Readonly<{ +export type TRelayConnection = { edges: TRelayEdge[]; pageInfo: TRelayPageInfo; -}>; +} & TConnectionExtraData -export type TIncomingRelay = { - edges?: TRelayEdge[]; - pageInfo?: TRelayPageInfo; -}; +export type TExistingRelay = Readonly>>; +export type TIncomingRelay = Readonly>>; -export type RelayFieldPolicy = FieldPolicy< - TExistingRelay | null, - TIncomingRelay | null, - TIncomingRelay | null +export type RelayFieldPolicy = FieldPolicy< + TExistingRelay | null, + TIncomingRelay | null, + TIncomingRelay | null >; // As proof of the flexibility of field policies, this function generates // one that handles Relay-style pagination, without Apollo Client knowing // anything about connections, edges, cursors, or pageInfo objects. -export function relayStylePagination( +export function relayStylePagination( keyArgs: KeyArgs = false, -): RelayFieldPolicy { +): RelayFieldPolicy { return { keyArgs, read(existing, { canRead, readField }) { if (!existing) return existing; - const edges: TRelayEdge[] = []; - let firstEdgeCursor = ""; - let lastEdgeCursor = ""; - existing.edges.forEach(edge => { - // Edges themselves could be Reference objects, so it's important - // to use readField to access the edge.edge.node property. - if (canRead(readField("node", edge))) { - edges.push(edge); - if (edge.cursor) { - firstEdgeCursor = firstEdgeCursor || edge.cursor || ""; - lastEdgeCursor = edge.cursor || lastEdgeCursor; - } - } - }); - - const { - startCursor, - endCursor, - } = existing.pageInfo || {}; - - return { + let read: TIncomingRelay = { // Some implementations return additional Connection fields, such // as existing.totalCount. These fields are saved by the merge // function, so the read function should also preserve them. - ...getExtras(existing), - edges, - pageInfo: { - ...existing.pageInfo, - // If existing.pageInfo.{start,end}Cursor are undefined or "", default - // to firstEdgeCursor and/or lastEdgeCursor. - startCursor: startCursor || firstEdgeCursor, - endCursor: endCursor || lastEdgeCursor, - }, - }; + ...getExtras(existing) + } + + if (existing.pageInfo) { + read = { ...read, pageInfo: existing.pageInfo} + } + + if (existing.edges) { + const edges: TRelayEdge[] = []; + let firstEdgeCursor = ""; + let lastEdgeCursor = ""; + existing.edges.forEach(edge => { + // Edges themselves could be Reference objects, so it's important + // to use readField to access the edge.edge.node property. + if (canRead(readField("node", edge))) { + edges.push(edge); + if (edge.cursor) { + firstEdgeCursor = firstEdgeCursor || edge.cursor || ""; + lastEdgeCursor = edge.cursor || lastEdgeCursor; + } + } + }); + + const { + startCursor, + endCursor, + } = existing?.pageInfo ?? { startCursor: null, endCursor: null}; + + read = { + ...read, + edges, + pageInfo: { + ...existing.pageInfo, + // If existing.pageInfo.{start,end}Cursor are undefined or "", default + // to firstEdgeCursor and/or lastEdgeCursor. + startCursor: startCursor || firstEdgeCursor, + endCursor: endCursor || lastEdgeCursor, + }, + } + } + + return read }, merge(existing, incoming, { args, isReference, readField }) { - if (!existing) { - existing = makeEmptyData(); + if (!incoming) { + return existing ?? null; } - if (!incoming) { - return existing; + let merged: SafeReadonly> = { + ...(existing || {}), + ...incoming, + } + + if (!incoming.edges) { + return merged } - const incomingEdges = incoming.edges ? incoming.edges.map(edge => { + const incomingEdges = incoming.edges.map(edge => { if (isReference(edge = { ...edge })) { // In case edge is a Reference, we read out its cursor field and // store it as an extra property of the Reference object. edge.cursor = readField("cursor", edge); } return edge; - }) : []; + }); if (incoming.pageInfo) { const { pageInfo } = incoming; - const { startCursor, endCursor } = pageInfo; + 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 @@ -184,7 +200,7 @@ export function relayStylePagination( } } - let prefix = existing.edges; + let prefix: TRelayEdge[] = existing?.edges ?? []; let suffix: typeof prefix = []; if (args && args.after) { @@ -207,34 +223,40 @@ export function relayStylePagination( prefix = []; } - const edges = [ - ...prefix, - ...incomingEdges, - ...suffix, - ]; - - const pageInfo: TRelayPageInfo = { - // 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, + merged = { + ...merged, + edges: [ + ...prefix, + ...incomingEdges, + ...suffix, + ] }; + merged = { + ...merged, + pageInfo: { + // 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 ?? {}, + } + } + if (incoming.pageInfo) { const { hasPreviousPage, hasNextPage, startCursor, endCursor, ...extras - } = incoming.pageInfo; + } = incoming.pageInfo ?? {}; // If incoming.pageInfo had any extra non-standard properties, // assume they should take precedence over any existing properties // of the same name, regardless of where this page falls with // respect to the existing data. - Object.assign(pageInfo, extras); + Object.assign(merged.pageInfo, extras); // Keep existing.pageInfo.has{Previous,Next}Page unless the // placement of the incoming edges means incoming.hasPreviousPage @@ -244,21 +266,15 @@ export function relayStylePagination( // 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 (void 0 !== hasPreviousPage) merged.pageInfo!.hasPreviousPage = hasPreviousPage; + if (void 0 !== startCursor) merged.pageInfo!.startCursor = startCursor; } if (!suffix.length) { - if (void 0 !== hasNextPage) pageInfo.hasNextPage = hasNextPage; - if (void 0 !== endCursor) pageInfo.endCursor = endCursor; + if (void 0 !== hasNextPage) merged.pageInfo!.hasNextPage = hasNextPage; + if (void 0 !== endCursor) merged.pageInfo!.endCursor = endCursor; } } - - return { - ...getExtras(existing), - ...getExtras(incoming), - edges, - pageInfo, - }; + return merged }, }; } @@ -266,15 +282,3 @@ export function relayStylePagination( // Returns any unrecognized properties of the given object. const getExtras = (obj: Record) => __rest(obj, notExtras); const notExtras = ["edges", "pageInfo"]; - -function makeEmptyData(): TExistingRelay { - return { - edges: [], - pageInfo: { - hasPreviousPage: false, - hasNextPage: true, - startCursor: "", - endCursor: "", - }, - }; -} From b02138ca13dce029c9387a7a768ed825e5fb34e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Lach=C3=A8ze?= Date: Tue, 14 Dec 2021 22:55:20 -0500 Subject: [PATCH 2/3] adapt InMemoryCache tests for relayStylePagination that ignores absent edges --- .../inmemory/__tests__/__snapshots__/policies.ts.snap | 2 -- src/cache/inmemory/__tests__/policies.ts | 7 ------- 2 files changed, 9 deletions(-) diff --git a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap index 3ed68fa48cf..5f1f61e23a3 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap @@ -824,7 +824,6 @@ Object { "__typename": "PageInfo", "endCursor": "YXJyYXljb25uZWN0aW9uOjI=", "hasNextPage": true, - "hasPreviousPage": false, "startCursor": "YXJyYXljb25uZWN0aW9uOjI=", }, "totalCount": 1292, @@ -861,7 +860,6 @@ Object { "__typename": "PageInfo", "endCursor": "YXJyYXljb25uZWN0aW9uOjI=", "hasNextPage": true, - "hasPreviousPage": false, "startCursor": "YXJyYXljb25uZWN0aW9uOjI=", }, "totalCount": 1293, diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index d8b5d16d64c..8c3a7add4ba 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -3220,13 +3220,6 @@ describe("type policies", function () { ROOT_QUERY: { __typename: "Query", todos: { - edges: [], - pageInfo: { - "endCursor": "", - "hasNextPage": true, - "hasPreviousPage": false, - "startCursor": "", - }, totalCount: 1292 }, } From 14746d2d1345551ce58db4f8b09bf469c1fea55e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Lach=C3=A8ze?= Date: Thu, 6 Jan 2022 22:02:40 +0100 Subject: [PATCH 3/3] remove TConnectionExtraData --- src/utilities/policies/pagination.ts | 36 +++++++++++++++------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/utilities/policies/pagination.ts b/src/utilities/policies/pagination.ts index e55fc9c056a..b4638756114 100644 --- a/src/utilities/policies/pagination.ts +++ b/src/utilities/policies/pagination.ts @@ -70,33 +70,37 @@ export type TRelayPageInfo = { endCursor: string; }; -export type TRelayConnection = { - edges: TRelayEdge[]; - pageInfo: TRelayPageInfo; -} & TConnectionExtraData +export type TExistingRelay = Readonly<{ + edges?: TRelayEdge[]; + pageInfo?: TRelayPageInfo; + [extra: string]: any; +}>; -export type TExistingRelay = Readonly>>; -export type TIncomingRelay = Readonly>>; +export type TIncomingRelay = { + edges?: TRelayEdge[]; + pageInfo?: TRelayPageInfo; + [extra: string]: any; +}; -export type RelayFieldPolicy = FieldPolicy< - TExistingRelay | null, - TIncomingRelay | null, - TIncomingRelay | null +export type RelayFieldPolicy = FieldPolicy< + TExistingRelay | null, + TIncomingRelay | null, + TIncomingRelay | null >; // As proof of the flexibility of field policies, this function generates // one that handles Relay-style pagination, without Apollo Client knowing // anything about connections, edges, cursors, or pageInfo objects. -export function relayStylePagination( +export function relayStylePagination( keyArgs: KeyArgs = false, -): RelayFieldPolicy { +): RelayFieldPolicy { return { keyArgs, read(existing, { canRead, readField }) { if (!existing) return existing; - let read: TIncomingRelay = { + let read: TIncomingRelay = { // Some implementations return additional Connection fields, such // as existing.totalCount. These fields are saved by the merge // function, so the read function should also preserve them. @@ -137,7 +141,7 @@ export function relayStylePagination> = { + let merged: SafeReadonly> = { ...(existing || {}), ...incoming, } @@ -242,7 +246,7 @@ export function relayStylePagination