Skip to content

Commit

Permalink
Fix warning about setState in useEffect (#24295)
Browse files Browse the repository at this point in the history
* Fix warning about setState in useEffect

* Fix test

* Fix multiple roots
  • Loading branch information
gaearon authored Apr 7, 2022
1 parent 0579154 commit d68b09d
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 8 deletions.
38 changes: 34 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2420,6 +2433,9 @@ function flushPassiveEffectsImpl() {
}

if (__DEV__) {
isFlushingPassiveEffects = true;
didScheduleUpdateDuringPassiveEffects = false;

if (enableDebugTracing) {
logPassiveEffectsStarted(lanes);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 " +
Expand Down
38 changes: 34 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2420,6 +2433,9 @@ function flushPassiveEffectsImpl() {
}

if (__DEV__) {
isFlushingPassiveEffects = true;
didScheduleUpdateDuringPassiveEffects = false;

if (enableDebugTracing) {
logPassiveEffectsStarted(lanes);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 " +
Expand Down

0 comments on commit d68b09d

Please sign in to comment.