Skip to content

Commit

Permalink
Client render Suspense content if there's no boundary match (#16945)
Browse files Browse the repository at this point in the history
Without the enableSuspenseServerRenderer flag there will never be a boundary match. Also when it is enabled, there might not be a boundary match if something was conditionally rendered by mistake.

With this PR it will now client render the content of a Suspense boundary in that case and issue a DEV only hydration warning. This is the only sound semantics for this case.

Unfortunately, landing this will once again break #16938. It will be less bad though because at least it'll just work by client rendering the content instead of hydrating and issue a DEV only warning.

However, we must land this before enabling the enableSuspenseServerRenderer flag since it does this anyway.

I did notice that we special case fallback={undefined} due to our unfortunate semantics for that. So technically a workaround that works is actually setting the fallback to undefined on the server and during hydration. Then flip it on only after hydration. That could be a workaround if you want to be able to have a Suspense boundary work only after hydration for some reason.

It's kind of unfortunate but at least those semantics are internally consistent. So I added a test for that.
  • Loading branch information
sebmarkbage authored and lunaruan committed Oct 16, 2019
1 parent 9169375 commit ed5f010
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -517,14 +517,45 @@ describe('ReactDOMServerHydration', () => {
});
}

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>.',
{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

0 comments on commit ed5f010

Please sign in to comment.