Skip to content

Commit

Permalink
Improve warning message for setState-on-unmounted
Browse files Browse the repository at this point in the history
This is one of the most common warnings people see, and I don't think the old text is especially clear. Improve it.
  • Loading branch information
sophiebits committed Mar 11, 2018
1 parent fcc4f52 commit 7c4e997
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 18 deletions.
34 changes: 21 additions & 13 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,11 @@ describe('ReactCompositeComponent', () => {
ReactDOM.unmountComponentAtNode(container);

expect(() => instance.forceUpdate()).toWarnDev(
'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.',
"Warning: Can't call setState (or forceUpdate) on an unmounted " +
'component. This is a no-op, but it indicates a memory leak in your ' +
'application. To fix, cancel all subscriptions and asynchronous ' +
'tasks in the componentWillUnmount method.\n' +
' in Component (at **)',
);

// No additional warning should be recorded
Expand All @@ -269,26 +270,33 @@ describe('ReactCompositeComponent', () => {
}
}

let instance = <Component />;
expect(instance.setState).not.toBeDefined();

instance = ReactDOM.render(instance, container);
let instance;
ReactDOM.render(
<div>
<span>
<Component ref={c => (instance = c || instance)} />
</span>
</div>,
container,
);

expect(renders).toBe(1);

instance.setState({value: 1});

expect(renders).toBe(2);

ReactDOM.unmountComponentAtNode(container);
ReactDOM.render(<div />, container);

expect(() => {
instance.setState({value: 2});
}).toWarnDev(
'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.',
"Warning: Can't call setState (or forceUpdate) on an unmounted " +
'component. This is a no-op, but it indicates a memory leak in your ' +
'application. To fix, cancel all subscriptions and asynchronous ' +
'tasks in the componentWillUnmount method.\n' +
' in Component (at **)\n' +
' in span',
);

expect(renders).toBe(2);
Expand Down
13 changes: 8 additions & 5 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {HydrationContext} from './ReactFiberHydrationContext';
import type {ExpirationTime} from './ReactFiberExpirationTime';

import ReactErrorUtils from 'shared/ReactErrorUtils';
import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState';
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
import {
Expand Down Expand Up @@ -114,17 +115,19 @@ if (__DEV__) {
const didWarnStateUpdateForUnmountedComponent = {};

warnAboutUpdateOnUnmounted = function(fiber: Fiber) {
// We show the whole stack but dedupe on the top component's name because
// the problematic code almost always lies inside that component.
const componentName = getComponentName(fiber) || 'ReactClass';
if (didWarnStateUpdateForUnmountedComponent[componentName]) {
return;
}
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.',
componentName,
"Can't call setState (or forceUpdate) on an unmounted component. This " +
'is a no-op, but it indicates a memory leak in your application. To ' +
'fix, cancel all subscriptions and asynchronous tasks in the ' +
'componentWillUnmount method.%s',
getStackAddendumByWorkInProgressFiber(fiber),
);
didWarnStateUpdateForUnmountedComponent[componentName] = true;
};
Expand Down

0 comments on commit 7c4e997

Please sign in to comment.