Skip to content

Commit e1dd0a2

Browse files
authored
Remove recoverable error when a sync update flows into a dehydrated boundary (#25692)
This just removes the error but the underlying issue is still there, and it's likely that the best course of action is to not update in effects and to wrap most updates in startTransition. However, that's more of a performance concern which is not something we generally do even in recoverable errors since they're less actionable and likely belong in another channel. It is also likely that in many cases this happens so rarely because you have to interact quickly enough that it can often be ignored. After changes to other parts of the model, this only happens for sync/discrete updates. There are three scenarios that can happen: - We replace a server rendered fallback with a client rendered fallback. Other than this potentially causing some flickering in the loading state, it's not a big deal. - We replace the server rendered content with a client side fallback if this suspends on the client. This is in line with what would happen anyway. We will loose state of forms which is not intended semantics. State and animations etc would've been lost anyway if it was client-side so that's not a concern. - We replace the server rendered content with a client side rendered tree and lose selection/state and form state. While the content looks the same, which is unfortunate. In most scenarios it's a bad loading state but it's the same scenario as flushing sync client-side. So it's not so bad. The big change here is that we consider this a bug of React that we should fix. Therefore it's not actionable to users today because it should just get fixed. So we're removing the error early. Although anyone that has fixed these issues already are probably better off for it. To fix this while still hydrating we need to be able to rewind a sync tree and then replay it. @tyao1 is going to add a Sync hydration lane. This is will allow us to rewind the tree when we hit this state, and replay it given the previous Context, hydrate and then reapply the update. The reason we didn't do this originally is because it causes sync mode to unwind where as for backwards compatibility we didn't want to cause that breaking semantic - outside Suspense boundaries - and we don't want that semantic longer term. We're only do this as a short term fix. We should also have a way to leave a partial tree in place. If the sync hydration lane suspends, we should be able to switch to a client side fallback without throwing away the state of the DOM and then hydrate later. We now know how we want to fix this longer term. We're going to move all Contexts into resumable trees like what Fizz/Flight does. That way we can leave the original Context at the hydration boundaries and then resume from there. That way the rewinding would never happen even in the existence of a sync hydration lane which would only apply locally to the dehydrated tree. So the steps are 1) remove the error 2) add the sync hydration lane with rewinding 3) Allow hiding server-rendered content while still not hydrated 4) add resumable contexts at these boundaries. Fixes #25625 and #24959.
1 parent c54e354 commit e1dd0a2

File tree

3 files changed

+18
-70
lines changed

3 files changed

+18
-70
lines changed

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -453,12 +453,6 @@ describe('ReactDOMServerPartialHydration', () => {
453453

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

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

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

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

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

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

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

23702334
// This will now be a new span because we weren't able to hydrate before
23712335
const newSpan = container.getElementsByTagName('span')[0];

packages/react-reconciler/src/ReactFiberBeginWork.new.js

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2717,9 +2717,6 @@ function updateDehydratedSuspenseComponent(
27172717
current,
27182718
workInProgress,
27192719
renderLanes,
2720-
// TODO: When we delete legacy mode, we should make this error argument
2721-
// required — every concurrent mode path that causes hydration to
2722-
// de-opt to client rendering should have an error message.
27232720
null,
27242721
);
27252722
}
@@ -2809,25 +2806,20 @@ function updateDehydratedSuspenseComponent(
28092806
}
28102807
}
28112808

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

packages/react-reconciler/src/ReactFiberBeginWork.old.js

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2717,9 +2717,6 @@ function updateDehydratedSuspenseComponent(
27172717
current,
27182718
workInProgress,
27192719
renderLanes,
2720-
// TODO: When we delete legacy mode, we should make this error argument
2721-
// required — every concurrent mode path that causes hydration to
2722-
// de-opt to client rendering should have an error message.
27232720
null,
27242721
);
27252722
}
@@ -2809,25 +2806,20 @@ function updateDehydratedSuspenseComponent(
28092806
}
28102807
}
28112808

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

0 commit comments

Comments
 (0)