fix(routing): @children slot explicit page wins over catchall (#1535)#1659
Conversation
…er catchall (#1535) In Next.js, the `@children` directory is special: it provides the layout's `children` prop rather than acting as a parallel slot. `app/foo/@children/page.tsx` should therefore register a real page route at `/foo` and take priority over a sibling `[...catchAll]` catch-all. Previously vinext excluded all `@`-prefixed directories from the page scan, so `@children/page.tsx` was hidden and `/` (or `/nested`) was either served by the catch-all or failed to resolve when no `default.tsx` existed. Three small changes restore Next.js parity: - Let `@children` directories through the page-scan exclusion so the page file at `@children/page.tsx` registers as a route. - Strip a trailing `@children` segment in `fileToAppRoute` so the route is anchored at the parent directory — sibling `@slot` directories remain owned (not inherited) and layout/boundary discovery is sourced from the parent. - Skip `@children` in `discoverParallelSlots` and `hasParallelSlotDirectory` so it never appears in `parallelSlots` (matching Next.js' types-plugin behaviour). Ported from Next.js: test/e2e/app-dir/parallel-routes-catchall-children-slot/ https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts References Next.js' `normalizeAppPath` and `normalize-catchall-routes` which both treat `@children` as a non-matchable slot: - packages/next/src/shared/lib/router/utils/app-paths.ts - packages/next/src/build/normalize-catchall-routes.ts - packages/next/src/build/webpack/plugins/next-types-plugin/index.ts Fixes #1535.
|
/bigbonk review |
commit: |
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean fix, well-scoped. The four touch points (excludeDir, hasParallelSlotDirectory, fileToAppRoute, discoverParallelSlots) are exactly the right places, the Next.js source references are accurate, and the tests port the right fixture.
A few observations (none blocking):
@children/route.ts (API route handler) now gets scanned too. The excludeDir change lets the scanner walk into @children for both the **/page and **/route passes. The fileToAppRoute stripping is gated on type === "page", so a route.ts inside @children wouldn't get its dir rewritten — but isInvisibleSegment in convertSegmentsToRouteParts still strips @children from the URL, so it'd register at the parent pattern (e.g., /). This is correct behavior (matches normalizeAppPath), but worth noting that @children/route.ts is a degenerate case nobody would write in practice. If you wanted to be defensive, you could add the same path.basename(dir) === "@children" stripping for type === "route" too, but it's not necessary since convertSegmentsToRouteParts already handles it.
Deep nesting like @children/sub/page.tsx works correctly for URL patterns because isInvisibleSegment drops @children from the segment list during conversion. The layout discovery walk would traverse through the @children directory on disk though (looking for @children/layout.tsx, etc.), which matches how all @-prefixed invisible segments are handled today.
Tests are well-structured — good use of withTempApp, good assertions on both pagePath provenance and slot attachment. The scope note about deferring the second symptom from #1535 is the right call.
|
Approved. The fix is correct and well-scoped — four surgical changes at the right touch points, consistent with how Next.js handles |
#1339) (#1689) Issue #1339 reported parallel route slots falling back to default.tsx instead of the matched page. That gap was already closed on main by #1389 (resolve nested slot pages over default.tsx) and #1659/#1535 (treat @children as transparent). This adds a regression test mirroring the Next.js parallel-routes-layouts fixture — the canonical "Hello from Nested" vs "default page" case — asserting that: - the children slot at /nested resolves nested/page.tsx, not nested/default.tsx; - sibling @foo/@bar slots that each own both a page and a default pick their page; and - at /nested/subroute (only @bar matches) the children slot falls back to default, @bar mirrors its subroute page, and @foo keeps its default. Locks in page-over-default priority across sibling slots so the behavior cannot silently regress.
…1852) * fix(app-router): resolve explicit parallel slot with no page (#1535) When a parallel slot defines an explicit sub-page (e.g. `@slot/baz/page.tsx`) but no corresponding children page (`baz/page.tsx`) and no `default.tsx` for the children slot, vinext minted a synthetic `/baz` route with a null children page. That route shadowed the sibling catch-all and rendered an empty children prop, so the request hung (the upstream "explicit slot but no page" case timed out). Next.js serves such a slot-only sub-route's children from the nearest sibling catch-all. Add `findCatchAllPage` and use it as the children fallback in `discoverSlotSubRoutes` when no `default.tsx` exists, so `/baz` renders `[...catchAll]/page.tsx` for children while `@slot/baz/page.tsx` fills the slot. A children `default.tsx` still wins over the catch-all when both exist. Finishes the explicit-slot-but-no-page case of #1535 (the children-slot priority case landed in #1659). Fixes #1535. * test(app-router): document catch-all children params limitation (#1535) Address ask-bonk review: the synthetic slot-only sub-route (e.g. /baz) has a static URL pattern, so a catch-all children page reading params.catchAll gets empty params (Next.js passes ["baz"]). Document the divergence inline in discoverSlotSubRoutes and add a graph-level test locking in the current static shape so a future param-population fix must update it.
1 participant
Summary
@childrenslot/page should win over catchall and resolve when nodefault#1535 —app/foo/@children/page.tsxnow registers a real page route at/fooand beats a sibling[...catchAll]catch-all, matching Next.js.@childrenis treated as transparent in routing: it is allowed through the page scan, stripped when computing the anchored route directory, and skipped bydiscoverParallelSlots/hasParallelSlotDirectoryso it never appears inparallelSlots.tests/app-route-graph.test.tsported from Next.js'parallel-routes-catchall-children-slote2e fixture, covering both the root case and the no-default.tsxnested case.Why this matches Next.js
@childrenis the way to name the layout'schildrenprop, not a parallel slot. Next.js mirrors this in three places we cite in the code:packages/next/src/shared/lib/router/utils/app-paths.ts—normalizeAppPathstrips every@segment (including@children) from the URL.packages/next/src/build/normalize-catchall-routes.ts—isMatchableSlotexplicitly excludes@childrenso a[...catchAll]is not pushed onto a path that already has an@children/pageentry.packages/next/src/build/webpack/plugins/next-types-plugin/index.ts— the slot enumerator skips@childrenbecause it is matched to thechildrenprop.Test plan
pnpm test tests/app-route-graph.test.ts(40 tests, including 2 new)pnpm test tests/routing.test.ts tests/route-sorting.test.ts tests/app-rsc-route-matching.test.ts tests/slot.test.ts tests/route-classification-manifest.test.ts tests/app-router.test.ts tests/intercepting-routes-build.test.ts tests/metadata-routes.test.ts(no regressions)pnpm run check(format/lint/types clean)Scope note
Issue #1535 listed a second symptom from
test/e2e/app-dir/parallel-routes-catchall("explicit slot but no page times out"). That one is more invasive (catch-all route slot mirroring across a parent that has nodefault.tsxfor children) and is best handled in a separate PR — this change deliberately stays focused on the@children-vs-catchall priority fix the issue's recommendation called out first.