Add handleMissedUpdates to subscription callback

Summary:
I've spent quite some time trying to come up with a unit test for the issue we observed in one of our apps. The problem was with the `setState` function in the RelayStore subscription callback. In specific conditions, such as a specific server response for the refetch query, batched updates in React, and GC runs from Relay, and something else that I couldn't fully figure out, the `useFragment` may return stale data.

Unfortunately, I don't have a unit test yet. So, I propose adding a logger function to detect how often we encounter these cases, and maybe there is a specific pattern we can identify that would help us find the root cause of the issue.

To detect these cases, we can use the `handleMissedUpdates` function, which will return a new state with the updated snapshot. We would log how often we have missed updates (this also needs to be integrated with the internal logger). I believe that this change will fix the issue, as we would be returning fresh data from the subscription hook.

Reviewed By: tyao1

Differential Revision: D49925999

fbshipit-source-id: 07944bb7209ed9f5550261b0549a7287d4e68909

Author: alunyov

packages/react-relay/relay-hooks/__tests__/useFragmentNode-test.js

@@ -975,6 +975,133 @@ describe.each([
       });
     });
 
+    it('should ignore updates to initially rendered data when fragment pointers change, but still handle updates to the new data', () => {
+      const Scheduler = require('scheduler');
+      const YieldChild = (props: any) => {
+        // NOTE the unstable_yield method will move to the static renderer.
+        // When React sync runs we need to update this.
+        Scheduler.log(props.children);
+        return props.children;
+      };
+      const YieldyUserComponent = ({user}: any) => (
+        <>
+          <YieldChild>Hey user,</YieldChild>
+          <YieldChild>{user.name}</YieldChild>
+          <YieldChild>with id {user.id}!</YieldChild>
+        </>
+      );
+
+      // Assert initial render
+      // $FlowFixMe[incompatible-type]
+      SingularRenderer = YieldyUserComponent;
+      internalAct(() => {
+        renderSingularFragment({isConcurrent: true});
+      });
+      expectSchedulerToHaveYielded([
+        'Hey user,',
+        'Alice',
+        ['with id ', '1', '!'],
+      ]);
+      assertFragmentResults([
+        {
+          data: {
+            id: '1',
+            name: 'Alice',
+            profile_picture: null,
+            ...createFragmentRef('1', singularQuery),
+          },
+        },
+      ]);
+
+      const newVariables = {...singularVariables, id: '200'};
+      const newQuery = createOperationDescriptor(
+        gqlSingularQuery,
+        newVariables,
+      );
+      internalAct(() => {
+        environment.commitPayload(newQuery, {
+          node: {
+            __typename: 'User',
+            id: '200',
+            name: 'Foo',
+            username: 'userfoo',
+            profile_picture: null,
+          },
+        });
+      });
+
+      internalAct(() => {
+        // Pass new fragment ref that points to new ID 200
+        setSingularOwner(newQuery);
+
+        // Flush some of the changes, but don't commit
+        expectSchedulerToFlushAndYieldThrough(['Hey user,', 'Foo']);
+
+        // Trigger an update for initially rendered data and for the new data
+        // while second render is in progress
+        environment.commitUpdate(store => {
+          store.get('1')?.setValue('Alice in Wonderland', 'name');
+          store.get('200')?.setValue('Foo Bar', 'name');
+        });
+
+        // Assert the component renders the data from newQuery/newVariables,
+        // ignoring any updates triggered while render was in progress.
+        const expectedData = {
+          data: {
+            id: '200',
+            name: 'Foo',
+            profile_picture: null,
+            ...createFragmentRef('200', newQuery),
+          },
+        };
+        expectSchedulerToFlushAndYield([
+          ['with id ', '200', '!'],
+          'Hey user,',
+          'Foo Bar',
+          ['with id ', '200', '!'],
+        ]);
+        assertFragmentResults([
+          expectedData,
+          {
+            data: {
+              id: '200',
+              name: 'Foo Bar',
+              profile_picture: null,
+              ...createFragmentRef('200', newQuery),
+            },
+          },
+        ]);
+
+        // Update latest rendered data
+        environment.commitPayload(newQuery, {
+          node: {
+            __typename: 'User',
+            id: '200',
+            // Update name
+            name: 'Foo Updated',
+            username: 'userfoo',
+            profile_picture: null,
+          },
+        });
+        expectSchedulerToFlushAndYield([
+          'Hey user,',
+          'Foo Updated',
+          ['with id ', '200', '!'],
+        ]);
+        assertFragmentResults([
+          {
+            data: {
+              id: '200',
+              // Assert name is updated
+              name: 'Foo Updated',
+              profile_picture: null,
+              ...createFragmentRef('200', newQuery),
+            },
+          },
+        ]);
+      });
+    });
+
     it('should re-read and resubscribe to fragment when variables change', () => {
       renderSingularFragment();
       assertFragmentResults([

packages/react-relay/relay-hooks/react-cache/useFragmentInternal_REACT_CACHE.js

@@ -257,7 +257,17 @@ function subscribeToSnapshot(
           prevState.kind !== 'singular' ||
           prevState.snapshot.selector !== latestSnapshot.selector
         ) {
-          return prevState;
+          const updates = handleMissedUpdates(environment, prevState);
+          if (updates != null) {
+            const [dataChanged, nextState] = updates;
+            environment.__log({
+              name: 'useFragment.subscription.missedUpdates',
+              hasDataChanges: dataChanged,
+            });
+            return dataChanged ? nextState : prevState;
+          } else {
+            return prevState;
+          }
         }
         return {
           kind: 'singular',
@@ -280,7 +290,17 @@ function subscribeToSnapshot(
             prevState.kind !== 'plural' ||
             prevState.snapshots[index]?.selector !== latestSnapshot.selector
           ) {
-            return prevState;
+            const updates = handleMissedUpdates(environment, prevState);
+            if (updates != null) {
+              const [dataChanged, nextState] = updates;
+              environment.__log({
+                name: 'useFragment.subscription.missedUpdates',
+                hasDataChanges: dataChanged,
+              });
+              return dataChanged ? nextState : prevState;
+            } else {
+              return prevState;
+            }
           }
           const updated = [...prevState.snapshots];
           updated[index] = latestSnapshot;

packages/relay-runtime/store/RelayStoreTypes.js

@@ -714,6 +714,11 @@ export type LiveResolverBatchEndLogEvent = {
   +name: 'liveresolver.batch.end',
 };
 
+export type UseFragmentSubscriptionMissedUpdates = {
+  +name: 'useFragment.subscription.missedUpdates',
+  +hasDataChanges: boolean,
+};
+
 export type LogEvent =
   | SuspenseFragmentLogEvent
   | SuspenseQueryLogEvent
@@ -741,7 +746,8 @@ export type LogEvent =
   | StoreNotifySubscriptionLogEvent
   | EntrypointRootConsumeLogEvent
   | LiveResolverBatchStartLogEvent
-  | LiveResolverBatchEndLogEvent;
+  | LiveResolverBatchEndLogEvent
+  | UseFragmentSubscriptionMissedUpdates;
 
 export type LogFunction = LogEvent => void;
 export type LogRequestInfoFunction = mixed => void;