Skip to content

Commit

Permalink
Warn if calling setState outside of render but before commit (#18838)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@sebmarkbage
sebmarkbage committed May 6, 2020
1 parent 33c3af2 commit 96f3b7d
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 2 deletions.
69 changes: 69 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Expand Up @@ -853,6 +853,75 @@ describe('ReactDOMServerPartialHydration', () => {
expect(span.className).toBe('hi');
});

// @gate experimental
it('warns but works if setState is called before commit in a dehydrated component', async () => {
let suspend = false;
let resolve;
const promise = new Promise(resolvePromise => (resolve = resolvePromise));

let updateText;

function Child() {
const [state, setState] = React.useState('Hello');
updateText = setState;
Scheduler.unstable_yieldValue('Child');
if (suspend) {
throw promise;
} else {
return state;
}
}

function Sibling() {
Scheduler.unstable_yieldValue('Sibling');
return null;
}

function App() {
return (
<div>
<Suspense fallback="Loading...">
<Child />
<Sibling />
</Suspense>
</div>
);
}

suspend = false;
const finalHTML = ReactDOMServer.renderToString(<App />);
expect(Scheduler).toHaveYielded(['Child', 'Sibling']);

const container = document.createElement('div');
container.innerHTML = finalHTML;

const root = ReactDOM.createRoot(container, {hydrate: true});

await act(async () => {
suspend = true;
root.render(<App />);
expect(Scheduler).toFlushAndYieldThrough(['Child']);

// While we're part way through the hydration, we update the state.
// This will schedule an update on the children of the suspense boundary.
expect(() => updateText('Hi')).toErrorDev(
"Can't perform a React state update on a component that hasn't mounted yet.",
);

// This will throw it away and rerender.
expect(Scheduler).toFlushAndYield(['Child', 'Sibling']);

expect(container.textContent).toBe('Hello');

suspend = false;
resolve();
await promise;
});
expect(Scheduler).toHaveYielded(['Child', 'Sibling']);

expect(container.textContent).toBe('Hello');
});

// @gate experimental
it('blocks the update to hydrate first if context has changed', async () => {
let suspend = false;
Expand Down
4 changes: 3 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Expand Up @@ -3112,7 +3112,9 @@ function beginWork(
// been unsuspended it has committed as a resolved Suspense component.
// If it needs to be retried, it should have work scheduled on it.
workInProgress.effectTag |= DidCapture;
break;
// We should never render the children of a dehydrated boundary until we
// upgrade it. We return null instead of bailoutOnAlreadyFinishedWork.
return null;
}
}

Expand Down
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Expand Up @@ -1110,6 +1110,8 @@ function updateHostComponent(current, workInProgress, renderExpirationTime) {
}
// Schedule this fiber to re-render at offscreen priority. Then bailout.
workInProgress.expirationTime = workInProgress.childExpirationTime = Never;
// We should never render the children of a dehydrated boundary until we
// upgrade it. We return null instead of bailoutOnAlreadyFinishedWork.
return null;
}

Expand Down Expand Up @@ -3128,7 +3130,8 @@ function beginWork(
// been unsuspended it has committed as a resolved Suspense component.
// If it needs to be retried, it should have work scheduled on it.
workInProgress.effectTag |= DidCapture;
break;

return null;
}
}

