fix(app-router): stream ancestor loading boundaries by james-elicx · Pull Request #2331 · cloudflare/vinext

james-elicx · 2026-06-25T12:38:31Z

Summary

preserve ancestor loading.tsx modules in App Router route metadata
place ancestor Suspense boundaries at their owning segment during RSC rendering
skip eager page probing and dynamic RSC reuse when any active ancestor loading boundary exists

Next.js parity

Targets the remaining non-cache initial-HTML loading failures from Next.js v16.2.6 test/e2e/app-dir/app/index.test.ts:

should render loading.js in initial html for slow layout
should render loading.js in initial html for slow layout and page

Bootstrap script count remains a separate unowned gap and is intentionally out of scope.

Research

verified Next.js v16.2.6 stores loading UI per loader-tree segment in create-component-tree.tsx
verified @vitejs/plugin-rsc 0.5.27 streams correctly placed React Suspense fallbacks without plugin changes
checked current open PRs; no PR owns ancestor loading initial-HTML parity

Validation

vp check on 18 changed files
145 focused graph/manifest/loader/wiring tests passed
targeted ancestor loading dispatch probe/cache regression passed
all 5 tests/e2e/app-router/loading.spec.ts cases passed with one worker
the two upstream parity cases passed with one worker after final changes
vp run vinext#build passed
self-review found and fixed ancestor-loading eager page probing; final review found no further issues

james-elicx · 2026-06-25T12:38:33Z

/bigbonk review for issues

pkg-pr-new · 2026-06-25T12:39:45Z

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2331

npm i https://pkg.pr.new/vinext@2331

commit: 52d148e

github-actions · 2026-06-25T12:39:58Z

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

github-actions · 2026-06-25T12:42:10Z

Performance benchmarks

Compared 52d148e against base de014d4 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	117.1 KB	117.1 KB	⚫ -0.0%
Client entry size (gzip)	vinext	112.5 KB	112.5 KB	⚫ -0.0%
Dev server cold start	vinext	2.45 s	2.44 s	⚫ -0.4%
Production build time	vinext	2.82 s	2.84 s	⚫ +0.6%
RSC entry closure size (gzip)	vinext	93.0 KB	93.3 KB	⚫ +0.3%
Server bundle size (gzip)	vinext	156.9 KB	157.2 KB	⚫ +0.2%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

ask-bonk

Review: stream ancestor loading boundaries

I reviewed the implementation, ran the targeted unit suite (199 tests across app-route-graph, app-page-route-wiring, app-page-dispatch, app-route-module-loader, entry-templates — all green), and ran vp check on the 5 changed source files (clean). The change is well-scoped and the indexing/wiring is correct.

What's correct

Tree-position parity. discoverAncestorLoadings walks the raw segments array with path.join and assigns index + 1 positions, matching discoverSegmentErrors exactly and aligning with computeLayoutTreePositions (both count all directory levels including route groups). Ancestor positions therefore interleave correctly with layouts/errors in orderedTreePositions.
No double-counting of the leaf. Root loading is only recorded when segments.length > 0, and the loop stops at segments.length - 1, so the route's own loadingPath (handled separately at the route level) is never duplicated. Home page (/) and single-segment routes were verified by hand.
Manifest wiring mirrors the existing errorPaths pattern (placeholder moduleArray + __loadAncestorLoadings lazy loaders + parallel ancestorLoadingTreePositions), and ensureAppRouteModulesLoaded hydrates positionally. Index alignment holds because both arrays derive from the same ordered ancestorLoadingEntries.
Dispatch refactor is behavior-preserving. hasActiveLoadingBoundary already gated hasLoadingBoundary and the prior probePageSpecialError early-return; the new form route.loading?.default || route.ancestorLoadings?.some(...) is a strict superset of the old condition, and replacing the inline probePageSpecialError check with hasActiveLoadingBoundary is equivalent (the suppress check is folded into the variable). The dispatch test now proves ancestor loadings skip the eager page probe and prevent dynamic-RSC reuse when the page reads searchParams.
Suspense placement sits between the segment's layout boundaries (outer) and error boundary (inner) per the documented per-segment nesting, keyed by segmentResetKey consistent with the route-level and slot-level loading boundaries.

Minor observations (non-blocking)

