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

Client render Suspense content if there's no boundary match #16945

Merged
merged 1 commit into from
Oct 16, 2019
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 @@ -660,14 +660,45 @@ describe('ReactDOMServerHydration', () => {
document.body.removeChild(parentContainer);
});

it('regression test: Suspense + hydration in legacy mode ', () => {
it('Suspense + hydration in legacy mode', () => {
const element = document.createElement('div');
element.innerHTML = '<div>Hello World</div>';
let div = element.firstChild;
let ref = React.createRef();
expect(() =>
ReactDOM.hydrate(
<React.Suspense fallback={null}>
<div ref={ref}>Hello World</div>
</React.Suspense>,
element,
),
).toWarnDev(
'Warning: Did not expect server HTML to contain a <div> in <div>.',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warns because of the superfluous div but before that it should warn about missing a Suspense boundary but it's currently missing that warning:

https://github.com/facebook/react/blob/master/packages/react-dom/src/client/ReactDOMHostConfig.js#L843

{withoutStack: true},
);

// The content should've been client rendered and replaced the
// existing div.
expect(ref.current).not.toBe(div);
// The HTML should be the same though.
expect(element.innerHTML).toBe('<div>Hello World</div>');
});

it('Suspense + hydration in legacy mode with no fallback', () => {
const element = document.createElement('div');
element.innerHTML = '<div>Hello World</div>';
let div = element.firstChild;
let ref = React.createRef();
ReactDOM.hydrate(
<React.Suspense>
<div>Hello World</div>
<div ref={ref}>Hello World</div>
</React.Suspense>,
element,
);

// Because this didn't have a fallback, it was hydrated as if it's
// not a Suspense boundary.
expect(ref.current).toBe(div);
expect(element.innerHTML).toBe('<div>Hello World</div>');
});
});
12 changes: 6 additions & 6 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1592,12 +1592,12 @@ function updateSuspenseComponent(
// children. It's essentially a very basic form of re-parenting.

if (current === null) {
if (enableSuspenseServerRenderer) {
// If we're currently hydrating, try to hydrate this boundary.
// But only if this has a fallback.
if (nextProps.fallback !== undefined) {
tryToClaimNextHydratableInstance(workInProgress);
// This could've been a dehydrated suspense component.
// If we're currently hydrating, try to hydrate this boundary.
// But only if this has a fallback.
if (nextProps.fallback !== undefined) {
tryToClaimNextHydratableInstance(workInProgress);
// This could've been a dehydrated suspense component.
if (enableSuspenseServerRenderer) {
const suspenseState: null | SuspenseState =
workInProgress.memoizedState;
if (suspenseState !== null) {
Expand Down
7 changes: 3 additions & 4 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -870,10 +870,9 @@ function completeWork(
const nextDidTimeout = nextState !== null;
let prevDidTimeout = false;
if (current === null) {
// In cases where we didn't find a suitable hydration boundary we never
// put this in dehydrated mode, but we still need to pop the hydration
// state since we might be inside the insertion tree.
popHydrationState(workInProgress);
if (workInProgress.memoizedProps.fallback !== undefined) {
popHydrationState(workInProgress);
}
} else {
const prevState: null | SuspenseState = current.memoizedState;
prevDidTimeout = prevState !== null;
Expand Down
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/ReactFiberHydrationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,11 @@ function skipPastDehydratedSuspenseInstance(
let suspenseState: null | SuspenseState = fiber.memoizedState;
let suspenseInstance: null | SuspenseInstance =
suspenseState !== null ? suspenseState.dehydrated : null;
if (suspenseInstance === null) {
// This Suspense boundary was hydrated without a match.
return nextHydratableInstance;
}
invariant(
suspenseInstance,
'Expected to have a hydrated suspense instance. ' +
'This error is likely caused by a bug in React. Please file an issue.',
);
return getNextHydratableInstanceAfterSuspenseInstance(suspenseInstance);
}

Expand Down