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.

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

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

Merged
merged 2 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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