Expand Down
71 changes: 71 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Expand Up @@ -76,6 +76,7 @@ import {
} from './ReactTypeOfMode';
import {
HostRoot,
IndeterminateComponent,
ClassComponent,
SuspenseComponent,
SuspenseListComponent,
Expand Down Expand Up @@ -500,6 +501,14 @@ function markUpdateLaneFromFiberToRoot(
if (alternate !== null) {
alternate.lanes = mergeLanes(alternate.lanes, lane);
}
if (__DEV__) {
if (
alternate === null &&
(fiber.effectTag & (Placement | Hydrating)) !== NoEffect
) {
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
}
}
// Walk the parent path to the root and update the child expiration time.
let node = fiber.return;
let root = null;
Expand All @@ -508,6 +517,14 @@ function markUpdateLaneFromFiberToRoot(
} else {
while (node !== null) {
alternate = node.alternate;
if (__DEV__) {
if (
alternate === null &&
(node.effectTag & (Placement | Hydrating)) !== NoEffect
) {
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
}
}
node.childLanes = mergeLanes(node.childLanes, lane);
if (alternate !== null) {
alternate.childLanes = mergeLanes(alternate.childLanes, lane);
Expand Down Expand Up @@ -2710,6 +2727,60 @@ function flushRenderPhaseStrictModeWarningsInDEV() {
}
}

let didWarnStateUpdateForNotYetMountedComponent: Set<string> | null = null;
function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
if (__DEV__) {
if ((executionContext & RenderContext) !== NoContext) {
// We let the other warning about render phase updates deal with this one.
return;
}

const tag = fiber.tag;
if (
tag !== IndeterminateComponent &&
tag !== HostRoot &&
tag !== ClassComponent &&
tag !== FunctionComponent &&
tag !== ForwardRef &&
tag !== MemoComponent &&
tag !== SimpleMemoComponent &&
tag !== Block
) {
// Only warn for user-defined components, not internal ones like Suspense.
return;
}

// We show the whole stack but dedupe on the top component's name because
// the problematic code almost always lies inside that component.
const componentName = getComponentName(fiber.type) || 'ReactComponent';
if (didWarnStateUpdateForNotYetMountedComponent !== null) {
if (didWarnStateUpdateForNotYetMountedComponent.has(componentName)) {
return;
}
didWarnStateUpdateForNotYetMountedComponent.add(componentName);
} else {
didWarnStateUpdateForNotYetMountedComponent = new Set([componentName]);
}

const previousFiber = ReactCurrentFiberCurrent;
try {
setCurrentDebugFiberInDEV(fiber);
console.error(
"Can't perform a React state update on a component that hasn't mounted yet. " +
'This indicates that you have a side-effect in your render function that ' +
'asynchronously later calls tries to update the component. Move this work to ' +
'useEffect instead.',
);
} finally {
if (previousFiber) {
setCurrentDebugFiberInDEV(fiber);
} else {
resetCurrentDebugFiberInDEV();
}
}
}
}

let didWarnStateUpdateForUnmountedComponent: Set<string> | null = null;
function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
if (__DEV__) {
Expand Down
72 changes: 72 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Expand Up @@ -96,6 +96,7 @@ import {
} from './ReactTypeOfMode';
import {
HostRoot,
IndeterminateComponent,
ClassComponent,
SuspenseComponent,
SuspenseListComponent,
Expand Down Expand Up @@ -498,6 +499,15 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) {
if (alternate !== null && alternate.expirationTime < expirationTime) {
alternate.expirationTime = expirationTime;
}
if (__DEV__) {
if (
alternate === null &&
(fiber.effectTag & (Placement | Hydrating)) !== NoEffect
) {
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
}
}

// Walk the parent path to the root and update the child expiration time.
let node = fiber.return;
let root = null;
Expand All @@ -506,6 +516,14 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) {
} else {
while (node !== null) {
alternate = node.alternate;
if (__DEV__) {
if (
alternate === null &&
(node.effectTag & (Placement | Hydrating)) !== NoEffect
) {
warnAboutUpdateOnNotYetMountedFiberInDEV(fiber);
}
}
if (node.childExpirationTime < expirationTime) {
node.childExpirationTime = expirationTime;
if (
Expand Down Expand Up @@ -2901,6 +2919,60 @@ function flushRenderPhaseStrictModeWarningsInDEV() {
}
}

let didWarnStateUpdateForNotYetMountedComponent: Set<string> | null = null;
function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
if (__DEV__) {
if ((executionContext & RenderContext) !== NoContext) {
// We let the other warning about render phase updates deal with this one.
return;
}

const tag = fiber.tag;
if (
tag !== IndeterminateComponent &&
tag !== HostRoot &&
tag !== ClassComponent &&
tag !== FunctionComponent &&
tag !== ForwardRef &&
tag !== MemoComponent &&
tag !== SimpleMemoComponent &&
tag !== Block
) {
// Only warn for user-defined components, not internal ones like Suspense.
return;
}

// We show the whole stack but dedupe on the top component's name because
// the problematic code almost always lies inside that component.
const componentName = getComponentName(fiber.type) || 'ReactComponent';
if (didWarnStateUpdateForNotYetMountedComponent !== null) {
if (didWarnStateUpdateForNotYetMountedComponent.has(componentName)) {
return;
}
didWarnStateUpdateForNotYetMountedComponent.add(componentName);
} else {
didWarnStateUpdateForNotYetMountedComponent = new Set([componentName]);
}

const previousFiber = ReactCurrentFiberCurrent;
try {
setCurrentDebugFiberInDEV(fiber);
console.error(
"Can't perform a React state update on a component that hasn't mounted yet. " +
'This indicates that you have a side-effect in your render function that ' +
'asynchronously later calls tries to update the component. Move this work to ' +
'useEffect instead.',
);
} finally {
if (previousFiber) {
setCurrentDebugFiberInDEV(fiber);
} else {
resetCurrentDebugFiberInDEV();
}
}
}
}

let didWarnStateUpdateForUnmountedComponent: Set<string> | null = null;
function warnAboutUpdateOnUnmountedFiberInDEV(fiber) {
if (__DEV__) {
Expand Down

0 comments on commit 96f3b7d

Please sign in to comment.