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

Bugfix: Selective hydration triggers false update loop error #27439

Merged
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 @@ -1853,4 +1853,58 @@ describe('ReactDOMServerSelectiveHydration', () => {
assertLog(['App', 'A', 'App', 'AA', 'DefaultContext', 'Commit']);
});
});

it('regression: selective hydration does not contribute to "maximum update limit" count', async () => {
const outsideRef = React.createRef(null);
const insideRef = React.createRef(null);
function Child() {
return (
<Suspense fallback="Loading...">
<div ref={insideRef} />
</Suspense>
);
}

let setIsMounted = false;
function App() {
const [isMounted, setState] = React.useState(false);
setIsMounted = setState;

const children = [];
for (let i = 0; i < 100; i++) {
children.push(<Child key={i} isMounted={isMounted} />);
}

return <div ref={outsideRef}>{children}</div>;
}

const finalHTML = ReactDOMServer.renderToString(<App />);
const container = document.createElement('div');
container.innerHTML = finalHTML;

await act(async () => {
ReactDOMClient.hydrateRoot(container, <App />);

// Commit just the shell
await waitForPaint([]);

// Assert that the shell has hydrated, but not the children
expect(outsideRef.current).not.toBe(null);
expect(insideRef.current).toBe(null);

// Update the shell synchronously. The update will flow into the children,
// which haven't hydrated yet. This will trigger a cascade of commits
// caused by selective hydration. However, since there's really only one
// update, it should not be treated as an update loop.
// NOTE: It's unfortunate that every sibling boundary is separately
// committed in this case. We should be able to commit everything in a
// render phase, which we could do if we had resumable context stacks.
ReactDOM.flushSync(() => {
setIsMounted(true);
});
});

// Should have successfully hydrated with no errors.
expect(insideRef.current).not.toBe(null);
});
});
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/ReactFiberLane.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ export const InputContinuousLane: Lane = /* */ 0b0000000000000000000
export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000000010000;
export const DefaultLane: Lane = /* */ 0b0000000000000000000000000100000;

export const SyncUpdateLanes: Lane = /* */ 0b0000000000000000000000000101010;
export const SyncUpdateLanes: Lane = enableUnifiedSyncLane
? SyncLane | InputContinuousLane | DefaultLane
: SyncLane;

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

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

// Any lane that might schedule an update. This is used to detect infinite
// update loops, so it doesn't include hydration lanes or retries.
export const UpdateLanes: Lanes =
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes;

// This function is used for the experimental timeline (react-devtools-timeline)
// It should be kept in sync with the Lanes values above.
export function getLabelForLane(lane: Lane): string | void {
Expand Down
14 changes: 13 additions & 1 deletion packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ import {
includesOnlyNonUrgentLanes,
includesSomeLane,
OffscreenLane,
SyncUpdateLanes,
UpdateLanes,
} from './ReactFiberLane';
import {
DiscreteEventPriority,
Expand Down Expand Up @@ -2932,7 +2934,17 @@ function commitRootImpl(

// Read this again, since a passive effect might have updated it
remainingLanes = root.pendingLanes;
if (includesSyncLane(remainingLanes)) {

// Check if this render scheduled a cascading synchronous update. This is a
// heurstic to detect infinite update loops. We are intentionally excluding
// hydration lanes in this check, because render triggered by selective
// hydration is conceptually not an update.
if (
// Was the finished render the result of an update (not hydration)?
includesSomeLane(lanes, UpdateLanes) &&
// Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes)
) {
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
markNestedUpdateScheduled();
}
Expand Down