Skip to content

Commit

Permalink
Remove recoverable error when a sync update flows into a dehydrated b…
Browse files Browse the repository at this point in the history 
…oundary
  • Loading branch information
@sebmarkbage
sebmarkbage committed Nov 16, 2022
1 parent d1e35c7 commit 2157c76
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 70 deletions.
36 changes: 0 additions & 36 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
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
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
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

0 comments on commit 2157c76

Please sign in to comment.