Skip to content

Commit

Permalink
Updating connection handler to be able to deal with streamed edges th…
Browse files Browse the repository at this point in the history 
…at are already in memory

Summary:
Fix fix fix

tl;dr: *Remembering the page_info of previously streamed pages causes ConnectionHandler to prematurely update the end cursor and not accept more than the initial count of items for the connection.*

Original Post:
https://fb.workplace.com/groups/relay.support/permalink/24536709345951016/

# Debugging:

On reload of pages/connections that have been in memory before it seems to recall/remember the pageInfo of those.
https://www.internalfb.com/code/fbsource/[8aede072216b66fd056bd12e1de7a463250acb3c]/xplat/js/RKJSModules/Libraries/Relay/oss/relay-runtime/handlers/connection/ConnectionHandler.js?lines=67

Usually, with stream_connection it doesn’t know the page_info till the end of the stream (its null until it comes back deferred).

But on second reload of the exact same page, it looks up the same key, and finds the page info from last time causing it to set the end cursor prematurely.
https://www.internalfb.com/code/fbsource/[8aede072216b66fd056bd12e1de7a463250acb3c][history][blame]/xplat/js/RKJSModules/Libraries/Relay/oss/relay-runtime/handlers/connection/ConnectionHandler.js?lines=221

This causes all subsequently streamed edges to fail due to this call:
https://www.internalfb.com/code/fbsource/[8aede072216b66fd056bd12e1de7a463250acb3c][history][blame]/xplat/js/RKJSModules/Libraries/Relay/oss/relay-runtime/handlers/connection/ConnectionHandler.js?lines=150

Remembering the page_info of previously streamed pages causes ConnectionHandler to prematurely update the end cursor and not accept more than the initial count of items for subsequent pages in the connection. This modifies the logic to be able to accept pages that either agree on after/end cursor or matching end cursors.

Reviewed By: keoskate, fred2028

Differential Revision: D45586049

fbshipit-source-id: 34ac5e342ca0975f2ad841a382ffde4ff5fc6bd3
  • Loading branch information
