Write linked ids before traversing plural linked fields

Reviewed By: blairvanderhoof, josephsavona Differential Revision: D3713253 fbshipit-source-id: 7ec9a83a6941688f509535186d31a7a360fee746
facebook · Aug 15, 2016 · be45692 · be45692
1 parent bb38793
commit be45692
Show file tree

Hide file tree

Showing 4 changed files with 119 additions and 9 deletions.
diff --git a/scripts/jest/testschema.graphql b/scripts/jest/testschema.graphql
@@ -498,6 +498,8 @@ type PhotoStory implements FeedUnit Node {
 }
 
 type Story implements FeedUnit Node {
+  attachments: [StoryAttachment]
+
   # FeedUnit
   canViewerDelete: Boolean
   seenState: String
@@ -540,6 +542,11 @@ type Story implements FeedUnit Node {
   viewerSavedState: String
 }
 
+type StoryAttachment {
+  target: Story
+  styleList: [String]
+}
+
 type StreetAddress {
   city: String
   country: String

diff --git a/scripts/jest/testschema.json b/scripts/jest/testschema.json
@@ -591,6 +591,22 @@
           "name": "Story",
           "description": null,
           "fields": [
+            {
+              "name": "attachments",
+              "description": null,
+              "args": [],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "StoryAttachment",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "canViewerDelete",
               "description": null,
@@ -6297,6 +6313,45 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "OBJECT",
+          "name": "StoryAttachment",
+          "description": null,
+          "fields": [
+            {
+              "name": "target",
+              "description": null,
+              "args": [],
+              "type": {
+                "kind": "OBJECT",
+                "name": "Story",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "styleList",
+              "description": null,
+              "args": [],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "String",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [],
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "FakeNode",

diff --git a/src/store/RelayQueryWriter.js b/src/store/RelayQueryWriter.js
@@ -595,6 +595,7 @@ class RelayQueryWriter extends RelayQueryVisitor<WriterState> {
 
     const prevLinkedIDs = this._store.getLinkedRecordIDs(recordID, storageKey);
     const nextLinkedIDs = [];
+    const nextRecords = {};
     let isUpdate = false;
     let nextIndex = 0;
     fieldData.forEach(nextRecord => {
@@ -620,19 +621,24 @@ class RelayQueryWriter extends RelayQueryVisitor<WriterState> {
 
       const path = RelayQueryPath.getPath(state.path, field, nextLinkedID);
       this.createRecordIfMissing(field, nextLinkedID, path, nextRecord);
+      nextRecords[nextLinkedID] = {record: nextRecord, path};
       isUpdate = isUpdate || nextLinkedID !== prevLinkedID;
-
-      this.traverse(field, {
-        nodeID: null, // never propagate `nodeID` past the first linked field
-        path,
-        recordID: nextLinkedID,
-        responseData: nextRecord,
-      });
       nextIndex++;
     });
-
+    // Write the linked records before traverse to prevent generating extraneous
+    // client ids.
     this._writer.putLinkedRecordIDs(recordID, storageKey, nextLinkedIDs);
-
+    nextLinkedIDs.forEach(nextLinkedID => {
+      const itemData = nextRecords[nextLinkedID];
+      if (itemData) {
+        this.traverse(field, {
+          nodeID: null, // never propagate `nodeID` past the first linked field
+          path: itemData.path,
+          recordID: nextLinkedID,
+          responseData: itemData.record,
+        });
+      }
+    });
     // Only broadcast a list-level change if a record was changed/added/removed
     if (
       isUpdate ||

diff --git a/src/traversal/__tests__/writeRelayQueryPayload_pluralLinkedField-test.js b/src/traversal/__tests__/writeRelayQueryPayload_pluralLinkedField-test.js
@@ -428,5 +428,47 @@ describe('writeRelayQueryPayload()', () => {
       writePayload(store, writer, query, payload);
       expect(store.getType('123')).toBe('User');
     });
+
+    it('does not overwrite nested child field', () => {
+      const records = {};
+      const store = new RelayRecordStore({records});
+      const writer = new RelayRecordWriter(records, {}, false);
+      const query = getNode(Relay.QL`
+        query {
+          node(id: "1") {
+            ... on Story {
+              attachments {
+                target {
+                  attachments {
+                    styleList
+                  }
+                }
+              }
+            }
+          }
+        }
+      `);
+      const styleList = ['Image'];
+      const payload = {
+        node: {
+          id: '1',
+          attachments: [{
+            target: {
+              id: '1',
+              attachments: [{
+                styleList,
+              }],
+            },
+          }],
+          __typename: 'Story',
+        },
+      };
+      writePayload(store, writer, query, payload);
+      const attachmentIDs = store.getLinkedRecordIDs('1', 'attachments');
+      expect(attachmentIDs.length).toBe(1);
+      expect(store.getField(attachmentIDs[0], 'styleList'))
+        .toEqual(styleList);
+      expect(store.getLinkedRecordID(attachmentIDs[0], 'target')).toBe('1');
+    });
   });
 });