Skip to content

Commit

Permalink
Deduplicate connection edges based on DataID (not just id field)
Browse files Browse the repository at this point in the history
Summary:
Changes ConnectionHandler to deduplicate edges based on their `node`'s *DataID* as opposed to their `id` value. This covers two cases that weren't deduplicated before:
* Connections where nodes have an `id`, but the injected GetDataID function would return a different value. In the past we used `id` interchangeably with DataID for any node that had an id, but now that DataID is configurable we should use that for determining uniqueness.
* Connections where nodes don't have an `id` weren't getting deduplicated. This is more problematic with streaming, where the same set of edges is replayed multiple times as new edges stream in.

Differential Revision: D15559107

fbshipit-source-id: 66b37b54780b3e3409d02a7268bd541c6c5849b6
  • Loading branch information
josephsavona authored and facebook-github-bot committed May 30, 2019
1 parent 6ba5890 commit 95720ee
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 4 deletions.
Expand Up @@ -508,7 +508,7 @@ function mergeEdges(
continue;
}
const node = edge.getLinkedRecord(NODE);
const nodeID = node && node.getValue('id');
const nodeID = node && node.getDataID();
if (nodeID) {
if (nodeIDs.has(nodeID)) {
continue;
Expand Down
Expand Up @@ -50,7 +50,7 @@ describe('RelayConnectionHandler', () => {
let sinkData;
let sinkSource;

function normalize(payload, variables) {
function normalize(payload, variables, options) {
RelayResponseNormalizer.normalize(
baseSource,
{
Expand All @@ -59,7 +59,7 @@ describe('RelayConnectionHandler', () => {
variables,
},
payload,
{
options ?? {
getDataID: defaultGetDataID,
},
);
Expand Down Expand Up @@ -1211,7 +1211,7 @@ describe('RelayConnectionHandler', () => {
});
});

it('skips edges with duplicate node ids', () => {
it('skips edges with duplicate node data id (server `id`)', () => {
normalize(
{
node: {
Expand Down Expand Up @@ -1305,6 +1305,110 @@ describe('RelayConnectionHandler', () => {
});
});

it('skips edges with duplicate node data id (client ids)', () => {
normalize(
{
node: {
id: '4',
__typename: 'User',
friends: {
edges: [
{
cursor: 'cursor:2', // new cursor
node: {
// below getDataID() rewrites to same __id as the existing
// edge
id: '<duplicate-1>',
},
},
{
cursor: 'cursor:3',
node: {
id: '3',
},
},
],
[PAGE_INFO]: {
[END_CURSOR]: 'cursor:3',
[HAS_NEXT_PAGE]: true,
[HAS_PREV_PAGE]: false,
[START_CURSOR]: 'cursor:3',
},
},
},
},
{
after: 'cursor:1',
before: null,
count: 10,
orderby: ['first name'],
id: '4',
},
{
getDataID: (value, typeName) => {
if (value.id === '<duplicate-1>') {
return '1';
}
return value.id;
},
},
);
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,
};
RelayConnectionHandler.update(proxy, payload);
expect(sinkData).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',
// '...edges:0' skipped bc of duplicate node id
'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]: '1'},
},
'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]: true,
},
});
});

it('adds edges with duplicate cursors', () => {
normalize(
{
Expand Down

0 comments on commit 95720ee

Please sign in to comment.