Prefetch loading shell now gains ancestor Suspense wrappers. In prefetch-loading-shell mode shouldSuppressLoadingBoundaries returns false, so the segment loop wraps the leaf loading shell with ancestor loading Suspense boundaries too. The fallbacks never fire (the shell content is synchronous), so this is almost certainly a visual no-op, but it is an untested change to prefetch-shell structure that isn't called out in the PR description. A focused assertion in app-page-route-wiring.test.ts for prefetch-shell + ancestor-loading would lock the behavior in.
Slot sub-routes inherit ancestorLoadingTreePositions verbatim from the parent (discoverSlotSubRoutes), the same as layoutErrorPaths/layoutTreePositions. Consistent with existing conventions, just flagging it as the one place positions are copied rather than recomputed.

Verification done

vp test run on the 5 changed test files: 199 passed.
vp check on the 5 changed source files: format + lint + types clean.

Overall this looks correct and ready. The two minor items are optional follow-ups, not merge blockers.

ask-bonk · 2026-06-25T12:42:43Z

Review posted to PR #2331.

Summary

I reviewed PR #2331 (stream ancestor loading boundaries) and found it correct and ready to merge. The change is well-scoped.

Verification:

Ran the 5 changed test files: 199 tests passed
Ran vp check on the 5 changed source files: clean (format + lint + types)

Key findings (all correct):

Tree-position assignment in discoverAncestorLoadings matches discoverSegmentErrors and aligns with the layout/error indexing convention, so ancestor loadings interleave correctly in the wiring.
No leaf double-counting — root loading is only recorded when segments.length > 0, and the loop excludes the leaf (whose loading is handled separately).
Manifest generation, lazy hydration (ensureAppRouteModulesLoaded), and slot sub-route inheritance all mirror the existing errorPaths pattern with consistent index alignment.
The app-page-dispatch.ts refactor (hasActiveLoadingBoundary extended to include ancestors, simplified probePageSpecialError) is behavior-preserving — the new condition is a strict superset of the old one.
Suspense placement and reset keys are consistent with existing route/slot loading boundaries.

Two non-blocking observations I noted in the review:

Prefetch loading shells now gain ancestor Suspense wrappers (almost certainly a visual no-op, but an untested structural change worth a focused assertion).
Slot sub-routes inherit ancestor positions verbatim from the parent — consistent with existing conventions, just flagged.

github run

james-elicx · 2026-06-25T12:55:44Z

/bigbonk review for issues

james-elicx · 2026-06-25T13:04:16Z

/bigbonk review for issues

ask-bonk · 2026-06-25T13:06:05Z

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

james-elicx · 2026-06-25T13:06:16Z

/bigbonk review for issues

james-elicx · 2026-06-25T13:08:16Z

/bigbonk review for issues

james-elicx · 2026-06-25T13:10:33Z

/bigbonk review for issues

ask-bonk

Review: stream ancestor loading boundaries

I independently verified the implementation, ran the targeted suite (229 tests across app-route-graph, app-page-route-wiring, app-page-dispatch, app-page-probe, app-route-module-loader, entry-templates — all green), ran vp check on the 5 changed source files (clean), and ran vp run vinext#build (passed). The change is correct, well-scoped, and ready to merge.

Verified correct

Tree-position parity. discoverAncestorLoadings walks segments with path.join + findFile and assigns index + 1 positions, matching discoverSegmentErrors/discoverLayouts/discoverTemplates and aligning with computeLayoutTreePositions (depth = directory levels from appDir, counting route groups). Ancestor positions therefore interleave correctly with layouts/errors in orderedTreePositions.
No leaf double-counting. Since segments = dir.split("/"), the leaf is the last segment and its loading.tsx is captured separately as loadingPath from routeDir. The ancestor loop stops at segments.length - 1, and the root entry is gated on segments.length > 0. Verified by hand for / (no ancestors), /dashboard (root only), and /dashboard/slow (root + dashboard).
shouldProbeLayoutAt is geometrically aligned with the Suspense placement. A layout is skipped (and marked "d") only when an ancestor loading sits strictly above it (loadingTreePosition < layoutTreePosition). This exactly matches the rendering: ancestor loadings wrap their segment subtree (line 1089), while a layout at the same position as the leaf loading renders synchronously above the route-level Suspense boundary (line 962) and is correctly still probed. The ?? 0 / optional-chaining defaults fail safe to "always probe".
Page-probe gating is a strict superset. hasActiveLoadingBoundary now adds route.ancestorLoadings?.some((l) => l?.default) to the prior route.loading?.default condition, and probePageSpecialError folds the suppress check into that variable. Behavior-preserving for the no-ancestor case. The new does not reuse ancestor-loading-boundary RSC test proves ancestor loadings skip the eager page probe and prevent dynamic-RSC reuse.
Manifest / lazy-hydration / slot inheritance mirror the existing errorPaths pattern (placeholder moduleArray + __loadAncestorLoadings + parallel ancestorLoadingTreePositions), hydrate positionally in ensureAppRouteModulesLoaded, and inherit verbatim to slot sub-routes — index alignment holds because both arrays derive from the same ordered ancestorLoadingEntries.
Prefetch shell uses the rootmost boundary. prefetchLoadingComponent sorts ancestor loadings by tree position and falls back to the route loading, matching Next.js's topmost-applicable-loading shell. Covered by the new uses the rootmost loading boundary for loading-shell prefetches test.

