Skip to content

Commit da83408

Browse files
gaearonacdlite
gaearon
authored and
acdlite
committed
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 7554ab7 commit da83408

File tree

5 files changed

+127
-15
lines changed

5 files changed

+127
-15
lines changed

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,4 +1749,114 @@ describe('ReactCompositeComponent', () => {
17491749
ReactDOM.render(<Shadow />, container);
17501750
expect(container.firstChild.tagName).toBe('DIV');
17511751
});
1752+
1753+
it('should not warn on updating function component from componentWillMount', () => {
1754+
let _setState;
1755+
function A() {
1756+
_setState = React.useState()[1];
1757+
return null;
1758+
}
1759+
class B extends React.Component {
1760+
UNSAFE_componentWillMount() {
1761+
_setState({});
1762+
}
1763+
render() {
1764+
return null;
1765+
}
1766+
}
1767+
function Parent() {
1768+
return (
1769+
<div>
1770+
<A />
1771+
<B />
1772+
</div>
1773+
);
1774+
}
1775+
const container = document.createElement('div');
1776+
ReactDOM.render(<Parent />, container);
1777+
});
1778+
1779+
it('should not warn on updating function component from componentWillUpdate', () => {
1780+
let _setState;
1781+
function A() {
1782+
_setState = React.useState()[1];
1783+
return null;
1784+
}
1785+
class B extends React.Component {
1786+
UNSAFE_componentWillUpdate() {
1787+
_setState({});
1788+
}
1789+
render() {
1790+
return null;
1791+
}
1792+
}
1793+
function Parent() {
1794+
return (
1795+
<div>
1796+
<A />
1797+
<B />
1798+
</div>
1799+
);
1800+
}
1801+
const container = document.createElement('div');
1802+
ReactDOM.render(<Parent />, container);
1803+
ReactDOM.render(<Parent />, container);
1804+
});
1805+
1806+
it('should not warn on updating function component from componentWillReceiveProps', () => {
1807+
let _setState;
1808+
function A() {
1809+
_setState = React.useState()[1];
1810+
return null;
1811+
}
1812+
class B extends React.Component {
1813+
UNSAFE_componentWillReceiveProps() {
1814+
_setState({});
1815+
}
1816+
render() {
1817+
return null;
1818+
}
1819+
}
1820+
function Parent() {
1821+
return (
1822+
<div>
1823+
<A />
1824+
<B />
1825+
</div>
1826+
);
1827+
}
1828+
const container = document.createElement('div');
1829+
ReactDOM.render(<Parent />, container);
1830+
ReactDOM.render(<Parent />, container);
1831+
});
1832+
1833+
it('should warn on updating function component from render', () => {
1834+
let _setState;
1835+
function A() {
1836+
_setState = React.useState()[1];
1837+
return null;
1838+
}
1839+
class B extends React.Component {
1840+
render() {
1841+
_setState({});
1842+
return null;
1843+
}
1844+
}
1845+
function Parent() {
1846+
return (
1847+
<div>
1848+
<A />
1849+
<B />
1850+
</div>
1851+
);
1852+
}
1853+
const container = document.createElement('div');
1854+
expect(() => {
1855+
ReactDOM.render(<Parent />, container);
1856+
}).toErrorDev(
1857+
'Cannot update a component (`A`) while rendering a different component (`B`)',
1858+
);
1859+
// Dedupe.
1860+
ReactDOM.render(<Parent />, container);
1861+
});
17521862
});

‎packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,6 +1356,7 @@ function mountIndeterminateComponent(
13561356
ReactStrictModeWarnings.recordLegacyContextWarning(workInProgress, null);
13571357
}
13581358

1359+
setIsRendering(true);
13591360
ReactCurrentOwner.current = workInProgress;
13601361
value = renderWithHooks(
13611362
null,
@@ -1365,6 +1366,7 @@ function mountIndeterminateComponent(
13651366
context,
13661367
renderExpirationTime,
13671368
);
1369+
setIsRendering(false);
13681370
} else {
13691371
value = renderWithHooks(
13701372
null,

‎packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2800,22 +2800,25 @@ if (__DEV__) {
28002800

28012801
function warnAboutRenderPhaseUpdatesInDEV(fiber) {
28022802
if (__DEV__) {
2803-
if ((executionContext & RenderContext) !== NoContext) {
2803+
if (
2804+
ReactCurrentDebugFiberIsRenderingInDEV &&
2805+
(executionContext & RenderContext) !== NoContext
2806+
) {
28042807
switch (fiber.tag) {
28052808
case FunctionComponent:
28062809
case ForwardRef:
28072810
case SimpleMemoComponent: {
28082811
const renderingComponentName =
28092812
(workInProgress && getComponentName(workInProgress.type)) ||
28102813
'Unknown';
2811-
const setStateComponentName =
2812-
getComponentName(fiber.type) || 'Unknown';
2813-
const dedupeKey =
2814-
renderingComponentName + ' ' + setStateComponentName;
2814+
// Dedupe by the rendering component because it's the one that needs to be fixed.
2815+
const dedupeKey = renderingComponentName;
28152816
if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) {
28162817
didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey);
2818+
const setStateComponentName =
2819+
getComponentName(fiber.type) || 'Unknown';
28172820
console.error(
2818-
'Cannot update a component (`%s`) from inside the function body of a ' +
2821+
'Cannot update a component (`%s`) while rendering a ' +
28192822
'different component (`%s`). To locate the bad setState() call inside `%s`, ' +
28202823
'follow the stack trace as described in https://fb.me/setstate-in-render',
28212824
setStateComponentName,
@@ -2826,18 +2829,15 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
28262829
break;
28272830
}
28282831
case ClassComponent: {
2829-
if (
2830-
ReactCurrentDebugFiberIsRenderingInDEV &&
2831-
!didWarnAboutUpdateInRender
2832-
) {
2832+
if (!didWarnAboutUpdateInRender) {
28332833
console.error(
28342834
'Cannot update during an existing state transition (such as ' +
28352835
'within `render`). Render methods should be a pure ' +
28362836
'function of props and state.',
28372837
);
28382838
didWarnAboutUpdateInRender = true;
2839-
break;
28402839
}
2840+
break;
28412841
}
28422842
}
28432843
}

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

Lines changed: 3 additions & 3 deletions
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

Lines changed: 1 addition & 1 deletion
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)