Skip to content

Commit 2353a58

Browse files
gaearonacdlite
authored andcommitted
Don't fire the render phase update warning for class lifecycles (facebook#18330)
* Change the warning to not say "function body" This warning is more generic and may happen with class components too. * Dedupe by the rendering component * Don't warn outside of render
1 parent 8205a00 commit 2353a58

File tree

5 files changed

+126
-10
lines changed

5 files changed

+126
-10
lines changed

packages/react-dom/src/__tests__/ReactCompositeComponent-test.js

+110
Original file line numberDiff line numberDiff line change
@@ -1786,4 +1786,114 @@ describe('ReactCompositeComponent', () => {
17861786
ReactDOM.render(<Shadow />, container);
17871787
expect(container.firstChild.tagName).toBe('DIV');
17881788
});
1789+
1790+
it('should not warn on updating function component from componentWillMount', () => {
1791+
let _setState;
1792+
function A() {
1793+
_setState = React.useState()[1];
1794+
return null;
1795+
}
1796+
class B extends React.Component {
1797+
UNSAFE_componentWillMount() {
1798+
_setState({});
1799+
}
1800+
render() {
1801+
return null;
1802+
}
1803+
}
1804+
function Parent() {
1805+
return (
1806+
<div>
1807+
<A />
1808+
<B />
1809+
</div>
1810+
);
1811+
}
1812+
const container = document.createElement('div');
1813+
ReactDOM.render(<Parent />, container);
1814+
});
1815+
1816+
it('should not warn on updating function component from componentWillUpdate', () => {
1817+
let _setState;
1818+
function A() {
1819+
_setState = React.useState()[1];
1820+
return null;
1821+
}
1822+
class B extends React.Component {
1823+
UNSAFE_componentWillUpdate() {
1824+
_setState({});
1825+
}
1826+
render() {
1827+
return null;
1828+
}
1829+
}
1830+
function Parent() {
1831+
return (
1832+
<div>
1833+
<A />
1834+
<B />
1835+
</div>
1836+
);
1837+
}
1838+
const container = document.createElement('div');
1839+
ReactDOM.render(<Parent />, container);
1840+
ReactDOM.render(<Parent />, container);
1841+
});
1842+
1843+
it('should not warn on updating function component from componentWillReceiveProps', () => {
1844+
let _setState;
1845+
function A() {
1846+
_setState = React.useState()[1];
1847+
return null;
1848+
}
1849+
class B extends React.Component {
1850+
UNSAFE_componentWillReceiveProps() {
1851+
_setState({});
1852+
}
1853+
render() {
1854+
return null;
1855+
}
1856+
}
1857+
function Parent() {
1858+
return (
1859+
<div>
1860+
<A />
1861+
<B />
1862+
</div>
1863+
);
1864+
}
1865+
const container = document.createElement('div');
1866+
ReactDOM.render(<Parent />, container);
1867+
ReactDOM.render(<Parent />, container);
1868+
});
1869+
1870+
it('should warn on updating function component from render', () => {
1871+
let _setState;
1872+
function A() {
1873+
_setState = React.useState()[1];
1874+
return null;
1875+
}
1876+
class B extends React.Component {
1877+
render() {
1878+
_setState({});
1879+
return null;
1880+
}
1881+
}
1882+
function Parent() {
1883+
return (
1884+
<div>
1885+
<A />
1886+
<B />
1887+
</div>
1888+
);
1889+
}
1890+
const container = document.createElement('div');
1891+
expect(() => {
1892+
ReactDOM.render(<Parent />, container);
1893+
}).toErrorDev(
1894+
'Cannot update a component (`A`) while rendering a different component (`B`)',
1895+
);
1896+
// Dedupe.
1897+
ReactDOM.render(<Parent />, container);
1898+
});
17891899
});

packages/react-reconciler/src/ReactFiberBeginWork.js

+2
Original file line numberDiff line numberDiff line change
@@ -1358,6 +1358,7 @@ function mountIndeterminateComponent(
13581358
ReactStrictModeWarnings.recordLegacyContextWarning(workInProgress, null);
13591359
}
13601360