Minor, non-blocking observations

Manifest default inconsistency (cosmetic). ancestorLoadingTreePositions defaults to [] while the sibling errorTreePositions defaults to null. Functionally identical for all consumers (?.some(...) / moduleArray(0)), just worth normalizing for consistency.
Prefetch-shell gains ancestor Suspense wrappers. Because shouldSuppressLoadingBoundaries returns false for prefetch-loading-shell, the segment loop wraps ancestor segments (and their slots) in Suspense even though elements[pageId] is null and the shell content is synchronous. A confirmed structural no-op — the fallbacks never fire — but an app-page-route-wiring.test.ts assertion for prefetch-shell + ancestor-loading would lock it in.

Verification

vp test run on the 5 changed test files: 229 passed.
vp check on the 5 changed source files: format + lint + types clean.
vp run vinext#build: passed.

No correctness issues found. The two items above are optional follow-ups, not merge blockers.

ask-bonk · 2026-06-25T13:10:48Z

Review posted to PR #2331.

Summary

I reviewed PR #2331 (stream ancestor loading boundaries) and found it correct and ready to merge.

Verification performed:

Ran the 6 changed test files: 229 tests passed
vp check on 5 changed source files: clean (format + lint + types)
vp run vinext#build: passed

Key findings (all correct):

discoverAncestorLoadings tree-position assignment matches the sibling discovery helpers and computeLayoutTreePositions — ancestor positions interleave correctly in the wiring.
No leaf double-counting: leaf loading is loadingPath (from routeDir), the ancestor loop stops at segments.length - 1, root is gated on segments.length > 0. Verified by hand for /, /dashboard, /dashboard/slow.
shouldProbeLayoutAt is geometrically aligned with Suspense placement — only skips layouts strictly below an ancestor loading boundary, exactly matching where the boundaries actually sit. The leaf's own layout (same position as leaf loading) correctly stays probed since it renders above the route-level boundary.
hasActiveLoadingBoundary extension is a strict superset of the prior condition (behavior-preserving for no-ancestor routes).
Manifest/lazy-hydration/slot inheritance mirror the existing errorPaths pattern with consistent index alignment.
Prefetch shell correctly picks the rootmost loading boundary.

Two non-blocking observations:

Cosmetic: ancestorLoadingTreePositions defaults to [] while errorTreePositions defaults to null (functionally identical for all consumers).
Prefetch-shell gains ancestor Suspense wrappers — a confirmed structural no-op worth a focused test assertion.

No correctness issues found.

github run

fix(app-router): stream ancestor loading boundaries

9104479

ask-bonk Bot reviewed Jun 25, 2026

View reviewed changes

fix(app-router): complete loading boundary parity

8705d43

fix(app-router): harden loading prefetch reuse

52d148e

ask-bonk Bot reviewed Jun 25, 2026

View reviewed changes

james-elicx marked this pull request as ready for review June 25, 2026 13:11

james-elicx mentioned this pull request Jun 25, 2026

fix(app-router): prefetch user Suspense shells #2364

Open

Uh oh!

Conversation

james-elicx commented Jun 25, 2026

Summary

Next.js parity

Research

Validation

Uh oh!

james-elicx commented Jun 25, 2026

Uh oh!

pkg-pr-new Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 25, 2026

Uh oh!

github-actions Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance benchmarks

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Review: stream ancestor loading boundaries

What's correct

Minor observations (non-blocking)

Verification done

Uh oh!

ask-bonk Bot commented Jun 25, 2026

Summary

Uh oh!

james-elicx commented Jun 25, 2026

Uh oh!

james-elicx commented Jun 25, 2026

Uh oh!

ask-bonk Bot commented Jun 25, 2026

Uh oh!

james-elicx commented Jun 25, 2026

Uh oh!

james-elicx commented Jun 25, 2026

Uh oh!

james-elicx commented Jun 25, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Review: stream ancestor loading boundaries

Verified correct

Minor, non-blocking observations

Verification

Uh oh!

ask-bonk Bot commented Jun 25, 2026

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

pkg-pr-new Bot commented Jun 25, 2026 •

edited

Loading

github-actions Bot commented Jun 25, 2026 •

edited

Loading