Add warning and test for useSyncExternalStore when getSnapshot isn't cached (#22262)
* add warning and test * Wrap console error in __DEV__ flag * prettier
salazarm committed
Sep 7, 2021 at 12:08 UTC
031abd24b62b7b0db2e8658ffce7728eb8592d32
2 files changed
+37
packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
+26
@@ -618,4 +618,30 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
618
expect(root).toMatchRenderedOutput('A1B1');
619
});
620
});
621
+
622
+ test('Infinite loop if getSnapshot keeps returning new reference', () => {
623
+ const store = createExternalStore({});
624
+
625
+ function App() {
626
+ const text = useSyncExternalStore(store.subscribe, () => ({}));
627
+ return <Text text={JSON.stringify(text)} />;
628
+ }
629
+
630
+ spyOnDev(console, 'error');
631
+
632
+ expect(() => {
633
+ act(() => {
634
+ createRoot(<App />);
635
+ });
636
+ }).toThrow(
637
+ 'Maximum update depth exceeded. This can happen when a component repeatedly ' +
638
+ 'calls setState inside componentWillUpdate or componentDidUpdate. React limits ' +
639
+ 'the number of nested updates to prevent infinite loops.',
640
+ );
641
+ if (__DEV__) {
642
+ expect(console.error.calls.argsFor(0)[0]).toMatch(
643
+ 'The result of getSnapshot should be cached to avoid an infinite loop',
644
+ );
645
+ }
646
+ });
647
});
packages/use-sync-external-store/src/useSyncExternalStore.js
+11
@@ -29,6 +29,7 @@ export const useSyncExternalStore =
29
builtInAPI !== undefined ? builtInAPI : useSyncExternalStore_shim;
30
31
let didWarnOld18Alpha = false;
32
+let didWarnUncachedGetSnapshot = false;
33
34
// Disclaimer: This shim breaks many of the rules of React, and only works
35
// because of a very particular set of implementation details and assumptions
@@ -63,6 +64,16 @@ function useSyncExternalStore_shim<T>(
64
// implementation details, most importantly that updates are
65
// always synchronous.
66
const value = getSnapshot();
67
+ if (__DEV__) {
68
+ if (!didWarnUncachedGetSnapshot) {
69
+ if (value !== getSnapshot()) {
70
+ console.error(
71
+ 'The result of getSnapshot should be cached to avoid an infinite loop',
72
+ );
73
+ didWarnUncachedGetSnapshot = true;
74
+ }
75
+ }
76
+ }
77
78
// Because updates are synchronous, we don't queue them. Instead we force a
79
// re-render whenever the subscribed state changes by updating an some