@samitouri / QOS-React-2 / commits / 78e816032c

Don't warn about unmounted updates if pending passive unmount (#18096)

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.

Brian Vaughn committed Feb 21, 2020 at 13:11 UTC 78e816032c8af962343abbf384e06f3e9bae9269
2 files changed +191
packages/react-reconciler/src/ReactFiberWorkLoop.js
+13
@@ -18,6 +18,7 @@ import type {Effect as HookEffect} from './ReactFiberHooks';
18
19 import {
20 warnAboutDeprecatedLifecycles,
21 + deferPassiveEffectCleanupDuringUnmount,
22 runAllPassiveEffectDestroysBeforeCreates,
23 enableUserTimingAPI,
24 enableSuspenseServerRenderer,
@@ -2687,6 +2688,18 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
2688 // Only warn for user-defined components, not internal ones like Suspense.
2689 return;
2690 }
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.indexOf(fiber) >= 0) {
2699 + return;
2700 + }
2701 + }
2702 +
2703 // We show the whole stack but dedupe on the top component's name because
2704 // the problematic code almost always lies inside that component.
2705 const componentName = getComponentName(fiber.type) || 'ReactComponent';
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+178
@@ -1144,6 +1144,184 @@ function loadModules({
1144 ]);
1145 });
1146 });
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 + });
1231 +
1232 + it('still warns if there are pending passive unmount effects but not for the current fiber', () => {
1233 + let completePendingRequest = null;
1234 + function ComponentWithXHR() {
1235 + Scheduler.unstable_yieldValue('Component');
1236 + const [didLoad, setDidLoad] = React.useState(false);
1237 + React.useLayoutEffect(() => {
1238 + Scheduler.unstable_yieldValue('a:layout create');
1239 + return () => {
1240 + Scheduler.unstable_yieldValue('a:layout destroy');
1241 + };
1242 + }, []);
1243 + React.useEffect(() => {
1244 + Scheduler.unstable_yieldValue('a:passive create');
1245 + // Mimic an XHR request with a complete handler that updates state.
1246 + completePendingRequest = () => setDidLoad(true);
1247 + }, []);
1248 + return didLoad;
1249 + }
1250 +
1251 + function ComponentWithPendingPassiveUnmount() {
1252 + React.useEffect(() => {
1253 + Scheduler.unstable_yieldValue('b:passive create');
1254 + return () => {
1255 + Scheduler.unstable_yieldValue('b:passive destroy');
1256 + };
1257 + }, []);
1258 + return null;
1259 + }
1260 +
1261 + act(() => {
1262 + ReactNoop.renderToRootWithID(
1263 + <>
1264 + <ComponentWithXHR />
1265 + <ComponentWithPendingPassiveUnmount />
1266 + </>,
1267 + 'root',
1268 + () => Scheduler.unstable_yieldValue('Sync effect'),
1269 + );
1270 + expect(Scheduler).toFlushAndYieldThrough([
1271 + 'Component',
1272 + 'a:layout create',
1273 + 'Sync effect',
1274 + ]);
1275 + ReactNoop.flushPassiveEffects();
1276 + expect(Scheduler).toHaveYielded([
1277 + 'a:passive create',
1278 + 'b:passive create',
1279 + ]);
1280 +
1281 + // Unmount but don't process pending passive destroy function
1282 + ReactNoop.unmountRootWithID('root');
1283 + expect(Scheduler).toFlushAndYieldThrough(['a:layout destroy']);
1284 +
1285 + // Simulate an XHR completing in the component without a pending passive effect..
1286 + expect(completePendingRequest).toErrorDev(
1287 + "Warning: Can't perform a React state update on an unmounted component.",
1288 + );
1289 + });
1290 + });
1291 +
1292 + it('still warns about state updates from within passive unmount function', () => {
1293 + function Component() {
1294 + Scheduler.unstable_yieldValue('Component');
1295 + const [didLoad, setDidLoad] = React.useState(false);
1296 + React.useEffect(() => {
1297 + Scheduler.unstable_yieldValue('passive create');
1298 + return () => {
1299 + setDidLoad(true);
1300 + Scheduler.unstable_yieldValue('passive destroy');
1301 + };
1302 + }, []);
1303 + return didLoad;
1304 + }
1305 +
1306 + act(() => {
1307 + ReactNoop.renderToRootWithID(<Component />, 'root', () =>
1308 + Scheduler.unstable_yieldValue('Sync effect'),
1309 + );
1310 + expect(Scheduler).toFlushAndYieldThrough([
1311 + 'Component',
1312 + 'Sync effect',
1313 + 'passive create',
1314 + ]);
1315 +
1316 + // Unmount but don't process pending passive destroy function
1317 + ReactNoop.unmountRootWithID('root');
1318 + expect(() => {
1319 + expect(Scheduler).toFlushAndYield(['passive destroy']);
1320 + }).toErrorDev(
1321 + "Warning: Can't perform a React state update on an unmounted component.",
1322 + );
1323 + });
1324 + });
1325 }
1326
1327 it('updates have async priority', () => {