@facebook-github-bot
Jesse Watts-Russell authored and facebook-github-bot committed May 6, 2023
1 parent 92fd38c commit 6f30869
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 4 deletions.
20 changes: 16 additions & 4 deletions packages/relay-runtime/handlers/connection/ConnectionHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,23 @@ function update(store: RecordSourceProxy, payload: HandleFieldPayload): void {
const args = payload.args;
if (prevEdges && serverEdges) {
if (args.after != null) {
const clientEndCursor = clientPageInfo?.getValue(END_CURSOR);
const serverEndCursor = serverPageInfo?.getValue(END_CURSOR);

const isAddingEdgesAfterCurrentPage =
clientPageInfo && args.after === clientEndCursor;
const isFillingOutCurrentPage =
clientPageInfo && clientEndCursor === serverEndCursor;

// Forward pagination from the end of the connection: append edges
if (
clientPageInfo &&
args.after === clientPageInfo.getValue(END_CURSOR)
) {
// Case 1: We're fetching edges for the first time and pageInfo for
// the upcoming page is missing, but our after cursor matches
// the last ending cursor. (adding after current page)
// Case 2: We've fetched these edges before and we know the end cursor
// from the first edge updating the END_CURSOR field. If the
// end cursor from the server matches the end cursor from the
// client then we're just filling out the rest of this page.
if (isAddingEdgesAfterCurrentPage || isFillingOutCurrentPage) {
const nodeIDs = new Set<mixed>();
mergeEdges(prevEdges, nextEdges, nodeIDs);
mergeEdges(serverEdges, nextEdges, nodeIDs);
Expand Down
184 changes: 184 additions & 0 deletions packages/relay-runtime/handlers/connection/__tests__/ConnectionHandler-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,190 @@ describe('ConnectionHandler', () => {
});
});

it('appends two streamed edges, which have been streamed before and know their end cursors', () => {
// First edge
normalize(
{
node: {
id: '4',
__typename: 'User',
friends: {
edges: [
{
cursor: 'cursor:2',
node: {
id: '2',
},
},
],
[PAGE_INFO]: {
// EACH EDGE ALREADY WILL KNOW ITS END CURSOR FOR THAT PAGE
[END_CURSOR]: 'cursor:3',
[HAS_NEXT_PAGE]: false,
[HAS_PREV_PAGE]: false,
[START_CURSOR]: 'cursor:2',
},
},
},
},
{
after: 'cursor:1',
before: null,
count: 10,
orderby: ['first name'],
id: '4',
},
);
const args = {after: 'cursor:1', first: 10, orderby: ['first name']};
const handleKey =
getRelayHandleKey(
'connection',
'ConnectionQuery_friends',
'friends',
) + '(orderby:["first name"])';
const payload = {
args,
dataID: '4',
fieldKey: getStableStorageKey('friends', args),
handleKey,
};
ConnectionHandler.update(proxy, payload);
expect(sinkSource.toJSON()).toEqual({
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"])':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"])',
[TYPENAME_KEY]: 'FriendsConnection',
edges: {
[REFS_KEY]: [
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:0',
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:1',
],
},
pageInfo: {
[REF_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo',
},
__connection_next_edge_index: 2,
},
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:1':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:1',
[TYPENAME_KEY]: 'FriendsEdge',
cursor: 'cursor:2',
node: {[REF_KEY]: '2'},
},
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo',
[TYPENAME_KEY]: 'PageInfo',
[END_CURSOR]: 'cursor:3',
[HAS_NEXT_PAGE]: false,
},
});

// Second Edge
normalize(
{
node: {
id: '4',
__typename: 'User',
friends: {
edges: [
{
cursor: 'cursor:3',
node: {
id: '3',
},
},
],
[PAGE_INFO]: {
// EACH EDGE ALREADY WILL KNOW ITS END CURSOR FOR THAT PAGE
// (THIS IS FINAL EDGE, BUT STILL...)
[END_CURSOR]: 'cursor:3',
[HAS_NEXT_PAGE]: false,
[HAS_PREV_PAGE]: false,
[START_CURSOR]: 'cursor:2',
},
},
},
},
{
after: 'cursor:1',
before: null,
count: 10,
orderby: ['first name'],
id: '4',
},
);
const secondArgs = {
after: 'cursor:1',
first: 10,
orderby: ['first name'],
};
const secondHandleKey =
getRelayHandleKey(
'connection',
'ConnectionQuery_friends',
'friends',
) + '(orderby:["first name"])';
const secondPayload = {
args,
dataID: '4',
fieldKey: getStableStorageKey('friends', secondArgs),
handleKey: secondHandleKey,
};
ConnectionHandler.update(proxy, secondPayload);

const result = {
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"])':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"])',
[TYPENAME_KEY]: 'FriendsConnection',
edges: {
[REFS_KEY]: [
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:0',
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:1',
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:2',
],
},
pageInfo: {
[REF_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo',
},
__connection_next_edge_index: 3,
},
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:1':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:1',
[TYPENAME_KEY]: 'FriendsEdge',
cursor: 'cursor:2',
node: {[REF_KEY]: '2'},
},
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:2':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):edges:2',
[TYPENAME_KEY]: 'FriendsEdge',
cursor: 'cursor:3',
node: {[REF_KEY]: '3'},
},
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo':
{
[ID_KEY]:
'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo',
[TYPENAME_KEY]: 'PageInfo',
[END_CURSOR]: 'cursor:3',
[HAS_NEXT_PAGE]: false,
},
};
expect(sinkSource.toJSON()).toEqual(result);
});

it('prepends new edges', () => {
normalize(
{
Expand Down

0 comments on commit 6f30869

Please sign in to comment.