@samitouri / QOS-React / commits / fe1f79b95b

Don't fire the render phase update warning for class lifecycles (#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

Dan Abramov committed Mar 18, 2020 at 00:07 UTC fe1f79b95b5987b7179d1eaf5ff5bb9a7bc4b7b5
5 files changed +127 -15
packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
+110
@@ -1749,4 +1749,114 @@ describe('ReactCompositeComponent', () => {
1749 ReactDOM.render(<Shadow />, container);
1750 expect(container.firstChild.tagName).toBe('DIV');
1751 });
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 + });
1862 });
packages/react-reconciler/src/ReactFiberBeginWork.js
+2
@@ -1363,6 +1363,7 @@ function mountIndeterminateComponent(
1363 ReactStrictModeWarnings.recordLegacyContextWarning(workInProgress, null);
1364 }
1365
1366 + setIsRendering(true);
1367 ReactCurrentOwner.current = workInProgress;
1368 value = renderWithHooks(
1369 null,
@@ -1372,6 +1373,7 @@ function mountIndeterminateComponent(
1373 context,
1374 renderExpirationTime,
1375 );
1376 + setIsRendering(false);
1377 } else {
1378 value = renderWithHooks(
1379 null,
packages/react-reconciler/src/ReactFiberWorkLoop.js
+11 -11
@@ -2927,7 +2927,10 @@ if (__DEV__) {
2927
2928 function warnAboutRenderPhaseUpdatesInDEV(fiber) {
2929 if (__DEV__) {
2930 - if ((executionContext & RenderContext) !== NoContext) {
2930 + if (
2931 + ReactCurrentDebugFiberIsRenderingInDEV &&
2932 + (executionContext & RenderContext) !== NoContext
2933 + ) {
2934 switch (fiber.tag) {
2935 case FunctionComponent:
2936 case ForwardRef:
@@ -2935,14 +2938,14 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
2938 const renderingComponentName =
2939 (workInProgress && getComponentName(workInProgress.type)) ||
2940 'Unknown';
2938 - const setStateComponentName =
2939 - getComponentName(fiber.type) || 'Unknown';
2940 - const dedupeKey =
2941 - renderingComponentName + ' ' + setStateComponentName;
2941 + // Dedupe by the rendering component because it's the one that needs to be fixed.
2942 + const dedupeKey = renderingComponentName;
2943 if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) {
2944 didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey);
2945 + const setStateComponentName =
2946 + getComponentName(fiber.type) || 'Unknown';
2947 console.error(
2945 - 'Cannot update a component (`%s`) from inside the function body of a ' +
2948 + 'Cannot update a component (`%s`) while rendering a ' +
2949 'different component (`%s`). To locate the bad setState() call inside `%s`, ' +
2950 'follow the stack trace as described in https://fb.me/setstate-in-render',
2951 setStateComponentName,
@@ -2953,18 +2956,15 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
2956 break;
2957 }
2958 case ClassComponent: {
2956 - if (
2957 - ReactCurrentDebugFiberIsRenderingInDEV &&
2958 - !didWarnAboutUpdateInRender
2959 - ) {
2959 + if (!didWarnAboutUpdateInRender) {
2960 console.error(
2961 'Cannot update during an existing state transition (such as ' +
2962 'within `render`). Render methods should be a pure ' +
2963 'function of props and state.',
2964 );
2965 didWarnAboutUpdateInRender = true;
2966 - break;
2966 }
2967 + break;
2968 }
2969 }
2970 }
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+3 -3
@@ -1087,7 +1087,7 @@ describe('ReactHooks', () => {
1087 ),
1088 ).toErrorDev([
1089 '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`).',
1091 ]);
1092 });
1093
@@ -1783,8 +1783,8 @@ describe('ReactHooks', () => {
1783 if (__DEV__) {
1784 expect(console.error).toHaveBeenCalledTimes(2);
1785 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`).',
1788 );
1789 }
1790 });
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+1 -1
@@ -476,7 +476,7 @@ describe('ReactHooksWithNoopRenderer', () => {
476 expect(() =>
477 expect(Scheduler).toFlushAndYield(['Foo [0]', 'Bar', 'Foo [1]']),
478 ).toErrorDev([
479 - 'Cannot update a component (`Foo`) from inside the function body of a ' +
479 + 'Cannot update a component (`Foo`) while rendering a ' +
480 'different component (`Bar`). To locate the bad setState() call inside `Bar`',
481 ]);
482 });