1361+
setIsRendering(true);
13611362
ReactCurrentOwner.current = workInProgress;
13621363
value = renderWithHooks(
13631364
null,
@@ -1367,6 +1368,7 @@ function mountIndeterminateComponent(
13671368
context,
13681369
renderExpirationTime,
13691370
);
1371+
setIsRendering(false);
13701372
} else {
13711373
value = renderWithHooks(
13721374
null,

packages/react-reconciler/src/ReactFiberWorkLoop.js

+10-6
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ import {
156156
import getComponentName from 'shared/getComponentName';
157157
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
158158
import {
159+
isRendering as ReactCurrentDebugFiberIsRenderingInDEV,
159160
phase as ReactCurrentDebugFiberPhaseInDEV,
160161
resetCurrentFiber as resetCurrentDebugFiberInDEV,
161162
setCurrentFiber as setCurrentDebugFiberInDEV,
@@ -2801,22 +2802,25 @@ if (__DEV__) {
28012802

28022803
function warnAboutRenderPhaseUpdatesInDEV(fiber) {
28032804
if (__DEV__) {
2804-
if ((executionContext & RenderContext) !== NoContext) {
2805+
if (
2806+
ReactCurrentDebugFiberIsRenderingInDEV &&
2807+
(executionContext & RenderContext) !== NoContext
2808+
) {
28052809
switch (fiber.tag) {
28062810
case FunctionComponent:
28072811
case ForwardRef:
28082812
case SimpleMemoComponent: {
28092813
const renderingComponentName =
28102814
(workInProgress && getComponentName(workInProgress.type)) ||
28112815
'Unknown';
2812-
const setStateComponentName =
2813-
getComponentName(fiber.type) || 'Unknown';
2814-
const dedupeKey =
2815-
renderingComponentName + ' ' + setStateComponentName;
2816+
// Dedupe by the rendering component because it's the one that needs to be fixed.
2817+
const dedupeKey = renderingComponentName;
28162818
if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) {
28172819
didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey);
2820+
const setStateComponentName =
2821+
getComponentName(fiber.type) || 'Unknown';
28182822
console.error(
2819-
'Cannot update a component (`%s`) from inside the function body of a ' +
2823+
'Cannot update a component (`%s`) while rendering a ' +
28202824
'different component (`%s`). To locate the bad setState() call inside `%s`, ' +
28212825
'follow the stack trace as described in https://fb.me/setstate-in-render',
28222826
setStateComponentName,

packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1087,7 +1087,7 @@ describe('ReactHooks', () => {
10871087
),
10881088
).toErrorDev([
10891089
'Context can only be read while React is rendering',
1090-
'Cannot update a component (`Fn`) from inside the function body of a different component (`Cls`).',
1090+
'Cannot update a component (`Fn`) while rendering a different component (`Cls`).',
10911091
]);
10921092
});
10931093

@@ -1783,8 +1783,8 @@ describe('ReactHooks', () => {
17831783
if (__DEV__) {
17841784
expect(console.error).toHaveBeenCalledTimes(2);
17851785
expect(console.error.calls.argsFor(0)[0]).toContain(
1786-
'Warning: Cannot update a component (`%s`) from inside the function body ' +
1787-
'of a different component (`%s`).',
1786+
'Warning: Cannot update a component (`%s`) while rendering ' +
1787+
'a different component (`%s`).',
17881788
);
17891789
}
17901790
});

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ function loadModules({
458458
expect(() =>
459459
expect(Scheduler).toFlushAndYield(['Foo [0]', 'Bar', 'Foo [1]']),
460460
).toErrorDev([
461-
'Cannot update a component (`Foo`) from inside the function body of a ' +
461+
'Cannot update a component (`Foo`) while rendering a ' +
462462
'different component (`Bar`). To locate the bad setState() call inside `Bar`',
463463
]);
464464
});

0 commit comments

Comments
 (0)