Warn for update on different component in render (#17099)
This warning already exists for class components, but not for functions. It does not apply to render phase updates to the same component, which have special semantics that we do support.
Andrew Clark committed
Feb 19, 2020 at 12:43 UTC
ea6ed3dbbd32515e2d4d9783c358beceeadd4b1d
3 files changed
+98
-29
packages/react-reconciler/src/ReactFiberWorkLoop.js
+33
-19
@@ -380,7 +380,7 @@ export function scheduleUpdateOnFiber(
380
expirationTime: ExpirationTime,
381
) {
382
checkForNestedUpdates();
383
- warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber);
383
+ warnAboutRenderPhaseUpdatesInDEV(fiber);
384
385
const root = markUpdateTimeFromFiberToRoot(fiber, expirationTime);
386
if (root === null) {
@@ -2781,30 +2781,44 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
2781
2782
let didWarnAboutUpdateInRender = false;
2783
let didWarnAboutUpdateInGetChildContext = false;
2784
-function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
2784
+function warnAboutRenderPhaseUpdatesInDEV(fiber) {
2785
if (__DEV__) {
2786
- if (fiber.tag === ClassComponent) {
2787
- switch (ReactCurrentDebugFiberPhaseInDEV) {
2788
- case 'getChildContext':
2789
- if (didWarnAboutUpdateInGetChildContext) {
2790
- return;
2791
- }
2786
+ if ((executionContext & RenderContext) !== NoContext) {
2787
+ switch (fiber.tag) {
2788
+ case FunctionComponent:
2789
+ case ForwardRef:
2790
+ case SimpleMemoComponent: {
2791
console.error(
2793
- 'setState(...): Cannot call setState() inside getChildContext()',
2792
+ 'Cannot update a component from inside the function body of a ' +
2793
+ 'different component.',
2794
);
2795
- didWarnAboutUpdateInGetChildContext = true;
2795
break;
2797
- case 'render':
2798
- if (didWarnAboutUpdateInRender) {
2799
- return;
2796
+ }
2797
+ case ClassComponent: {
2798
+ switch (ReactCurrentDebugFiberPhaseInDEV) {
2799
+ case 'getChildContext':
2800
+ if (didWarnAboutUpdateInGetChildContext) {
2801
+ return;
2802
+ }
2803
+ console.error(
2804
+ 'setState(...): Cannot call setState() inside getChildContext()',
2805
+ );
2806
+ didWarnAboutUpdateInGetChildContext = true;
2807
+ break;
2808
+ case 'render':
2809
+ if (didWarnAboutUpdateInRender) {
2810
+ return;
2811
+ }
2812
+ console.error(
2813
+ 'Cannot update during an existing state transition (such as ' +
2814
+ 'within `render`). Render methods should be a pure ' +
2815
+ 'function of props and state.',
2816
+ );
2817
+ didWarnAboutUpdateInRender = true;
2818
+ break;
2819
}
2801
- console.error(
2802
- 'Cannot update during an existing state transition (such as ' +
2803
- 'within `render`). Render methods should be a pure function of ' +
2804
- 'props and state.',
2805
- );
2806
- didWarnAboutUpdateInRender = true;
2820
break;
2821
+ }
2822
}
2823
}
2824
}
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+21
-10
@@ -1085,7 +1085,10 @@ describe('ReactHooks', () => {
1085
<Cls />
1086
</>,
1087
),
1088
- ).toErrorDev(['Context can only be read while React is rendering']);
1088
+ ).toErrorDev([
1089
+ 'Context can only be read while React is rendering',
1090
+ 'Cannot update a component from inside the function body of a different component.',
1091
+ ]);
1092
});
1093
1094
it('warns when calling hooks inside useReducer', () => {
@@ -1749,8 +1752,9 @@ describe('ReactHooks', () => {
1752
});
1753
1754
// Regression test for #14674
1752
- it('does not swallow original error when updating another component in render phase', () => {
1755
+ it('does not swallow original error when updating another component in render phase', async () => {
1756
let {useState} = React;
1757
+ spyOnDev(console, 'error');
1758
1759
let _setState;
1760
function A() {
@@ -1760,22 +1764,29 @@ describe('ReactHooks', () => {
1764
}
1765
1766
function B() {
1763
- act(() =>
1764
- _setState(() => {
1765
- throw new Error('Hello');
1766
- }),
1767
- );
1767
+ _setState(() => {
1768
+ throw new Error('Hello');
1769
+ });
1770
return null;
1771
}
1772
1771
- expect(() =>
1773
+ await act(async () => {
1774
ReactTestRenderer.create(
1775
<>
1776
<A />
1777
<B />
1778
</>,
1777
- ),
1778
- ).toThrow('Hello');
1779
+ );
1780
+ expect(() => Scheduler.unstable_flushAll()).toThrow('Hello');
1781
+ });
1782
+
1783
+ if (__DEV__) {
1784
+ expect(console.error).toHaveBeenCalledTimes(2);
1785
+ expect(console.error.calls.argsFor(0)[0]).toContain(
1786
+ 'Warning: Cannot update a component from inside the function body ' +
1787
+ 'of a different component.%s',
1788
+ );
1789
+ }
1790
});
1791
1792
// Regression test for https://github.com/facebook/react/issues/15057
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+44
@@ -420,6 +420,50 @@ function loadModules({
420
]);
421
});
422
423
+ it('warns about render phase update on a different component', async () => {
424
+ let setStep;
425
+ function Foo() {
426
+ const [step, _setStep] = useState(0);
427
+ setStep = _setStep;
428
+ return <Text text={`Foo [${step}]`} />;
429
+ }
430
+
431
+ function Bar({triggerUpdate}) {
432
+ if (triggerUpdate) {
433
+ setStep(1);
434
+ }
435
+ return <Text text="Bar" />;
436
+ }
437
+
438
+ const root = ReactNoop.createRoot();
439
+
440
+ await ReactNoop.act(async () => {
441
+ root.render(
442
+ <>
443
+ <Foo />
444
+ <Bar />
445
+ </>,
446
+ );
447
+ });
448
+ expect(Scheduler).toHaveYielded(['Foo [0]', 'Bar']);
449
+
450
+ // Bar will update Foo during its render phase. React should warn.
451
+ await ReactNoop.act(async () => {
452
+ root.render(
453
+ <>
454
+ <Foo />
455
+ <Bar triggerUpdate={true} />
456
+ </>,
457
+ );
458
+ expect(() =>
459
+ expect(Scheduler).toFlushAndYield(['Foo [0]', 'Bar', 'Foo [1]']),
460
+ ).toErrorDev([
461
+ 'Cannot update a component from inside the function body of a ' +
462
+ 'different component.',
463
+ ]);
464
+ });
465
+ });
466
+
467
it('keeps restarting until there are no more new updates', () => {
468
function Counter({row: newRow}) {
469
let [count, setCount] = useState(0);