From fd93bc345e4a14f82e212b60f81ab5c738ebdae1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 3 Apr 2018 21:07:57 +0100 Subject: [PATCH 1/2] Tweak not-yet-mounted setState warning --- .../__tests__/ReactComponentLifeCycle-test.js | 10 ++-- .../__tests__/ReactCompositeComponent-test.js | 52 +++++++++++++++++++ packages/react/src/ReactNoopUpdateQueue.js | 11 ++-- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 505bb14bf04c1..f773221b07434 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -214,10 +214,12 @@ describe('ReactComponentLifeCycle', () => { expect(() => { ReactTestUtils.renderIntoDocument(); }).toWarnDev( - '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 ' + - 'StatefulComponent component.', + "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 StatefulComponent constructor. ' + + 'If the state needs to reflect an external data source, ' + + 'you may also add a componentDidMount lifecycle hook to StatefulComponent ' + + 'and call setState there if the external data has changed.', ); // Check deduplication; (no extra warnings should be logged). diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 10f63f6ac1fa3..94af2fb9926c9 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -225,6 +225,58 @@ 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
; + } + } + + const container = document.createElement('div'); + expect(() => ReactDOM.render(, 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(, container2); + }); + + it('should warn about `setState` on not-yet-mounted components', () => { + class MyComponent extends React.Component { + constructor(props) { + super(props); + this.setState(); + } + render() { + return
; + } + } + + const container = document.createElement('div'); + expect(() => ReactDOM.render(, 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(, container2); + }); + 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 index 5aa1ccf91589d..16ce9ff757884 100644 --- 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; } From 76c03ea4e3c23c37caca9c72b6dbbabe354c3b20 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 3 Apr 2018 21:17:23 +0100 Subject: [PATCH 2/2] Add \n\n --- .../react-dom/src/__tests__/ReactComponentLifeCycle-test.js | 2 +- .../react-dom/src/__tests__/ReactCompositeComponent-test.js | 4 ++-- packages/react/src/ReactNoopUpdateQueue.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index f773221b07434..5c8488417d33a 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -215,7 +215,7 @@ describe('ReactComponentLifeCycle', () => { ReactTestUtils.renderIntoDocument(); }).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. ' + + 'This is a no-op, but it might indicate a bug in your application.\n\n' + 'To fix, assign the initial state in the StatefulComponent constructor. ' + 'If the state needs to reflect an external data source, ' + 'you may also add a componentDidMount lifecycle hook to StatefulComponent ' + diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 94af2fb9926c9..4b8b996467914 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -239,7 +239,7 @@ describe('ReactCompositeComponent', () => { const container = document.createElement('div'); expect(() => ReactDOM.render(, 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. ' + + 'This is a no-op, but it might indicate a bug in your application.\n\n' + '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 ' + @@ -265,7 +265,7 @@ describe('ReactCompositeComponent', () => { const container = document.createElement('div'); expect(() => ReactDOM.render(, 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. ' + + 'This is a no-op, but it might indicate a bug in your application.\n\n' + '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 ' + diff --git a/packages/react/src/ReactNoopUpdateQueue.js b/packages/react/src/ReactNoopUpdateQueue.js index 16ce9ff757884..3b18c5b8b9b9f 100644 --- a/packages/react/src/ReactNoopUpdateQueue.js +++ b/packages/react/src/ReactNoopUpdateQueue.js @@ -22,7 +22,7 @@ function warnNoop(publicInstance, callerName) { warning( false, "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. ' + + 'This is a no-op, but it might indicate a bug in your application.\n\n' + '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 ' +