From 537919df7bc6aa5f0f4a831e3e0a4056d6985f22 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 17 Mar 2020 20:20:32 +0000 Subject: [PATCH 1/3] Change the warning to not say "function body" This warning is more generic and may happen with class components too. --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 2 +- .../src/__tests__/ReactHooks-test.internal.js | 6 +++--- .../__tests__/ReactHooksWithNoopRenderer-test.internal.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 67ba3d39f46b..8a2ca8c076b1 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2942,7 +2942,7 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) { if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) { didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey); console.error( - 'Cannot update a component (`%s`) from inside the function body of a ' + + 'Cannot update a component (`%s`) while rendering a ' + 'different component (`%s`). To locate the bad setState() call inside `%s`, ' + 'follow the stack trace as described in https://fb.me/setstate-in-render', setStateComponentName, diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 6de3567163a3..c1d0ea18f3f2 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1087,7 +1087,7 @@ describe('ReactHooks', () => { ), ).toErrorDev([ 'Context can only be read while React is rendering', - 'Cannot update a component (`Fn`) from inside the function body of a different component (`Cls`).', + 'Cannot update a component (`Fn`) while rendering a different component (`Cls`).', ]); }); @@ -1783,8 +1783,8 @@ describe('ReactHooks', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(2); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Cannot update a component (`%s`) from inside the function body ' + - 'of a different component (`%s`).', + 'Warning: Cannot update a component (`%s`) while rendering ' + + 'a different component (`%s`).', ); } }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 302894091b86..8216850a7a89 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -440,7 +440,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(() => expect(Scheduler).toFlushAndYield(['Foo [0]', 'Bar', 'Foo [1]']), ).toErrorDev([ - 'Cannot update a component (`Foo`) from inside the function body of a ' + + 'Cannot update a component (`Foo`) while rendering a ' + 'different component (`Bar`). To locate the bad setState() call inside `Bar`', ]); }); From 959b2796a47d304c93668ad1d22787a93ddc96c2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 17 Mar 2020 20:22:17 +0000 Subject: [PATCH 2/3] Dedupe by the rendering component --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 8a2ca8c076b1..3883923352d3 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2935,12 +2935,12 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) { const renderingComponentName = (workInProgress && getComponentName(workInProgress.type)) || 'Unknown'; - const setStateComponentName = - getComponentName(fiber.type) || 'Unknown'; - const dedupeKey = - renderingComponentName + ' ' + setStateComponentName; + // Dedupe by the rendering component because it's the one that needs to be fixed. + const dedupeKey = renderingComponentName; if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) { didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey); + const setStateComponentName = + getComponentName(fiber.type) || 'Unknown'; console.error( 'Cannot update a component (`%s`) while rendering a ' + 'different component (`%s`). To locate the bad setState() call inside `%s`, ' + From 933157ffd712580ec6ab21b10085d9b096199c26 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 17 Mar 2020 20:58:18 +0000 Subject: [PATCH 3/3] Don't warn outside of render --- .../__tests__/ReactCompositeComponent-test.js | 110 ++++++++++++++++++ .../src/ReactFiberBeginWork.js | 2 + .../src/ReactFiberWorkLoop.js | 12 +- 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 49e107f877ef..63cd1f97c531 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -1749,4 +1749,114 @@ describe('ReactCompositeComponent', () => { ReactDOM.render(, container); expect(container.firstChild.tagName).toBe('DIV'); }); + + it('should not warn on updating function component from componentWillMount', () => { + let _setState; + function A() { + _setState = React.useState()[1]; + return null; + } + class B extends React.Component { + UNSAFE_componentWillMount() { + _setState({}); + } + render() { + return null; + } + } + function Parent() { + return ( +
+ + +
+ ); + } + const container = document.createElement('div'); + ReactDOM.render(, container); + }); + + it('should not warn on updating function component from componentWillUpdate', () => { + let _setState; + function A() { + _setState = React.useState()[1]; + return null; + } + class B extends React.Component { + UNSAFE_componentWillUpdate() { + _setState({}); + } + render() { + return null; + } + } + function Parent() { + return ( + + ); + } + const container = document.createElement('div'); + ReactDOM.render(, container); + ReactDOM.render(, container); + }); + + it('should not warn on updating function component from componentWillReceiveProps', () => { + let _setState; + function A() { + _setState = React.useState()[1]; + return null; + } + class B extends React.Component { + UNSAFE_componentWillReceiveProps() { + _setState({}); + } + render() { + return null; + } + } + function Parent() { + return ( + + ); + } + const container = document.createElement('div'); + ReactDOM.render(, container); + ReactDOM.render(, container); + }); + + it('should warn on updating function component from render', () => { + let _setState; + function A() { + _setState = React.useState()[1]; + return null; + } + class B extends React.Component { + render() { + _setState({}); + return null; + } + } + function Parent() { + return ( + + ); + } + const container = document.createElement('div'); + expect(() => { + ReactDOM.render(, container); + }).toErrorDev( + 'Cannot update a component (`A`) while rendering a different component (`B`)', + ); + // Dedupe. + ReactDOM.render(, container); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 99dc79eb2fad..207429a3d7a8 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -1363,6 +1363,7 @@ function mountIndeterminateComponent( ReactStrictModeWarnings.recordLegacyContextWarning(workInProgress, null); } + setIsRendering(true); ReactCurrentOwner.current = workInProgress; value = renderWithHooks( null, @@ -1372,6 +1373,7 @@ function mountIndeterminateComponent( context, renderExpirationTime, ); + setIsRendering(false); } else { value = renderWithHooks( null, diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 3883923352d3..c7a867864a16 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2927,7 +2927,10 @@ if (__DEV__) { function warnAboutRenderPhaseUpdatesInDEV(fiber) { if (__DEV__) { - if ((executionContext & RenderContext) !== NoContext) { + if ( + ReactCurrentDebugFiberIsRenderingInDEV && + (executionContext & RenderContext) !== NoContext + ) { switch (fiber.tag) { case FunctionComponent: case ForwardRef: @@ -2953,18 +2956,15 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) { break; } case ClassComponent: { - if ( - ReactCurrentDebugFiberIsRenderingInDEV && - !didWarnAboutUpdateInRender - ) { + if (!didWarnAboutUpdateInRender) { console.error( 'Cannot update during an existing state transition (such as ' + 'within `render`). Render methods should be a pure ' + 'function of props and state.', ); didWarnAboutUpdateInRender = true; - break; } + break; } } }