Warn about calling setState on an unmounted component

facebook · Feb 8, 2017 · e9aca8e · e9aca8e
1 parent 3955e22
commit e9aca8e
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 13 deletions.
diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt
@@ -57,8 +57,6 @@ src/renderers/__tests__/ReactComponentTreeHook-test.js
 * works
 
 src/renderers/__tests__/ReactCompositeComponent-test.js
-* should warn about `forceUpdate` on unmounted components
-* should warn about `setState` on unmounted components
 * should warn about `setState` in render
 * should warn about `setState` in getChildContext
 * should disallow nested render calls

diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
@@ -622,6 +622,8 @@ src/renderers/__tests__/ReactCompositeComponent-test.js
 * should not pass this to getDefaultProps
 * should use default values for undefined props
 * should not mutate passed-in props object
+* should warn about `forceUpdate` on unmounted components
+* should warn about `setState` on unmounted components
 * should silently allow `setState`, not call cb on unmounting components
 * should cleanup even if render() fatals
 * should call componentWillUnmount before unmounting

diff --git a/src/renderers/__tests__/ReactCompositeComponent-test.js b/src/renderers/__tests__/ReactCompositeComponent-test.js
@@ -277,11 +277,15 @@ describe('ReactCompositeComponent', () => {
 
     instance.forceUpdate();
     expectDev(console.error.calls.count()).toBe(1);
-    expectDev(console.error.calls.argsFor(0)[0]).toBe(
-      'Warning: forceUpdate(...): Can only update a mounted or ' +
-      'mounting component. This usually means you called forceUpdate() on an ' +
-      'unmounted component. This is a no-op.\n\nPlease check the code for the ' +
-      'Component component.'
+    expectDev(console.error.calls.argsFor(0)[0]).toContain(
+      ReactDOMFeatureFlags.useFiber ?
+        'Can only update a mounted or mounting component. This usually means ' +
+        'you called setState, replaceState, or forceUpdate on an unmounted ' +
+        'component. This is a no-op.\n\nPlease check the code for the ' +
+        'Component component.' :
+        'Can only update a mounted or mounting component. This usually means ' +
+        'you called forceUpdate() on an unmounted component. This is a no-op.' +
+        '\n\nPlease check the code for the Component component.'
     );
   });
 
@@ -321,11 +325,15 @@ describe('ReactCompositeComponent', () => {
     expect(renders).toBe(2);
 
     expectDev(console.error.calls.count()).toBe(1);
-    expectDev(console.error.calls.argsFor(0)[0]).toBe(
-      'Warning: setState(...): Can only update a mounted or ' +
-      'mounting component. This usually means you called setState() on an ' +
-      'unmounted component. This is a no-op.\n\nPlease check the code for the ' +
-      'Component component.'
+    expectDev(console.error.calls.argsFor(0)[0]).toContain(
+      ReactDOMFeatureFlags.useFiber ?
+        'Can only update a mounted or mounting component. This usually means ' +
+        'you called setState, replaceState, or forceUpdate on an unmounted ' +
+        'component. This is a no-op.\n\nPlease check the code for the ' +
+        'Component component.' :
+        'Can only update a mounted or mounting component. This usually means ' +
+        'you called setState() on an unmounted component. This is a no-op.' +
+        '\n\nPlease check the code for the Component component.'
     );
   });
 

diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -86,8 +86,21 @@ var {
 var invariant = require('invariant');
 
 if (__DEV__) {
+  var warning = require('warning');
   var ReactFiberInstrumentation = require('ReactFiberInstrumentation');
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
+
+  var warnAboutUpdateOnUnmounted = function(instance : ReactClass<any>) {
+    const ctor = instance.constructor;
+    warning(
+      false,
+      'Can only update a mounted or mounting component. This usually means ' +
+      'you called setState, replaceState, or forceUpdate on an unmounted ' +
+      'component. This is a no-op.\n\nPlease check the code for the ' +
+      '%s component.',
+      ctor && (ctor.displayName || ctor.name) || 'ReactClass'
+    );
+  };
 }
 
 var timeHeuristicForUnitOfWork = 1;
@@ -1137,7 +1150,11 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
               return;
           }
         } else {
-          // TODO: Warn about setting state on an unmounted component.
+          if (__DEV__) {
+            if (fiber.tag === ClassComponent) {
+              warnAboutUpdateOnUnmounted(fiber.stateNode);
+            }
+          }
           return;
         }
       }