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

Remove recoverable error when a sync update flows into a dehydrated boundary #25692

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Remove recoverable error when a sync update flows into a dehydrated boundary #25692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 0 additions & 36 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,6 @@ describe('ReactDOMServerPartialHydration', () => {

Scheduler.unstable_flushAll();
jest.runAllTimers();
expect(Scheduler).toHaveYielded([
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
]);

expect(hydrated.length).toBe(1);
expect(deleted.length).toBe(1);
Expand Down Expand Up @@ -1102,12 +1096,6 @@ describe('ReactDOMServerPartialHydration', () => {
root.render(<App text="Hi" className="hi" />);
Scheduler.unstable_flushAll();
jest.runAllTimers();
expect(Scheduler).toHaveYielded([
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client ' +
'rendering. The usual way to fix this is to wrap the original ' +
'update in startTransition.',
]);

// Flushing now should delete the existing content and show the fallback.

Expand Down Expand Up @@ -1191,12 +1179,6 @@ describe('ReactDOMServerPartialHydration', () => {
// Flushing now should delete the existing content and show the fallback.
Scheduler.unstable_flushAll();
jest.runAllTimers();
expect(Scheduler).toHaveYielded([
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
]);

expect(container.getElementsByTagName('span').length).toBe(1);
expect(ref.current).toBe(span);
Expand Down Expand Up @@ -1284,12 +1266,6 @@ describe('ReactDOMServerPartialHydration', () => {
suspend = false;
resolve();
await promise;
expect(Scheduler).toHaveYielded([
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
]);

Scheduler.unstable_flushAll();
jest.runAllTimers();
Expand Down Expand Up @@ -1602,12 +1578,6 @@ describe('ReactDOMServerPartialHydration', () => {
// Flushing now should delete the existing content and show the fallback.
Scheduler.unstable_flushAll();
jest.runAllTimers();
expect(Scheduler).toHaveYielded([
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
]);

expect(container.getElementsByTagName('span').length).toBe(0);
expect(ref.current).toBe(null);
Expand Down Expand Up @@ -2360,12 +2330,6 @@ describe('ReactDOMServerPartialHydration', () => {
// This will force all expiration times to flush.
Scheduler.unstable_flushAll();
jest.runAllTimers();
expect(Scheduler).toHaveYielded([
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
]);

// This will now be a new span because we weren't able to hydrate before
const newSpan = container.getElementsByTagName('span')[0];
Expand Down
26 changes: 9 additions & 17 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2717,9 +2717,6 @@ function updateDehydratedSuspenseComponent(
current,
workInProgress,
renderLanes,
// TODO: When we delete legacy mode, we should make this error argument
// required — every concurrent mode path that causes hydration to
// de-opt to client rendering should have an error message.
null,
);
}
Expand Down Expand Up @@ -2809,25 +2806,20 @@ function updateDehydratedSuspenseComponent(
}
}

// If we have scheduled higher pri work above, this will probably just abort the render
// since we now have higher priority work, but in case it doesn't, we need to prepare to
// render something, if we time out. Even if that requires us to delete everything and
// skip hydration.
// Delay having to do this as long as the suspense timeout allows us.
// If we have scheduled higher pri work above, this will just abort the render
// since we now have higher priority work. We'll try to infinitely suspend until
// we yield. TODO: We could probably just force yielding earlier instead.
renderDidSuspendDelayIfPossible();
const capturedValue = createCapturedValue(
new Error(
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
),
);
// If we rendered synchronously, we won't yield so have to render something.
// This will cause us to delete any existing content.
// TODO: We should ideally have a sync hydration lane that we can apply to do
// a pass where we hydrate this subtree in place using the previous Context and then
// reapply the update afterwards.
return retrySuspenseComponentWithoutHydrating(
current,
workInProgress,
renderLanes,
capturedValue,
null,
);
} else if (isSuspenseInstancePending(suspenseInstance)) {
// This component is still pending more data from the server, so we can't hydrate its
Expand Down
26 changes: 9 additions & 17 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2717,9 +2717,6 @@ function updateDehydratedSuspenseComponent(
current,
workInProgress,
renderLanes,
// TODO: When we delete legacy mode, we should make this error argument
// required — every concurrent mode path that causes hydration to
// de-opt to client rendering should have an error message.
null,
);
}
Expand Down Expand Up @@ -2809,25 +2806,20 @@ function updateDehydratedSuspenseComponent(
}
}

// If we have scheduled higher pri work above, this will probably just abort the render
// since we now have higher priority work, but in case it doesn't, we need to prepare to
// render something, if we time out. Even if that requires us to delete everything and
// skip hydration.
// Delay having to do this as long as the suspense timeout allows us.
// If we have scheduled higher pri work above, this will just abort the render
// since we now have higher priority work. We'll try to infinitely suspend until
// we yield. TODO: We could probably just force yielding earlier instead.
renderDidSuspendDelayIfPossible();
const capturedValue = createCapturedValue(
new Error(
'This Suspense boundary received an update before it finished ' +
'hydrating. This caused the boundary to switch to client rendering. ' +
'The usual way to fix this is to wrap the original update ' +
'in startTransition.',
),
);
// If we rendered synchronously, we won't yield so have to render something.
// This will cause us to delete any existing content.
// TODO: We should ideally have a sync hydration lane that we can apply to do
// a pass where we hydrate this subtree in place using the previous Context and then
// reapply the update afterwards.
return retrySuspenseComponentWithoutHydrating(
current,
workInProgress,
renderLanes,
capturedValue,
null,
);
} else if (isSuspenseInstancePending(suspenseInstance)) {
// This component is still pending more data from the server, so we can't hydrate its
Expand Down