fix getSnapshot warning when a selector returns NaN (#23333)
* fix getSnapshot warning when a selector returns NaN useSyncExternalStoreWithSelector delegate a selector as getSnapshot of useSyncExternalStore. * Fiber's use sync external store has a same issue * Small nits We use Object.is to check whether the snapshot value has been updated, so we should also use it to check whether the value is cached. Co-authored-by: Andrew Clark <git@andrewclark.io>
OGURA Daiki committed
Feb 21, 2022 at 15:07 UTC
4de99b3ca689e8f995f90f199cd8d7bfb4b35eca
4 files changed
+37
-5
packages/react-reconciler/src/ReactFiberHooks.new.js
+4
-2
@@ -1290,7 +1290,8 @@ function mountSyncExternalStore<T>(
1290
nextSnapshot = getSnapshot();
1291
if (__DEV__) {
1292
if (!didWarnUncachedGetSnapshot) {
1293
- if (nextSnapshot !== getSnapshot()) {
1293
+ const cachedSnapshot = getSnapshot();
1294
+ if (!is(nextSnapshot, cachedSnapshot)) {
1295
console.error(
1296
'The result of getSnapshot should be cached to avoid an infinite loop',
1297
);
@@ -1362,7 +1363,8 @@ function updateSyncExternalStore<T>(
1363
const nextSnapshot = getSnapshot();
1364
if (__DEV__) {
1365
if (!didWarnUncachedGetSnapshot) {
1365
- if (nextSnapshot !== getSnapshot()) {
1366
+ const cachedSnapshot = getSnapshot();
1367
+ if (!is(nextSnapshot, cachedSnapshot)) {
1368
console.error(
1369
'The result of getSnapshot should be cached to avoid an infinite loop',
1370
);
packages/react-reconciler/src/ReactFiberHooks.old.js
+4
-2
@@ -1290,7 +1290,8 @@ function mountSyncExternalStore<T>(
1290
nextSnapshot = getSnapshot();
1291
if (__DEV__) {
1292
if (!didWarnUncachedGetSnapshot) {
1293
- if (nextSnapshot !== getSnapshot()) {
1293
+ const cachedSnapshot = getSnapshot();
1294
+ if (!is(nextSnapshot, cachedSnapshot)) {
1295
console.error(
1296
'The result of getSnapshot should be cached to avoid an infinite loop',
1297
);
@@ -1362,7 +1363,8 @@ function updateSyncExternalStore<T>(
1363
const nextSnapshot = getSnapshot();
1364
if (__DEV__) {
1365
if (!didWarnUncachedGetSnapshot) {
1365
- if (nextSnapshot !== getSnapshot()) {
1366
+ const cachedSnapshot = getSnapshot();
1367
+ if (!is(nextSnapshot, cachedSnapshot)) {
1368
console.error(
1369
'The result of getSnapshot should be cached to avoid an infinite loop',
1370
);
packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
+27
@@ -587,6 +587,33 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
587
);
588
});
589
590
+ test('getSnapshot can return NaN without infinite loop warning', async () => {
591
+ const store = createExternalStore('not a number');
592
+
593
+ function App() {
594
+ const value = useSyncExternalStore(store.subscribe, () =>
595
+ parseInt(store.getState(), 10),
596
+ );
597
+ return <Text text={value} />;
598
+ }
599
+
600
+ const container = document.createElement('div');
601
+ const root = createRoot(container);
602
+
603
+ // Initial render that reads a snapshot of NaN. This is OK because we use
604
+ // Object.is algorithm to compare values.
605
+ await act(() => root.render(<App />));
606
+ expect(container.textContent).toEqual('NaN');
607
+
608
+ // Update to real number
609
+ await act(() => store.set(123));
610
+ expect(container.textContent).toEqual('123');
611
+
612
+ // Update back to NaN
613
+ await act(() => store.set('not a number'));
614
+ expect(container.textContent).toEqual('NaN');
615
+ });
616
+
617
describe('extra features implemented in user-space', () => {
618
// The selector implementation uses the lazy ref initialization pattern
619
// @gate !(enableUseRefAccessWarning && __DEV__)
packages/use-sync-external-store/src/useSyncExternalStoreShimClient.js
+2
-1
@@ -57,7 +57,8 @@ export function useSyncExternalStore<T>(
57
const value = getSnapshot();
58
if (__DEV__) {
59
if (!didWarnUncachedGetSnapshot) {
60
- if (value !== getSnapshot()) {
60
+ const cachedValue = getSnapshot();
61
+ if (!is(value, cachedValue)) {
62
console.error(
63
'The result of getSnapshot should be cached to avoid an infinite loop',
64
);