Skip to content

Commit d900fad

Browse files
authored
Bugfix: Selective hydration triggers false update loop error (facebook#27439)
This adds a regression test and fix for a case where a sync update triggers selective hydration, which then leads to a "Maximum update depth exceeded" error, even though there was only a single update. This happens when a single sync update flows into many sibling dehydrated Suspense boundaries. This fix is, if a commit was the result of selective hydration, we should not increment the nested update count, because those renders conceptually are not updates. Ideally, they wouldn't even be in a separate commit — we should be able to hydrate a tree and apply an update on top of it within the same render phase. We could do this once we implement resumable context stacks.
1 parent 13d0225 commit d900fad

File tree

3 files changed

+75
-2
lines changed

3 files changed

+75
-2
lines changed

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,4 +1853,58 @@ describe('ReactDOMServerSelectiveHydration', () => {
18531853
assertLog(['App', 'A', 'App', 'AA', 'DefaultContext', 'Commit']);
18541854
});
18551855
});
1856+
1857+
it('regression: selective hydration does not contribute to "maximum update limit" count', async () => {
1858+
const outsideRef = React.createRef(null);
1859+
const insideRef = React.createRef(null);
1860+
function Child() {
1861+
return (
1862+
<Suspense fallback="Loading...">
1863+
<div ref={insideRef} />
1864+
</Suspense>
1865+
);
1866+
}
1867+
1868+
let setIsMounted = false;
1869+
function App() {
1870+
const [isMounted, setState] = React.useState(false);
1871+
setIsMounted = setState;
1872+
1873+
const children = [];
1874+
for (let i = 0; i < 100; i++) {
1875+
children.push(<Child key={i} isMounted={isMounted} />);
1876+
}
1877+
1878+
return <div ref={outsideRef}>{children}</div>;
1879+
}
1880+
1881+
const finalHTML = ReactDOMServer.renderToString(<App />);
1882+
const container = document.createElement('div');
1883+
container.innerHTML = finalHTML;
1884+
1885+
await act(async () => {
1886+
ReactDOMClient.hydrateRoot(container, <App />);
1887+
1888+
// Commit just the shell
1889+
await waitForPaint([]);
1890+
1891+
// Assert that the shell has hydrated, but not the children
1892+
expect(outsideRef.current).not.toBe(null);
1893+
expect(insideRef.current).toBe(null);
1894+
1895+
// Update the shell synchronously. The update will flow into the children,
1896+
// which haven't hydrated yet. This will trigger a cascade of commits
1897+
// caused by selective hydration. However, since there's really only one
1898+
// update, it should not be treated as an update loop.
1899+
// NOTE: It's unfortunate that every sibling boundary is separately
1900+
// committed in this case. We should be able to commit everything in a
1901+
// render phase, which we could do if we had resumable context stacks.
1902+
ReactDOM.flushSync(() => {
1903+
setIsMounted(true);
1904+
});
1905+
});
1906+
1907+
// Should have successfully hydrated with no errors.
1908+
expect(insideRef.current).not.toBe(null);
1909+
});
18561910
});

packages/react-reconciler/src/ReactFiberLane.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ export const InputContinuousLane: Lane = /* */ 0b0000000000000000000
4646
export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000010000;
4747
export const DefaultLane: Lane = /* */ 0b0000000000000000000000000100000;
4848

49-
export const SyncUpdateLanes: Lane = /* */ 0b0000000000000000000000000101010;
49+
export const SyncUpdateLanes: Lane = enableUnifiedSyncLane
50+
? SyncLane | InputContinuousLane | DefaultLane
51+
: SyncLane;
5052

5153
const TransitionHydrationLane: Lane = /* */ 0b0000000000000000000000001000000;
5254
const TransitionLanes: Lanes = /* */ 0b0000000011111111111111110000000;
@@ -84,6 +86,11 @@ export const IdleLane: Lane = /* */ 0b0100000000000000000
8486

8587
export const OffscreenLane: Lane = /* */ 0b1000000000000000000000000000000;
8688

89+
// Any lane that might schedule an update. This is used to detect infinite
90+
// update loops, so it doesn't include hydration lanes or retries.
91+
export const UpdateLanes: Lanes =
92+
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes;
93+
8794
// This function is used for the experimental timeline (react-devtools-timeline)
8895
// It should be kept in sync with the Lanes values above.
8996
export function getLabelForLane(lane: Lane): string | void {

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ import {
154154
includesOnlyNonUrgentLanes,
155155
includesSomeLane,
156156
OffscreenLane,
157+
SyncUpdateLanes,
158+
UpdateLanes,
157159
} from './ReactFiberLane';
158160
import {
159161
DiscreteEventPriority,
@@ -2932,7 +2934,17 @@ function commitRootImpl(
29322934

29332935
// Read this again, since a passive effect might have updated it
29342936
remainingLanes = root.pendingLanes;
2935-
if (includesSyncLane(remainingLanes)) {
2937+
2938+
// Check if this render scheduled a cascading synchronous update. This is a
2939+
// heurstic to detect infinite update loops. We are intentionally excluding
2940+
// hydration lanes in this check, because render triggered by selective
2941+
// hydration is conceptually not an update.
2942+
if (
2943+
// Was the finished render the result of an update (not hydration)?
2944+
includesSomeLane(lanes, UpdateLanes) &&
2945+
// Did it schedule a sync update?
2946+
includesSomeLane(remainingLanes, SyncUpdateLanes)
2947+
) {
29362948
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
29372949
markNestedUpdateScheduled();
29382950
}

0 commit comments

Comments
 (0)