Add warning and test for useSyncExternalStore when getSnapshot isn't …

…cached (#22262) * add warning and test * Wrap console error in __DEV__ flag * prettier
facebook · Sep 7, 2021 · 031abd2 · 031abd2
1 parent e07039b
commit 031abd2
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 0 deletions.
diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
@@ -618,4 +618,30 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
       expect(root).toMatchRenderedOutput('A1B1');
     });
   });
+
+  test('Infinite loop if getSnapshot keeps returning new reference', () => {
+    const store = createExternalStore({});
+
+    function App() {
+      const text = useSyncExternalStore(store.subscribe, () => ({}));
+      return <Text text={JSON.stringify(text)} />;
+    }
+
+    spyOnDev(console, 'error');
+
+    expect(() => {
+      act(() => {
+        createRoot(<App />);
+      });
+    }).toThrow(
+      'Maximum update depth exceeded. This can happen when a component repeatedly ' +
+        'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
+        'the number of nested updates to prevent infinite loops.',
+    );
+    if (__DEV__) {
+      expect(console.error.calls.argsFor(0)[0]).toMatch(
+        'The result of getSnapshot should be cached to avoid an infinite loop',
+      );
+    }
+  });
 });
diff --git a/packages/use-sync-external-store/src/useSyncExternalStore.js b/packages/use-sync-external-store/src/useSyncExternalStore.js
@@ -29,6 +29,7 @@ export const useSyncExternalStore =
   builtInAPI !== undefined ? builtInAPI : useSyncExternalStore_shim;
 
 let didWarnOld18Alpha = false;
+let didWarnUncachedGetSnapshot = false;
 
 // Disclaimer: This shim breaks many of the rules of React, and only works
 // because of a very particular set of implementation details and assumptions
@@ -63,6 +64,16 @@ function useSyncExternalStore_shim<T>(
   // implementation details, most importantly that updates are
   // always synchronous.
   const value = getSnapshot();
+  if (__DEV__) {
+    if (!didWarnUncachedGetSnapshot) {
+      if (value !== getSnapshot()) {
+        console.error(
+          'The result of getSnapshot should be cached to avoid an infinite loop',
+        );
+        didWarnUncachedGetSnapshot = true;
+      }
+    }
+  }
 
   // Because updates are synchronous, we don't queue them. Instead we force a
   // re-render whenever the subscribed state changes by updating an some