Skip to content

Commit f4be010

Browse files
author
Brian Vaughn
committed
Don't warn about unmounted updates if pending passive unmount
I recently landed a change to the timing of passive effect cleanup functions during unmount (see #17925). This change defers flushing of passive effects for unmounted components until later (whenever we next flush pending passive effects). Since this change increases the likelihood of a (not actionable) state update warning for unmounted components, I've suppressed that warning for Fibers that have scheduled passive effect unmounts pending.
1 parent 65bbda7 commit f4be010

File tree

2 files changed

+97
-0
lines changed

2 files changed

+97
-0
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {Effect as HookEffect} from './ReactFiberHooks';
1818

1919
import {
2020
warnAboutDeprecatedLifecycles,
21+
deferPassiveEffectCleanupDuringUnmount,
2122
runAllPassiveEffectDestroysBeforeCreates,
2223
enableUserTimingAPI,
2324
enableSuspenseServerRenderer,
@@ -2687,6 +2688,18 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
26872688
// Only warn for user-defined components, not internal ones like Suspense.
26882689
return;
26892690
}
2691+
2692+
if (
2693+
deferPassiveEffectCleanupDuringUnmount &&
2694+
runAllPassiveEffectDestroysBeforeCreates
2695+
) {
2696+
// If there are pending passive effects unmounts for this Fiber,
2697+
// we can assume that they would have prevented this update.
2698+
if (pendingPassiveHookEffectsUnmount.includes(fiber)) {
2699+
return;
2700+
}
2701+
}
2702+
26902703
// We show the whole stack but dedupe on the top component's name because
26912704
// the problematic code almost always lies inside that component.
26922705
const componentName = getComponentName(fiber.type) || 'ReactComponent';

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,90 @@ function loadModules({
11441144
]);
11451145
});
11461146
});
1147+
1148+
it('does not warn about state updates for unmounted components with pending passive unmounts', () => {
1149+
let completePendingRequest = null;
1150+
function Component() {
1151+
Scheduler.unstable_yieldValue('Component');
1152+
const [didLoad, setDidLoad] = React.useState(false);
1153+
React.useLayoutEffect(() => {
1154+
Scheduler.unstable_yieldValue('layout create');
1155+
return () => {
1156+
Scheduler.unstable_yieldValue('layout destroy');
1157+
};
1158+
}, []);
1159+
React.useEffect(() => {
1160+
Scheduler.unstable_yieldValue('passive create');
1161+
// Mimic an XHR request with a complete handler that updates state.
1162+
completePendingRequest = () => setDidLoad(true);
1163+
return () => {
1164+
Scheduler.unstable_yieldValue('passive destroy');
1165+
};
1166+
}, []);
1167+
return didLoad;
1168+
}
1169+
1170+
act(() => {
1171+
ReactNoop.renderToRootWithID(<Component />, 'root', () =>
1172+
Scheduler.unstable_yieldValue('Sync effect'),
1173+
);
1174+
expect(Scheduler).toFlushAndYieldThrough([
1175+
'Component',
1176+
'layout create',
1177+
'Sync effect',
1178+
]);
1179+
ReactNoop.flushPassiveEffects();
1180+
expect(Scheduler).toHaveYielded(['passive create']);
1181+
1182+
// Unmount but don't process pending passive destroy function
1183+
ReactNoop.unmountRootWithID('root');
1184+
expect(Scheduler).toFlushAndYieldThrough(['layout destroy']);
1185+
1186+
// Simulate an XHR completing, which will cause a state update-
1187+
// but should not log a warning.
1188+
completePendingRequest();
1189+
1190+
ReactNoop.flushPassiveEffects();
1191+
expect(Scheduler).toHaveYielded(['passive destroy']);
1192+
});
1193+
});
1194+
1195+
it('still warns about state updates for unmounted components with no pending passive unmounts', () => {
1196+
let completePendingRequest = null;
1197+
function Component() {
1198+
Scheduler.unstable_yieldValue('Component');
1199+
const [didLoad, setDidLoad] = React.useState(false);
1200+
React.useLayoutEffect(() => {
1201+
Scheduler.unstable_yieldValue('layout create');
1202+
// Mimic an XHR request with a complete handler that updates state.
1203+
completePendingRequest = () => setDidLoad(true);
1204+
return () => {
1205+
Scheduler.unstable_yieldValue('layout destroy');
1206+
};
1207+
}, []);
1208+
return didLoad;
1209+
}
1210+
1211+
act(() => {
1212+
ReactNoop.renderToRootWithID(<Component />, 'root', () =>
1213+
Scheduler.unstable_yieldValue('Sync effect'),
1214+
);
1215+
expect(Scheduler).toFlushAndYieldThrough([
1216+
'Component',
1217+
'layout create',
1218+
'Sync effect',
1219+
]);
1220+
1221+
// Unmount but don't process pending passive destroy function
1222+
ReactNoop.unmountRootWithID('root');
1223+
expect(Scheduler).toFlushAndYieldThrough(['layout destroy']);
1224+
1225+
// Simulate an XHR completing.
1226+
expect(completePendingRequest).toErrorDev(
1227+
"Warning: Can't perform a React state update on an unmounted component.",
1228+
);
1229+
});
1230+
});
11471231
}
11481232

11491233
it('updates have async priority', () => {

0 commit comments

Comments
 (0)