Return leaf selected layout segment for named parallel routes#909
Conversation
commit: |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Aligns useSelectedLayoutSegment(parallelRoutesKey) behavior with Next.js for named parallel routes by returning the leaf segment for named slots while keeping first child behavior for the default children route.
Changes:
- Update
useSelectedLayoutSegment()reduction logic to return the last segment for non-childrenparallel route keys. - Add a nested
@team/members/profilefixture to exercise nested named parallel slot routing. - Add an integration test asserting both
useSelectedLayoutSegments("team")anduseSelectedLayoutSegment("team")behaviors.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/fixtures/app-basic/app/dashboard/@team/members/profile/page.tsx | Adds a nested named slot leaf page fixture (profile). |
| tests/app-router.test.ts | Adds regression coverage for leaf segment selection in nested named slots. |
| packages/vinext/src/shims/navigation.ts | Implements Next.js-aligned segment selection for named parallel routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(html).toMatch( | ||
| /data-testid="team-segments"[^>]*>\["members","profile"\]/, | ||
| ); |
There was a problem hiding this comment.
This assertion is tightly coupled to the exact HTML entity encoding and formatting of the rendered array (e.g., ", spacing, potential newlines). That makes the test brittle to unrelated rendering/serialization changes. Prefer extracting the data-testid=\"team-segments\" element’s text content and asserting on a normalized value (e.g., parse JSON / compare arrays), or relax the regex to tolerate whitespace and different escaping.
useSelectedLayoutSegment(parallelRoutesKey) returned the first segment for every route key. That matches children routes, but it misreports nested named parallel routes such as a slot path with members/profile by returning members instead of the active leaf profile. The root cause was treating the array returned by useSelectedLayoutSegments() as having the same reduction rule for children and named slots. Match Next.js by keeping first-segment behaviour for children and using the final segment for non-children parallel route keys. Adds a nested @team fixture route that mirrors Next.js parallel-routes-use-selected-layout-segment coverage.
NathanDrake2406
force-pushed
the
nathan/fix-selected-layout-segment-parallel-routes
branch
from
e6d3ea4 to
dd6f56f
Compare
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nice fix — the implementation matches Next.js's computeSelectedLayoutSegment() exactly (source) for the cases vinext exercises.
Notes:
- The early
return nullfor empty segments correctly subsumes Next.js'sDEFAULT_SEGMENT_KEYcheck, because vinext omits default-slot segments entirely rather than using a__DEFAULT__sentinel (verified via grep — no__DEFAULT__references in the codebase). If vinext ever introduces a sentinel for default slots, this hook will need to be revisited. - The new
textContentByTestId+JSON.parsehelper neatly addresses the brittleness Copilot flagged in earlier"-based regex assertions, and tightens theuseSelectedLayoutSegmentschecks across the file. - Test coverage hits the exact case from the linked Next.js E2E (
useSelectedLayoutSegments("team")→["members", "profile"],useSelectedLayoutSegment("team")→"profile"). - Fixture layout is sensible:
@team/members/profile/page.tsxreuses the existing@teamslot and the children slot falls back todefault.tsxon/dashboard/members/profile(noapp/dashboard/members/...children page exists). That matches what the existing/dashboard/memberstests already rely on.
Only nit below — non-blocking.
| expect(res.status).toBe(200); | ||
| const html = await res.text(); | ||
|
|
||
| expect(html).toContain('data-testid="team-member-profile-page"'); |
There was a problem hiding this comment.
Minor: this toContain is redundant with the segment assertions on the next two lines — if team-segments resolves to ["members", "profile"], the leaf page must have rendered. Not worth blocking on; feel free to keep it as a readability sanity check or drop it.
|
Review posted: #909 (review) SummaryThe PR is a clean, well-targeted fix that aligns
Posted one non-blocking nit about a redundant |
NathanDrake2406
deleted the
nathan/fix-selected-layout-segment-parallel-routes
branch
What this changes
useSelectedLayoutSegment(parallelRoutesKey)now matches Next.js for named parallel routes: the defaultchildrenroute still returns the first child segment, while named slots return the active leaf segment from that slot path.This fixes nested parallel slot routes such as
@team/members/profile, whereuseSelectedLayoutSegments("team")returns["members", "profile"]anduseSelectedLayoutSegment("team")should return"profile".Why
Vinext previously reduced every segment array with
segments[0]. That is only correct for the defaultchildrenroute. Next.js intentionally treats named parallel route keys differently incomputeSelectedLayoutSegment():children: first segmentchildrenparallel route keys: last segmentSource references:
Approach
The shim now returns
nullfor an empty segment list, returns the first segment forundefinedor"children", and returns the final segment for named parallel route keys.The regression test adds a nested
@teamslot fixture and asserts both behaviours together:useSelectedLayoutSegments("team")exposes the full slot path["members", "profile"]useSelectedLayoutSegment("team")returns the leaf segment"profile"Validation
vp test run tests/app-router.test.ts -t "useSelectedLayoutSegment"vp check tests/app-router.test.ts packages/vinext/src/shims/navigation.ts tests/fixtures/app-basic/app/dashboard/@team/members/profile/page.tsxRisks / follow-ups
This is scoped to the client navigation shim reduction rule. It does not change route discovery or segment-map construction, which already preserves the full slot segment path needed for this behaviour.