Skip to content

Commit 606f76b

Browse files
authored
Fix hydration bug with nested suspense boundaries (#16288)
This happens in this case: `<!--$!--><!--$!-->...<!--/$--><!--/$-->...` getNextHydratableInstanceAfterSuspenseInstance didn't take SUSPENSE_FALLBACK_START_DATA or SUSPENSE_PENDING_START_DATA into account so if a boundary was in one of those states, it wouldn't be considered to push the stack of boundaries. As a result the first end comment was considered the end but it really should've been the second end comment. The next comment then would've been considered something that can be skipped. However, since the logic in there considered all comments as "hydratable", it was considered a hydratable node. Since it was considered a node that didn't actually correspond to anything in the React tree it got deleted. The HTML is now `<!--$!--><!--$!-->...<!--/$-->...` and the trailing content is now hydrated since it did match something. Next, since this was client rendered, we're going to delete the suspended boundary by calling clearSuspenseBoundary and then inserting new content. However, clearSuspenseBoundary *is* aware of SUSPENSE_FALLBACK_START_DATA and assumes that there must be another `<!--/$-->` after the first one. As a result it deleted the trailing content from the DOM since it should be part of the boundary. However, those DOM nodes were already hydrated in the React tree. So we end up in an inconsistent state. When we then try to insert the new content we throw as a result. I think we would have recovered correctly if clearSuspenseBoundary and getNextHydratableInstanceAfterSuspenseInstance had the *same* bug but because they were inconsistent we ended up in a bad place.
1 parent a1dbb85 commit 606f76b

File tree

2 files changed

+67
-10
lines changed

2 files changed

+67
-10
lines changed

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,4 +1003,58 @@ describe('ReactDOMServerPartialHydration', () => {
10031003
let div = container.getElementsByTagName('div')[0];
10041004
expect(ref.current).toBe(div);
10051005
});
1006+
1007+
it('can client render nested boundaries', async () => {
1008+
let suspend = false;
1009+
let promise = new Promise(() => {});
1010+
let ref = React.createRef();
1011+
1012+
function Child() {
1013+
if (suspend) {
1014+
throw promise;
1015+
} else {
1016+
return 'Hello';
1017+
}
1018+
}
1019+
1020+
function App() {
1021+
return (
1022+
<div>
1023+
<Suspense
1024+
fallback={
1025+
<React.Fragment>
1026+
<Suspense fallback="Loading...">
1027+
<Child />
1028+
</Suspense>
1029+
<span>Inner Sibling</span>
1030+
</React.Fragment>
1031+
}>
1032+
<Child />
1033+
</Suspense>
1034+
<span ref={ref}>Sibling</span>
1035+
</div>
1036+
);
1037+
}
1038+
1039+
suspend = true;
1040+
let html = ReactDOMServer.renderToString(<App />);
1041+
1042+
let container = document.createElement('div');
1043+
container.innerHTML = html + '<!--unrelated comment-->';
1044+
1045+
let span = container.getElementsByTagName('span')[1];
1046+
1047+
suspend = false;
1048+
let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
1049+
root.render(<App />);
1050+
Scheduler.unstable_flushAll();
1051+
jest.runAllTimers();
1052+
1053+
expect(ref.current).toBe(span);
1054+
expect(span.parentNode).not.toBe(null);
1055+
1056+
// It leaves non-React comments alone.
1057+
expect(container.lastChild.nodeType).toBe(8);
1058+
expect(container.lastChild.data).toBe('unrelated comment');
1059+
});
10061060
});

packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -610,15 +610,14 @@ function getNextHydratable(node) {
610610
}
611611
if (enableSuspenseServerRenderer) {
612612
if (nodeType === COMMENT_NODE) {
613-
break;
614-
}
615-
const nodeData = (node: any).data;
616-
if (
617-
nodeData === SUSPENSE_START_DATA ||
618-
nodeData === SUSPENSE_FALLBACK_START_DATA ||
619-
nodeData === SUSPENSE_PENDING_START_DATA
620-
) {
621-
break;
613+
const nodeData = (node: any).data;
614+
if (
615+
nodeData === SUSPENSE_START_DATA ||
616+
nodeData === SUSPENSE_FALLBACK_START_DATA ||
617+
nodeData === SUSPENSE_PENDING_START_DATA
618+
) {
619+
break;
620+
}
622621
}
623622
}
624623
}
@@ -691,7 +690,11 @@ export function getNextHydratableInstanceAfterSuspenseInstance(
691690
} else {
692691
depth--;
693692
}
694-
} else if (data === SUSPENSE_START_DATA) {
693+
} else if (
694+
data === SUSPENSE_START_DATA ||
695+
data === SUSPENSE_FALLBACK_START_DATA ||
696+
data === SUSPENSE_PENDING_START_DATA
697+
) {
695698
depth++;
696699
}
697700
}

0 commit comments

Comments
 (0)