fix: rewrite useSelectedLayoutSegment(s) to use context-based child segments

[NathanDrake2406]

Summary

• Replaces depth-based pathname slicing with context-based child segments for useSelectedLayoutSegment(s), fixing incorrect behavior when route groups are present in the App Router tree
• Each layout now receives pre-computed child segments through React context, including route groups (which don't appear in URLs but Next.js includes in the segments array) and resolved dynamic param values
• Passes matched params through to error/fallback rendering paths so LayoutSegmentProvider values stay consistent with buildPageElement() during error page rendering
• Handles optional catch-all edge cases (empty array / undefined params) and extends routeSegments for parallel slot sub-routes

Test plan

• Existing unit tests updated and passing (tests/shims.test.ts, tests/app-router.test.ts)
• New integration test added (tests/nextjs-compat/layout-segments.test.ts) with fixture pages covering:
  • Simple paths with outer/inner layout segments
  • Route groups appearing in segments array
  • Dynamic param resolution in segments
  • Catch-all segments joined with "/"
  • Leaf page returning empty segments / null
• Typecheck passes
• CI (Lint, Typecheck, Vitest, Playwright E2E)

[pkg-pr-new]

Open in StackBlitz

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

commit: 3f6cd51

[james-elicx]

/bigbonk review

[ask-bonk]

Review: fix useSelectedLayoutSegment(s) to use context-based child segments

This is a well-designed fix. The old approach (slicing pathname by URL depth) was fundamentally broken for route groups because groups exist in the filesystem tree but not in URLs. By pre-computing the child segments from the route tree and passing them through React context, the hooks now correctly report route groups in the segments array, matching Next.js behavior.

The architecture is sound:

• routeSegments captures the full filesystem path (including route groups, dynamic templates)
• layoutTreePositions counts ALL directory levels (not just URL-contributing ones), so slicing routeSegments at each layout's position gives the correct "below this layout" segments
• __resolveChildSegments resolves dynamic templates to actual param values at render time
• Leaf pages get wrapped with childSegments: [] so hooks return empty at leaf level
• Error/not-found paths correctly receive params to maintain segment consistency

A few minor observations below, none blocking.

[ask-bonk]

Nit: when v2 is falsy (e.g. params[pn2] is undefined), this falls back to the raw template string like [...slug]. That's a reasonable defensive fallback, but catch-all params should always be present when the route matches. Consider whether logging a warning here would help catch routing bugs, or if the silent fallback is fine.

Same applies to line 304 for simple dynamic segments.

[ask-bonk]

Nit: this function uses var declarations with suffixed names (pn, pn2, pn3) to avoid scoping issues in the if/else if chain. This is consistent with the rest of the generated code in this file, but using let with a shared variable name would be cleaner since each branch is mutually exclusive:

Suggested change

	function __resolveChildSegments(routeSegments, treePosition, params) {
	function __resolveChildSegments(routeSegments, treePosition, params) {
	let raw = routeSegments.slice(treePosition);
	let result = [];
	for (let j = 0; j < raw.length; j++) {
	let seg = raw[j];

Not a correctness issue — just readability.

[ask-bonk]

Good change — switching from "next/navigation" to the relative "./navigation.js" avoids a potential circular resolution through the virtual module system. Since layout-segment-context.tsx is itself resolved via vinext/layout-segment-context, importing next/navigation could create an unnecessary resolution hop.

[ask-bonk]

Nice test coverage. The fixture structure exercises the key scenarios well (simple path, dynamic params, route groups, catch-all, leaf page).

One scenario that might be worth adding in a follow-up: optional catch-all [[...slug]] where the page is visited at the base path (no slug provided). The __resolveChildSegments function handles this by skipping the segment, but it's not tested here.

[ask-bonk]

Review submitted successfully. Here's a summary of the review:

Approved with minor comments.

The PR correctly fixes useSelectedLayoutSegment(s) by replacing the broken depth-based pathname slicing approach with context-based child segments. The old approach couldn't handle route groups since they exist in the filesystem but not in URLs. The new approach pre-computes child segments from the route tree (including route groups and resolved dynamic params) and passes them through React context.

Key observations left on the PR:

1. __resolveChildSegments fallback behavior — when catch-all or dynamic params are missing, the function silently falls back to the raw template string (e.g., [...slug]). Worth considering whether a warning would help catch routing bugs.

2. var declarations style — the function uses var with suffixed names (pn, pn2, pn3) where let with shared names would be cleaner, though this matches the generated code style elsewhere in the file.

3. Import path change — commended the switch from "next/navigation" to "./navigation.js" in layout-segment-context.tsx, which avoids circular resolution through the virtual module system.

4. Test coverage — suggested adding optional catch-all [[...slug]] at the base path as a follow-up test, since the code handles it but it's not exercised in the new test suite.

github run