Skip to content

Commit ea6ed3d

Browse files
authored
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.
1 parent 085d021 commit ea6ed3d

File tree

3 files changed

+98
-29
lines changed

3 files changed

+98
-29
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.js

+33-19
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ export function scheduleUpdateOnFiber(
380380
expirationTime: ExpirationTime,
381381
) {
382382
checkForNestedUpdates();
383-
warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber);
383+
warnAboutRenderPhaseUpdatesInDEV(fiber);
384384

385385
const root = markUpdateTimeFromFiberToRoot(fiber, expirationTime);
386386
if (root === null) {
@@ -2781,30 +2781,44 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
27812781

27822782
let didWarnAboutUpdateInRender = false;
27832783
let didWarnAboutUpdateInGetChildContext = false;
2784-
function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
2784+
function warnAboutRenderPhaseUpdatesInDEV(fiber) {
27852785
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: {
27922791
console.error(
2793-
'setState(...): Cannot call setState() inside getChildContext()',
2792+
'Cannot update a component from inside the function body of a ' +
2793+
'different component.',
27942794
);
2795-
didWarnAboutUpdateInGetChildContext = true;
27962795
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;
28002819
}
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;
28072820
break;
2821+
}
28082822
}
28092823
}
28102824
}

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

+21-10
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,10 @@ describe('ReactHooks', () => {
10851085
<Cls />
10861086
</>,
10871087
),
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+
]);
10891092
});
10901093

10911094
it('warns when calling hooks inside useReducer', () => {
@@ -1749,8 +1752,9 @@ describe('ReactHooks', () => {
17491752
});
17501753

17511754
// 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 () => {
17531756
let {useState} = React;
1757+
spyOnDev(console, 'error');
17541758

17551759
let _setState;
17561760
function A() {
@@ -1760,22 +1764,29 @@ describe('ReactHooks', () => {
17601764
}
17611765

17621766
function B() {
1763-
act(() =>
1764-
_setState(() => {
1765-
throw new Error('Hello');
1766-
}),
1767-
);
1767+
_setState(() => {
1768+
throw new Error('Hello');
1769+
});
17681770
return null;
17691771
}
17701772

1771-
expect(() =>
1773+
await act(async () => {
17721774
ReactTestRenderer.create(
17731775
<>
17741776
<A />
17751777
<B />
17761778
</>,
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+
}
17791790
});
17801791

17811792
// Regression test for https://github.com/facebook/react/issues/15057

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

+44
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,50 @@ function loadModules({
420420
]);
421421
});
422422

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+
423467
it('keeps restarting until there are no more new updates', () => {
424468
function Counter({row: newRow}) {
425469
let [count, setCount] = useState(0);

0 commit comments

Comments
 (0)