Skip to content

Commit

Permalink
Clear extra nodes if there's a hydration mismatch within a suspense b…
Browse files Browse the repository at this point in the history
…oundary (#22592)

* Clear extra nodes if there's a mismatch within a suspense boundary

This usually happens when we exit out a DOM node but a suspense boundary
is a virtual DOM node and we didn't do it in that case because we took a
short cut by calling resetHydrationState directly since we know we won't
need to pop.

* Tighten up the types of getFirstHydratableChild

We currently call getFirstHydratableChild to step into the children of
a suspense boundary. This can be a text node or a suspense boundary
which isn't compatible with getFirstHydratableChild, and we cheat the type.

This accidentally works because .firstChild always returns null on those
nodes in the DOM.

This just makes that explicit.
  • Loading branch information
sebmarkbage authored Oct 19, 2021
1 parent fe0356c commit 34e4c97
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,64 @@ describe('ReactDOMServerPartialHydration', () => {
expect(deleted.length).toBe(1);
});

it('hydrates an empty suspense boundary', async () => {
function App() {
return (
<div>
<Suspense fallback="Loading..." />
<div>Sibling</div>
</div>
);
}

const finalHTML = ReactDOMServer.renderToString(<App />);

const container = document.createElement('div');
container.innerHTML = finalHTML;

ReactDOM.hydrateRoot(container, <App />);
Scheduler.unstable_flushAll();
jest.runAllTimers();

expect(container.innerHTML).toContain('<div>Sibling</div>');
});

it('recovers when server rendered additional nodes', async () => {
const ref = React.createRef();
function App({hasB}) {
return (
<div>
<Suspense fallback="Loading...">
<span ref={ref}>A</span>
{hasB ? <span>B</span> : null}
</Suspense>
<div>Sibling</div>
</div>
);
}

const finalHTML = ReactDOMServer.renderToString(<App hasB={true} />);

const container = document.createElement('div');
container.innerHTML = finalHTML;

const span = container.getElementsByTagName('span')[0];

expect(container.innerHTML).toContain('<span>A</span>');
expect(container.innerHTML).toContain('<span>B</span>');
expect(ref.current).toBe(null);

ReactDOM.hydrateRoot(container, <App hasB={false} />);
expect(() => {
Scheduler.unstable_flushAll();
}).toErrorDev('Did not expect server HTML to contain a <span> in <div>');
jest.runAllTimers();

expect(container.innerHTML).toContain('<span>A</span>');
expect(container.innerHTML).not.toContain('<span>B</span>');
expect(ref.current).toBe(span);
});

it('calls the onDeleted hydration callback if the parent gets deleted', async () => {
let suspend = false;
const promise = new Promise(() => {});
Expand Down
3 changes: 3 additions & 0 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,9 @@ function getNextHydratable(node) {
) {
break;
}
if (nodeData === SUSPENSE_END_DATA) {
return null;
}
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1007,16 +1007,16 @@ function completeWork(

if (enableSuspenseServerRenderer) {
if (nextState !== null && nextState.dehydrated !== null) {
// We might be inside a hydration state the first time we're picking up this
// Suspense boundary, and also after we've reentered it for further hydration.
const wasHydrated = popHydrationState(workInProgress);
if (current === null) {
const wasHydrated = popHydrationState(workInProgress);

if (!wasHydrated) {
throw new Error(
'A dehydrated suspense component was completed without a hydrated node. ' +
'This is probably a bug in React.',
);
}

prepareToHydrateHostSuspenseInstance(workInProgress);
bubbleProperties(workInProgress);
if (enableProfilerTimer) {
Expand All @@ -1034,9 +1034,8 @@ function completeWork(
}
return null;
} else {
// We should never have been in a hydration state if we didn't have a current.
// However, in some of those paths, we might have reentered a hydration state
// and then we might be inside a hydration state. In that case, we'll need to exit out of it.
// We might have reentered this boundary to hydrate it. If so, we need to reset the hydration
// state since we're now exiting out of it. popHydrationState doesn't do that for us.
resetHydrationState();
if ((workInProgress.flags & DidCapture) === NoFlags) {
// This boundary did not suspend so it's now hydrated and unsuspended.
Expand Down
11 changes: 5 additions & 6 deletions packages/react-reconciler/src/ReactFiberCompleteWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -1007,16 +1007,16 @@ function completeWork(

if (enableSuspenseServerRenderer) {
if (nextState !== null && nextState.dehydrated !== null) {
// We might be inside a hydration state the first time we're picking up this
// Suspense boundary, and also after we've reentered it for further hydration.
const wasHydrated = popHydrationState(workInProgress);
if (current === null) {
const wasHydrated = popHydrationState(workInProgress);

if (!wasHydrated) {
throw new Error(
'A dehydrated suspense component was completed without a hydrated node. ' +
'This is probably a bug in React.',
);
}

prepareToHydrateHostSuspenseInstance(workInProgress);
bubbleProperties(workInProgress);
if (enableProfilerTimer) {
Expand All @@ -1034,9 +1034,8 @@ function completeWork(
}
return null;
} else {
// We should never have been in a hydration state if we didn't have a current.
// However, in some of those paths, we might have reentered a hydration state
// and then we might be inside a hydration state. In that case, we'll need to exit out of it.
// We might have reentered this boundary to hydrate it. If so, we need to reset the hydration
// state since we're now exiting out of it. popHydrationState doesn't do that for us.
resetHydrationState();
if ((workInProgress.flags & DidCapture) === NoFlags) {
// This boundary did not suspend so it's now hydrated and unsuspended.
Expand Down
17 changes: 11 additions & 6 deletions packages/react-reconciler/src/ReactFiberHydrationContext.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ function tryHydrate(fiber, nextInstance) {
const instance = canHydrateInstance(nextInstance, type, props);
if (instance !== null) {
fiber.stateNode = (instance: Instance);
hydrationParentFiber = fiber;
nextHydratableInstance = getFirstHydratableChild(instance);
return true;
}
return false;
Expand All @@ -270,6 +272,9 @@ function tryHydrate(fiber, nextInstance) {
const textInstance = canHydrateTextInstance(nextInstance, text);
if (textInstance !== null) {
fiber.stateNode = (textInstance: TextInstance);
hydrationParentFiber = fiber;
// Text Instances don't have children so there's nothing to hydrate.
nextHydratableInstance = null;
return true;
}
return false;
Expand All @@ -294,6 +299,10 @@ function tryHydrate(fiber, nextInstance) {
);
dehydratedFragment.return = fiber;
fiber.child = dehydratedFragment;
hydrationParentFiber = fiber;
// While a Suspense Instance does have children, we won't step into
// it during the first pass. Instead, we'll reenter it later.
nextHydratableInstance = null;
return true;
}
}
Expand Down Expand Up @@ -322,6 +331,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
// We use this as a heuristic. It's based on intuition and not data so it
// might be flawed or unnecessary.
nextInstance = getNextHydratableSibling(firstAttemptedInstance);
const prevHydrationParentFiber: Fiber = (hydrationParentFiber: any);
if (!nextInstance || !tryHydrate(fiber, nextInstance)) {
// Nothing to hydrate. Make it an insertion.
insertNonHydratedInstance((hydrationParentFiber: any), fiber);
Expand All @@ -333,13 +343,8 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
// superfluous and we'll delete it. Since we can't eagerly delete it
// we'll have to schedule a deletion. To do that, this node needs a dummy
// fiber associated with it.
deleteHydratableInstance(
(hydrationParentFiber: any),
firstAttemptedInstance,
);
deleteHydratableInstance(prevHydrationParentFiber, firstAttemptedInstance);
}
hydrationParentFiber = fiber;
nextHydratableInstance = getFirstHydratableChild((nextInstance: any));
}

function prepareToHydrateHostInstance(
Expand Down
17 changes: 11 additions & 6 deletions packages/react-reconciler/src/ReactFiberHydrationContext.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ function tryHydrate(fiber, nextInstance) {
const instance = canHydrateInstance(nextInstance, type, props);
if (instance !== null) {
fiber.stateNode = (instance: Instance);
hydrationParentFiber = fiber;
nextHydratableInstance = getFirstHydratableChild(instance);
return true;
}
return false;
Expand All @@ -270,6 +272,9 @@ function tryHydrate(fiber, nextInstance) {
const textInstance = canHydrateTextInstance(nextInstance, text);
if (textInstance !== null) {
fiber.stateNode = (textInstance: TextInstance);
hydrationParentFiber = fiber;
// Text Instances don't have children so there's nothing to hydrate.
nextHydratableInstance = null;
return true;
}
return false;
Expand All @@ -294,6 +299,10 @@ function tryHydrate(fiber, nextInstance) {
);
dehydratedFragment.return = fiber;
fiber.child = dehydratedFragment;
hydrationParentFiber = fiber;
// While a Suspense Instance does have children, we won't step into
// it during the first pass. Instead, we'll reenter it later.
nextHydratableInstance = null;
return true;
}
}
Expand Down Expand Up @@ -322,6 +331,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
// We use this as a heuristic. It's based on intuition and not data so it
// might be flawed or unnecessary.
nextInstance = getNextHydratableSibling(firstAttemptedInstance);
const prevHydrationParentFiber: Fiber = (hydrationParentFiber: any);
if (!nextInstance || !tryHydrate(fiber, nextInstance)) {
// Nothing to hydrate. Make it an insertion.
insertNonHydratedInstance((hydrationParentFiber: any), fiber);
Expand All @@ -333,13 +343,8 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
// superfluous and we'll delete it. Since we can't eagerly delete it
// we'll have to schedule a deletion. To do that, this node needs a dummy
// fiber associated with it.
deleteHydratableInstance(
(hydrationParentFiber: any),
firstAttemptedInstance,
);
deleteHydratableInstance(prevHydrationParentFiber, firstAttemptedInstance);
}
hydrationParentFiber = fiber;
nextHydratableInstance = getFirstHydratableChild((nextInstance: any));
}

function prepareToHydrateHostInstance(
Expand Down

0 comments on commit 34e4c97

Please sign in to comment.