From d68b09defca37c30d3a7a34d4de884e374c74858 Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 7 Apr 2022 18:06:35 +0100 Subject: [PATCH] Fix warning about setState in useEffect (#24295) * Fix warning about setState in useEffect * Fix test * Fix multiple roots --- .../src/ReactFiberWorkLoop.new.js | 38 +++++++++++++++++-- .../src/ReactFiberWorkLoop.old.js | 38 +++++++++++++++++-- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 4da17c7bb6e8..3f4071164df5 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -396,9 +396,12 @@ let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes; const NESTED_UPDATE_LIMIT = 50; let nestedUpdateCount: number = 0; let rootWithNestedUpdates: FiberRoot | null = null; +let isFlushingPassiveEffects = false; +let didScheduleUpdateDuringPassiveEffects = false; const NESTED_PASSIVE_UPDATE_LIMIT = 50; let nestedPassiveUpdateCount: number = 0; +let rootWithPassiveNestedUpdates: FiberRoot | null = null; // If two updates are scheduled within the same event, we should treat their // event times as simultaneous, even if the actual clock time has advanced @@ -522,6 +525,12 @@ export function scheduleUpdateOnFiber( return null; } + if (__DEV__) { + if (isFlushingPassiveEffects) { + didScheduleUpdateDuringPassiveEffects = true; + } + } + // Mark that the root has a pending update. markRootUpdated(root, lane, eventTime); @@ -2204,6 +2213,10 @@ function commitRootImpl( // There were no passive effects, so we can immediately release the cache // pool for this render. releaseRootPooledCache(root, remainingLanes); + if (__DEV__) { + nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = null; + } } // Read this again, since an effect might have updated it @@ -2420,6 +2433,9 @@ function flushPassiveEffectsImpl() { } if (__DEV__) { + isFlushingPassiveEffects = true; + didScheduleUpdateDuringPassiveEffects = false; + if (enableDebugTracing) { logPassiveEffectsStarted(lanes); } @@ -2463,10 +2479,22 @@ function flushPassiveEffectsImpl() { flushSyncCallbacks(); - // If additional passive effects were scheduled, increment a counter. If this - // exceeds the limit, we'll fire a warning. - nestedPassiveUpdateCount = - rootWithPendingPassiveEffects === null ? 0 : nestedPassiveUpdateCount + 1; + if (__DEV__) { + // If additional passive effects were scheduled, increment a counter. If this + // exceeds the limit, we'll fire a warning. + if (didScheduleUpdateDuringPassiveEffects) { + if (root === rootWithPassiveNestedUpdates) { + nestedPassiveUpdateCount++; + } else { + nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = root; + } + } else { + nestedPassiveUpdateCount = 0; + } + isFlushingPassiveEffects = false; + didScheduleUpdateDuringPassiveEffects = false; + } // TODO: Move to commitPassiveMountEffects onPostCommitRootDevTools(root); @@ -2739,6 +2767,8 @@ function checkForNestedUpdates() { if (__DEV__) { if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) { nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = null; + console.error( 'Maximum update depth exceeded. This can happen when a component ' + "calls setState inside useEffect, but useEffect either doesn't " + diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 2c940a71001d..604a585406ae 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -396,9 +396,12 @@ let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes; const NESTED_UPDATE_LIMIT = 50; let nestedUpdateCount: number = 0; let rootWithNestedUpdates: FiberRoot | null = null; +let isFlushingPassiveEffects = false; +let didScheduleUpdateDuringPassiveEffects = false; const NESTED_PASSIVE_UPDATE_LIMIT = 50; let nestedPassiveUpdateCount: number = 0; +let rootWithPassiveNestedUpdates: FiberRoot | null = null; // If two updates are scheduled within the same event, we should treat their // event times as simultaneous, even if the actual clock time has advanced @@ -522,6 +525,12 @@ export function scheduleUpdateOnFiber( return null; } + if (__DEV__) { + if (isFlushingPassiveEffects) { + didScheduleUpdateDuringPassiveEffects = true; + } + } + // Mark that the root has a pending update. markRootUpdated(root, lane, eventTime); @@ -2204,6 +2213,10 @@ function commitRootImpl( // There were no passive effects, so we can immediately release the cache // pool for this render. releaseRootPooledCache(root, remainingLanes); + if (__DEV__) { + nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = null; + } } // Read this again, since an effect might have updated it @@ -2420,6 +2433,9 @@ function flushPassiveEffectsImpl() { } if (__DEV__) { + isFlushingPassiveEffects = true; + didScheduleUpdateDuringPassiveEffects = false; + if (enableDebugTracing) { logPassiveEffectsStarted(lanes); } @@ -2463,10 +2479,22 @@ function flushPassiveEffectsImpl() { flushSyncCallbacks(); - // If additional passive effects were scheduled, increment a counter. If this - // exceeds the limit, we'll fire a warning. - nestedPassiveUpdateCount = - rootWithPendingPassiveEffects === null ? 0 : nestedPassiveUpdateCount + 1; + if (__DEV__) { + // If additional passive effects were scheduled, increment a counter. If this + // exceeds the limit, we'll fire a warning. + if (didScheduleUpdateDuringPassiveEffects) { + if (root === rootWithPassiveNestedUpdates) { + nestedPassiveUpdateCount++; + } else { + nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = root; + } + } else { + nestedPassiveUpdateCount = 0; + } + isFlushingPassiveEffects = false; + didScheduleUpdateDuringPassiveEffects = false; + } // TODO: Move to commitPassiveMountEffects onPostCommitRootDevTools(root); @@ -2739,6 +2767,8 @@ function checkForNestedUpdates() { if (__DEV__) { if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) { nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = null; + console.error( 'Maximum update depth exceeded. This can happen when a component ' + "calls setState inside useEffect, but useEffect either doesn't " +