Skip to content

Commit 96f3b7d

Browse files
sebmarkbagesebmarkbage
authored
Warn if calling setState outside of render but before commit (#18838)
* Don't attempt to render the children of a dehydrated Suspense boundary The DehydratedFragment tag doesn't exist so doing so throws. This can happen if we schedule childExpirationTime on the boundary and bail out. * Warn if scheduling work on a component before it is committed
1 parent 33c3af2 commit 96f3b7d

5 files changed

Lines changed: 219 additions & 2 deletions

File tree

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,75 @@ describe('ReactDOMServerPartialHydration', () => {
853853
expect(span.className).toBe('hi');
854854
});
855855

856+
// @gate experimental
857+
it('warns but works if setState is called before commit in a dehydrated component', async () => {
858+
let suspend = false;
859+
let resolve;
860+
const promise = new Promise(resolvePromise => (resolve = resolvePromise));
861+
862+
let updateText;
863+
864+
function Child() {
865+
const [state, setState] = React.useState('Hello');
866+
updateText = setState;
867+
Scheduler.unstable_yieldValue('Child');
868+
if (suspend) {
869+
throw promise;
870+
} else {
871+
return state;
872+
}
873+
}
874+
875+
function Sibling() {
876+
Scheduler.unstable_yieldValue('Sibling');
877+
return null;
878+
}
879+
880+
function App() {
881+
return (
882+
<div>
883+
<Suspense fallback="Loading...">
884+
<Child />
885+
<Sibling />
886+
</Suspense>
887+
</div>
888+
);
889+
}
890+
891+
suspend = false;
892+
const finalHTML = ReactDOMServer.renderToString(<App />);
893+
expect(Scheduler).toHaveYielded(['Child', 'Sibling']);
894+
895+
const container = document.createElement('div');
896+
container.innerHTML = finalHTML;
897+
898+
const root = ReactDOM.createRoot(container, {hydrate: true});
899+
900+
await act(async () => {
901+
suspend = true;
902+
root.render(<App />);
903+
expect(Scheduler).toFlushAndYieldThrough(['Child']);
904+
905+
// While we're part way through the hydration, we update the state.
906+
// This will schedule an update on the children of the suspense boundary.
907+
expect(() => updateText('Hi')).toErrorDev(
908+
"Can't perform a React state update on a component that hasn't mounted yet.",
909+
);
910+
911+
// This will throw it away and rerender.
912+
expect(Scheduler).toFlushAndYield(['Child', 'Sibling']);
913+
914+
expect(container.textContent).toBe('Hello');
915+
916+
suspend = false;
917+
resolve();
918+
await promise;
919+
});
920+
expect(Scheduler).toHaveYielded(['Child', 'Sibling']);
921+
922+
expect(container.textContent).toBe('Hello');
923+
});
924+
856925
// @gate experimental
857926
it('blocks the update to hydrate first if context has changed', async () => {
858927
let suspend = false;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3112,7 +3112,9 @@ function beginWork(
31123112
// been unsuspended it has committed as a resolved Suspense component.
31133113
// If it needs to be retried, it should have work scheduled on it.
31143114
workInProgress.effectTag |= DidCapture;
3115-
break;
3115+
// We should never render the children of a dehydrated boundary until we
3116+
// upgrade it. We return null instead of bailoutOnAlreadyFinishedWork.
3117+
return null;
31163118
}
31173119
}
31183120

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,8 @@ function updateHostComponent(current, workInProgress, renderExpirationTime) {
11101110
}
11111111
// Schedule this fiber to re-render at offscreen priority. Then bailout.
11121112
workInProgress.expirationTime = workInProgress.childExpirationTime = Never;
1113+
// We should never render the children of a dehydrated boundary until we
1114+
// upgrade it. We return null instead of bailoutOnAlreadyFinishedWork.
11131115
return null;
11141116
}
11151117

@@ -3128,7 +3130,8 @@ function beginWork(
31283130
// been unsuspended it has committed as a resolved Suspense component.
31293131
// If it needs to be retried, it should have work scheduled on it.
31303132
workInProgress.effectTag |= DidCapture;
3131-
break;
3133+
3134+
return null;
31323135
}
31333136
}
31343137

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import {
7676
} from './ReactTypeOfMode';
7777
import {
7878
HostRoot,
79+
IndeterminateComponent,
7980
ClassComponent,
8081
SuspenseComponent,
8182
SuspenseListComponent,
@@ -500,6 +501,14 @@ function markUpdateLaneFromFiberToRoot(
500501
if (alternate !== null) {
501502
alternate.lanes = mergeLanes(alternate.lanes, lane);
502503
}
504+
if (__DEV__) {
505+
if (
506+
alternate === null &&
507+
(fiber.effectTag & (Placement | Hydrating)) !== NoEffect
508+
) {
509+
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
510+
}
511+
}
503512
// Walk the parent path to the root and update the child expiration time.
504513
let node = fiber.return;
505514
let root = null;
@@ -508,6 +517,14 @@ function markUpdateLaneFromFiberToRoot(
508517
} else {
509518
while (node !== null) {
510519
alternate = node.alternate;
520+
if (__DEV__) {
521+
if (
522+
alternate === null &&
523+
(node.effectTag & (Placement | Hydrating)) !== NoEffect
524+
) {
525+
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
526+
}
527+
}
511528
node.childLanes = mergeLanes(node.childLanes, lane);
512529
if (alternate !== null) {
513530
alternate.childLanes = mergeLanes(alternate.childLanes, lane);
@@ -2710,6 +2727,60 @@ function flushRenderPhaseStrictModeWarningsInDEV() {
27102727
}
27112728
}
27122729

2730+
let didWarnStateUpdateForNotYetMountedComponent: Set<string> | null = null;
2731+
function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
2732+
if (__DEV__) {
2733+
if ((executionContext & RenderContext) !== NoContext) {
2734+
// We let the other warning about render phase updates deal with this one.
2735+
return;
2736+
}
2737+
2738+
const tag = fiber.tag;
2739+
if (
2740+
tag !== IndeterminateComponent &&
2741+
tag !== HostRoot &&
2742+
tag !== ClassComponent &&
2743+
tag !== FunctionComponent &&
2744+
tag !== ForwardRef &&
2745+
tag !== MemoComponent &&
2746+
tag !== SimpleMemoComponent &&
2747+
tag !== Block
2748+
) {
2749+
// Only warn for user-defined components, not internal ones like Suspense.
2750+
return;
2751+
}
2752+
2753+
// We show the whole stack but dedupe on the top component's name because
2754+
// the problematic code almost always lies inside that component.
2755+
const componentName = getComponentName(fiber.type) || 'ReactComponent';
2756+
if (didWarnStateUpdateForNotYetMountedComponent !== null) {
2757+
if (didWarnStateUpdateForNotYetMountedComponent.has(componentName)) {
2758+
return;
2759+
}
2760+
didWarnStateUpdateForNotYetMountedComponent.add(componentName);
2761+
} else {
2762+
didWarnStateUpdateForNotYetMountedComponent = new Set([componentName]);
2763+
}
2764+
2765+
const previousFiber = ReactCurrentFiberCurrent;
2766+
try {
2767+
setCurrentDebugFiberInDEV(fiber);
2768+
console.error(
2769+
"Can't perform a React state update on a component that hasn't mounted yet. " +
2770+
'This indicates that you have a side-effect in your render function that ' +
2771+
'asynchronously later calls tries to update the component. Move this work to ' +
2772+
'useEffect instead.',
2773+
);
2774+
} finally {
2775+
if (previousFiber) {
2776+
setCurrentDebugFiberInDEV(fiber);
2777+
} else {
2778+
resetCurrentDebugFiberInDEV();
2779+
}
2780+
}
2781+
}
2782+
}
2783+
27132784
let didWarnStateUpdateForUnmountedComponent: Set<string> | null = null;
27142785
function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
27152786
if (__DEV__) {

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ import {
9696
} from './ReactTypeOfMode';
9797
import {
9898
HostRoot,
99+
IndeterminateComponent,
99100
ClassComponent,
100101
SuspenseComponent,
101102
SuspenseListComponent,
@@ -498,6 +499,15 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) {
498499
if (alternate !== null && alternate.expirationTime < expirationTime) {
499500
alternate.expirationTime = expirationTime;
500501
}
502+
if (__DEV__) {
503+
if (
504+
alternate === null &&
505+
(fiber.effectTag & (Placement | Hydrating)) !== NoEffect
506+
) {
507+
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
508+
}
509+
}
510+
501511
// Walk the parent path to the root and update the child expiration time.
502512
let node = fiber.return;
503513
let root = null;
@@ -506,6 +516,14 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) {
506516
} else {
507517
while (node !== null) {
508518
alternate = node.alternate;
519+
if (__DEV__) {
520+
if (
521+
alternate === null &&
522+
(node.effectTag & (Placement | Hydrating)) !== NoEffect
523+
) {
524+
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
525+
}
526+
}
509527
if (node.childExpirationTime < expirationTime) {
510528
node.childExpirationTime = expirationTime;
511529
if (
@@ -2901,6 +2919,60 @@ function flushRenderPhaseStrictModeWarningsInDEV() {
29012919
}
29022920
}
29032921

2922+
let didWarnStateUpdateForNotYetMountedComponent: Set<string> | null = null;
2923+
function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
2924+
if (__DEV__) {
2925+
if ((executionContext & RenderContext) !== NoContext) {
2926+
// We let the other warning about render phase updates deal with this one.
2927+
return;
2928+
}
2929+
2930+
const tag = fiber.tag;
2931+
if (
2932+
tag !== IndeterminateComponent &&
2933+
tag !== HostRoot &&
2934+
tag !== ClassComponent &&
2935+
tag !== FunctionComponent &&
2936+
tag !== ForwardRef &&
2937+
tag !== MemoComponent &&
2938+
tag !== SimpleMemoComponent &&
2939+
tag !== Block
2940+
) {
2941+
// Only warn for user-defined components, not internal ones like Suspense.
2942+
return;
2943+
}
2944+
2945+
// We show the whole stack but dedupe on the top component's name because
2946+
// the problematic code almost always lies inside that component.
2947+
const componentName = getComponentName(fiber.type) || 'ReactComponent';
2948+
if (didWarnStateUpdateForNotYetMountedComponent !== null) {
2949+
if (didWarnStateUpdateForNotYetMountedComponent.has(componentName)) {
2950+
return;
2951+
}
2952+
didWarnStateUpdateForNotYetMountedComponent.add(componentName);
2953+
} else {
2954+
didWarnStateUpdateForNotYetMountedComponent = new Set([componentName]);
2955+
}
2956+
2957+
const previousFiber = ReactCurrentFiberCurrent;
2958+
try {
2959+
setCurrentDebugFiberInDEV(fiber);
2960+
console.error(
2961+
"Can't perform a React state update on a component that hasn't mounted yet. " +
2962+
'This indicates that you have a side-effect in your render function that ' +
2963+
'asynchronously later calls tries to update the component. Move this work to ' +
2964+
'useEffect instead.',
2965+
);
2966+
} finally {
2967+
if (previousFiber) {
2968+
setCurrentDebugFiberInDEV(fiber);
2969+
} else {
2970+
resetCurrentDebugFiberInDEV();
2971+
}
2972+
}
2973+
}
2974+
}
2975+
29042976
let didWarnStateUpdateForUnmountedComponent: Set<string> | null = null;
29052977
function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
29062978
if (__DEV__) {

0 commit comments

Comments
 (0)