Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Warn if calling setState outside of render but before commit #18838

Merged
merged 2 commits into from May 6, 2020
+219 −2
Merged

Warn if calling setState outside of render but before commit #18838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69c0952
Don't attempt to render the children of a dehydrated Suspense boundary
sebmarkbage May 6, 2020
2e018a9
Warn if scheduling work on a component before it is committed
sebmarkbage May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 69 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Open in desktop
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
Open in desktop
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;
sebmarkbage marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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