Skip to content

Commit

Permalink
Revert #24236 (Don't recreate the same fallback on the client if hydr…
Browse files Browse the repository at this point in the history
…ating suspends) (#24434)

* Revert #24236 (Don't recreate the same fallback on the client if hydrating suspends)

* Use @GATE FIXME
  • Loading branch information
gaearon authored Apr 25, 2022
1 parent 6d3b6d0 commit bd4784c
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 28 deletions.
38 changes: 23 additions & 15 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -869,16 +869,15 @@ describe('ReactDOMFizzServer', () => {
});

// We still can't render it on the client.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// We now resolve it on the client.
resolveText('Hello');

expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to an ' +
'error during server rendering. Switched to client rendering.',
]);
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// We now resolve it on the client.
resolveText('Hello');
Scheduler.unstable_flushAll();

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(
Expand Down Expand Up @@ -2189,7 +2188,10 @@ describe('ReactDOMFizzServer', () => {
},
);

// Disabled because of a WWW late mutations regression.
// We may want to re-enable this if we figure out why.
// @gate experimental
// @gate FIXME
it('does not recreate the fallback if server errors and hydration suspends', async () => {
let isClient = false;

Expand Down Expand Up @@ -2268,7 +2270,10 @@ describe('ReactDOMFizzServer', () => {
);
});

// Disabled because of a WWW late mutations regression.
// We may want to re-enable this if we figure out why.
// @gate experimental
// @gate FIXME
it(
'does not recreate the fallback if server errors and hydration suspends ' +
'and root receives a transition',
Expand Down Expand Up @@ -2364,7 +2369,10 @@ describe('ReactDOMFizzServer', () => {
},
);

// Disabled because of a WWW late mutations regression.
// We may want to re-enable this if we figure out why.
// @gate experimental
// @gate FIXME
it(
'recreates the fallback if server errors and hydration suspends but ' +
'client receives new props',
Expand Down Expand Up @@ -2542,25 +2550,25 @@ describe('ReactDOMFizzServer', () => {
},
});

// An error happened but instead of surfacing it to the UI, we suspended.
expect(Scheduler).toFlushAndYield([]);
// An error logged but instead of surfacing it to the UI, we switched
// to client rendering.
expect(Scheduler).toFlushAndYield([
'Hydration error',
'There was an error while hydrating this Suspense boundary. Switched ' +
'to client rendering.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
<span>Yay!</span>
Loading...
<span />
</div>,
);

await act(async () => {
resolveText('Yay!');
});
expect(Scheduler).toFlushAndYield([
'Yay!',
'Hydration error',
'There was an error while hydrating this Suspense boundary. Switched ' +
'to client rendering.',
]);
expect(Scheduler).toFlushAndYield(['Yay!']);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
Expand Down
18 changes: 12 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ describe('ReactDOMServerHydration', () => {
});

// @gate __DEV__
it('does not warn when client renders an extra node inside Suspense fallback', () => {
it('warns when client renders an extra node inside Suspense fallback', () => {
function Mismatch({isClient}) {
return (
<div className="parent">
Expand All @@ -809,12 +809,15 @@ describe('ReactDOMServerHydration', () => {
</div>
);
}
// There is no error because we don't actually hydrate fallbacks.
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`);
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`
Array [
"Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]",
]
`);
});

// @gate __DEV__
it('does not warn when server renders an extra node inside Suspense fallback', () => {
it('warns when server renders an extra node inside Suspense fallback', () => {
function Mismatch({isClient}) {
return (
<div className="parent">
Expand All @@ -831,8 +834,11 @@ describe('ReactDOMServerHydration', () => {
</div>
);
}
// There is no error because we don't actually hydrate fallbacks.
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`);
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`
Array [
"Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]",
]
`);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2115,7 +2115,10 @@ describe('ReactDOMServerPartialHydration', () => {
});

suspend = true;
expect(Scheduler).toFlushAndYield([]);
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to ' +
'an error during server rendering. Switched to client rendering.',
]);

// We haven't hydrated the second child but the placeholder is still in the list.
expect(container.textContent).toBe('ALoading B');
Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2270,7 +2270,6 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
} else {
// Suspended but we should no longer be in dehydrated mode.
// Therefore we now have to render the fallback.
renderDidSuspendDelayIfPossible();
const nextPrimaryChildren = nextProps.children;
const nextFallbackChildren = nextProps.fallback;
const fallbackChildFragment = mountSuspenseFallbackAfterRetryWithoutHydrating(
Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2270,7 +2270,6 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
} else {
// Suspended but we should no longer be in dehydrated mode.
// Therefore we now have to render the fallback.
renderDidSuspendDelayIfPossible();
const nextPrimaryChildren = nextProps.children;
const nextFallbackChildren = nextProps.fallback;
const fallbackChildFragment = mountSuspenseFallbackAfterRetryWithoutHydrating(
Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberLane.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,9 @@ export function includesOnlyNonUrgentLanes(lanes: Lanes) {
const UrgentLanes = SyncLane | InputContinuousLane | DefaultLane;
return (lanes & UrgentLanes) === NoLanes;
}
export function includesOnlyTransitions(lanes: Lanes) {
return (lanes & TransitionLanes) === lanes;
}

export function includesBlockingLane(root: FiberRoot, lanes: Lanes) {
if (
Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberLane.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,9 @@ export function includesOnlyNonUrgentLanes(lanes: Lanes) {
const UrgentLanes = SyncLane | InputContinuousLane | DefaultLane;
return (lanes & UrgentLanes) === NoLanes;
}
export function includesOnlyTransitions(lanes: Lanes) {
return (lanes & TransitionLanes) === lanes;
}

export function includesBlockingLane(root: FiberRoot, lanes: Lanes) {
if (
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ import {
pickArbitraryLane,
includesNonIdleWork,
includesOnlyRetries,
includesOnlyNonUrgentLanes,
includesOnlyTransitions,
includesBlockingLane,
includesExpiredLane,
getNextLanes,
Expand Down Expand Up @@ -1150,7 +1150,7 @@ function finishConcurrentRender(root, exitStatus, lanes) {
case RootSuspendedWithDelay: {
markRootSuspended(root, lanes);

if (includesOnlyNonUrgentLanes(lanes)) {
if (includesOnlyTransitions(lanes)) {
// This is a transition, so we should exit without committing a
// placeholder and without scheduling a timeout. Delay indefinitely
// until we receive more data.
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ import {
pickArbitraryLane,
includesNonIdleWork,
includesOnlyRetries,
includesOnlyNonUrgentLanes,
includesOnlyTransitions,
includesBlockingLane,
includesExpiredLane,
getNextLanes,
Expand Down Expand Up @@ -1150,7 +1150,7 @@ function finishConcurrentRender(root, exitStatus, lanes) {
case RootSuspendedWithDelay: {
markRootSuspended(root, lanes);

if (includesOnlyNonUrgentLanes(lanes)) {
if (includesOnlyTransitions(lanes)) {
// This is a transition, so we should exit without committing a
// placeholder and without scheduling a timeout. Delay indefinitely
// until we receive more data.
Expand Down

0 comments on commit bd4784c

Please sign in to comment.