If any item in a plural field is missing data, refetch the whole thin…

…g. Otherwise, refetch nothing.

Summary: You may have noticed a regression due to which your plural fields, despite their query not having changed, would not be diffed out in subsequent queries. Instead, what we can do is to take an all-or-nothing approach; refetch all data if any record in a plural field appears to be missing data, or fetch nothing at all if every record has all the data it needs.

Reviewed By: josephsavona

Differential Revision: D3511313

fbshipit-source-id: 26ecbae3c46130465d6cf691496c4e78a4be041f

src/traversal/__tests__/diffRelayQuery-test.js

@@ -1080,39 +1080,33 @@ describe('diffRelayQuery', () => {
     expect(trackedQueries[0][0]).toEqualQueryNode(trackedQuery);
   });
 
-  it('diffs plural fields having exactly one linked record', () => {
+  it('diffs plural fields out when every record has all specified data', () => {
     const records = {
       '12345': {
         __dataID__: '12345',
         id: '12345',
         screennames: [
           {__dataID__: 'client:1'},
+          {__dataID__: 'client:2'},
         ],
       },
       'client:1': {
         __dataID__: 'client:1',
         service: 'GTALK',
       },
+      'client:2': {
+        __dataID__: 'client:2',
+        service: 'ICQ',
+      },
     };
     const store = new RelayRecordStore({records});
-    const expected = getNode(Relay.QL`
-      query {
-        node(id:"12345") {
-          id
-          screennames {
-            name
-          }
-        }
-      }
-    `);
 
     // Assume node(12345) is a Story
     const query = getNode(Relay.QL`
       query {
         node(id:"12345") {
           id
           screennames {
-            name
             service
           }
         }
@@ -1121,12 +1115,10 @@ describe('diffRelayQuery', () => {
 
     const tracker = new RelayQueryTracker();
     const diffQueries = diffRelayQuery(query, store, tracker);
-    expect(diffQueries.length).toBe(1);
-    expect(diffQueries[0].getName()).toBe(query.getName());
-    expect(diffQueries[0]).toEqualQueryRoot(expected);
+    expect(diffQueries.length).toBe(0);
   });
 
-  it('does not diff plural fields having more than one linked record', () => {
+  it('does not diff plural fields if any record is missing data', () => {
     const records = {
       '12345': {
         __dataID__: '12345',
@@ -1138,10 +1130,12 @@ describe('diffRelayQuery', () => {
       },
       'client:1': {
         __dataID__: 'client:1',
+        // missing `name`
         service: 'GTALK',
       },
       'client:2': {
         __dataID__: 'client:2',
+        name: 'steveluscher',
         service: 'TWITTER',
       },
     };

src/traversal/diffRelayQuery.js

@@ -503,26 +503,25 @@ class RelayDiffQueryBuilder {
           trackedNode: this._queryTracker ? field : null,
         };
       }
-    } else if (linkedIDs.length === 1) {
-      // The one item in this array is not fetchable by ID, so nothing else
-      // could have fetched additional data for individual items. Therefore,
-      // we can diff the sole record to figure out which fields have previously
-      // been fetched.
-      const sampleItemID = linkedIDs[0];
-      return this.traverse(
-        field,
-        RelayQueryPath.getPath(path, field, sampleItemID),
-        makeScope(sampleItemID)
-      );
     } else {
-      // There are multiple linked items that are not fetchable by ID (ie. they
-      // have only client IDs). Refetch every field: since each linked record
-      // could be the result of having requested a different set of fields, we
-      // can not reason about all of them by looking at one of them.
-      return {
-        diffNode: field,
-        trackedNode: null,
-      };
+      // The items in this array are not fetchable by ID, so nothing else could
+      // have fetched additional data for individual items. If any item in this
+      // list is missing data, refetch the whole field.
+      const atLeastOneItemHasMissingData =
+        linkedIDs.some(itemID => {
+          const itemState = this.traverse(
+            field,
+            RelayQueryPath.getPath(path, field, itemID),
+            makeScope(itemID)
+          );
+          return itemState && (itemState.diffNode || itemState.trackedNode);
+        });
+      if (atLeastOneItemHasMissingData) {
+        return {
+          diffNode: field,
+          trackedNode: null,
+        };
+      }
     }
     return null;
   }