Tweak not-yet-mounted setState warning

facebook · Apr 3, 2018 · 3a0b539 · 3a0b539
1 parent eb6e752
commit 3a0b539
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 4 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
@@ -225,6 +225,86 @@ describe('ReactCompositeComponent', () => {
     expect(inputProps.prop).not.toBeDefined();
   });
 
+  it('should warn about `forceUpdate` on not-yet-mounted components', () => {
+    class MyComponent extends React.Component {
+      constructor(props) {
+        super(props);
+        this.forceUpdate();
+      }
+      render() {
+        return <div />;
+      }
+    }
+
+    const container = document.createElement('div');
+    expect(() => ReactDOM.render(<MyComponent />, container)).toWarnDev(
+      "Warning: Can't call forceUpdate on a component that is not yet mounted. " +
+        'This is a no-op, but it might indicate a bug in your application. ' +
+        'To fix, assign the initial state in the MyComponent constructor. ' +
+        'If the state needs to reflect an external data source, ' +
+        'you may also add a componentDidMount lifecycle hook to MyComponent ' +
+        'and call setState there if the external data has changed.',
+    );
+
+    // No additional warning should be recorded
+    const container2 = document.createElement('div');
+    ReactDOM.render(<MyComponent />, container2);
+  });
+
+  it('should warn about `setState` on not-yet-mounted components', () => {
+    class MyComponent extends React.Component {
+      constructor(props) {
+        super(props);
+        this.setState();
+      }
+      render() {
+        return <div />;
+      }
+    }
+
+    const container = document.createElement('div');
+    expect(() => ReactDOM.render(<MyComponent />, container)).toWarnDev(
+      "Warning: Can't call setState on a component that is not yet mounted. " +
+        'This is a no-op, but it might indicate a bug in your application. ' +
+        'To fix, assign the initial state in the MyComponent constructor. ' +
+        'If the state needs to reflect an external data source, ' +
+        'you may also add a componentDidMount lifecycle hook to MyComponent ' +
+        'and call setState there if the external data has changed.',
+    );
+
+    // No additional warning should be recorded
+    const container2 = document.createElement('div');
+    ReactDOM.render(<MyComponent />, container2);
+  });
+
+  it('should silently allow `setState`, not call cb on unmounting components', () => {
+    let cbCalled = false;
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+
+    class Component extends React.Component {
+      state = {value: 0};
+
+      componentWillUnmount() {
+        expect(() => {
+          this.setState({value: 2}, function() {
+            cbCalled = true;
+          });
+        }).not.toThrow();
+      }
+
+      render() {
+        return <div />;
+      }
+    }
+
+    const instance = ReactDOM.render(<Component />, container);
+    instance.setState({value: 1});
+
+    ReactDOM.unmountComponentAtNode(container);
+    expect(cbCalled).toBe(false);
+  });
+
   it('should warn about `forceUpdate` on unmounted components', () => {
     const container = document.createElement('div');
     document.body.appendChild(container);

diff --git a/packages/react/src/ReactNoopUpdateQueue.js b/packages/react/src/ReactNoopUpdateQueue.js
@@ -21,12 +21,15 @@ function warnNoop(publicInstance, callerName) {
     }
     warning(
       false,
-      '%s(...): Can only update a mounted or mounting component. ' +
-        'This usually means you called %s() on an unmounted component. ' +
-        'This is a no-op.\n\nPlease check the code for the %s component.',
-      callerName,
+      "Can't call %s on a component that is not yet mounted. " +
+        'This is a no-op, but it might indicate a bug in your application. ' +
+        'To fix, assign the initial state in the %s constructor. ' +
+        'If the state needs to reflect an external data source, ' +
+        'you may also add a componentDidMount lifecycle hook to %s ' +
+        'and call setState there if the external data has changed.',
       callerName,
       componentName,
+      componentName,
     );
     didWarnStateUpdateForUnmountedComponent[warningKey] = true;
   }