From 2b250d1ef74bec1115dff68225f7dfffa1baf547 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Fri, 5 Jan 2024 13:19:17 -0800 Subject: [PATCH] ensure @children slots work with catch-all routes --- .../build/normalize-catchall-routes.test.ts | 13 +++++++++ .../src/build/normalize-catchall-routes.ts | 15 ++++++++-- .../plugins/next-types-plugin/index.ts | 7 ++++- .../app/@children/page.tsx | 3 ++ .../app/@slot/default.tsx | 3 ++ .../app/@slot/page.tsx | 3 ++ .../app/[...catchAll]/page.tsx | 3 ++ .../app/layout.tsx | 18 ++++++++++++ .../app/nested/@children/page.tsx | 3 ++ .../app/nested/layout.tsx | 3 ++ .../next.config.js | 6 ++++ ...llel-routes-catchall-children-slot.test.ts | 28 +++++++++++++++++++ 12 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@children/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/default.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/[...catchAll]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/layout.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/@children/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/layout.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/next.config.js create mode 100644 test/e2e/app-dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts diff --git a/packages/next/src/build/normalize-catchall-routes.test.ts b/packages/next/src/build/normalize-catchall-routes.test.ts index 1ae0c771d883a8..8e4ca26dc19aa1 100644 --- a/packages/next/src/build/normalize-catchall-routes.test.ts +++ b/packages/next/src/build/normalize-catchall-routes.test.ts @@ -96,4 +96,17 @@ describe('normalizeCatchallRoutes', () => { ], }) }) + + it('should not add the catch-all route to a path that has a @children slot', async () => { + const appPaths = { + '/': ['/@children/page', '/@slot/page'], + '/[...slug]': ['/[...slug]/page'], + '/nested': ['/nested/@children/page'], + } + + const initialAppPaths = JSON.parse(JSON.stringify(appPaths)) + normalizeCatchAllRoutes(appPaths) + + expect(appPaths).toMatchObject(initialAppPaths) + }) }) diff --git a/packages/next/src/build/normalize-catchall-routes.ts b/packages/next/src/build/normalize-catchall-routes.ts index f24f95897e77be..a5c544834dce6e 100644 --- a/packages/next/src/build/normalize-catchall-routes.ts +++ b/packages/next/src/build/normalize-catchall-routes.ts @@ -50,11 +50,13 @@ export function normalizeCatchAllRoutes( } function hasMatchedSlots(path1: string, path2: string): boolean { - const slots1 = path1.split('/').filter((segment) => segment.startsWith('@')) - const slots2 = path2.split('/').filter((segment) => segment.startsWith('@')) + const slots1 = path1.split('/').filter(isMatchableSlot) + const slots2 = path2.split('/').filter(isMatchableSlot) + // if the catch-all route does not have the same number of slots as the app path, it can't match if (slots1.length !== slots2.length) return false + // compare the slots in both paths. For there to be a match, each slot must be the same for (let i = 0; i < slots1.length; i++) { if (slots1[i] !== slots2[i]) return false } @@ -62,6 +64,15 @@ function hasMatchedSlots(path1: string, path2: string): boolean { return true } +/** + * Returns true for slots that should be considered when checking for match compatability. + * Excludes children slots because these are similar to having a segment-level `page` + * which would cause a slot length mismatch when comparing it to a catch-all route. + */ +function isMatchableSlot(segment: string): boolean { + return segment.startsWith('@') && segment !== '@children' +} + const catchAllRouteRegex = /\[?\[\.\.\./ function isCatchAllRoute(pathname: string): boolean { diff --git a/packages/next/src/build/webpack/plugins/next-types-plugin/index.ts b/packages/next/src/build/webpack/plugins/next-types-plugin/index.ts index 7c8f7ecedd5a31..d1b4854812f2af 100644 --- a/packages/next/src/build/webpack/plugins/next-types-plugin/index.ts +++ b/packages/next/src/build/webpack/plugins/next-types-plugin/index.ts @@ -213,7 +213,12 @@ async function collectNamedSlots(layoutPath: string) { const items = await fs.readdir(layoutDir, { withFileTypes: true }) const slots = [] for (const item of items) { - if (item.isDirectory() && item.name.startsWith('@')) { + if ( + item.isDirectory() && + item.name.startsWith('@') && + // `@children slots are matched to the children prop, and should not be handled separately for type-checking + item.name !== '@children' + ) { slots.push(item.name.slice(1)) } } diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@children/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@children/page.tsx new file mode 100644 index 00000000000000..ea6444ccf0c099 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@children/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
Hello from @children/page
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/default.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/default.tsx new file mode 100644 index 00000000000000..87752cbd4d4aa1 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return
Default @slot
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/page.tsx new file mode 100644 index 00000000000000..d5331f9cbe1b8c --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/@slot/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
@slot content
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/[...catchAll]/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/[...catchAll]/page.tsx new file mode 100644 index 00000000000000..eea2f8d424d1c5 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/[...catchAll]/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return 'root catchall' +} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/layout.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/layout.tsx new file mode 100644 index 00000000000000..c6bbc6382dfdd7 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/layout.tsx @@ -0,0 +1,18 @@ +import React from 'react' + +export default function Root({ + children, + slot, +}: { + children: React.ReactNode + slot: React.ReactNode +}) { + return ( + + +
{children}
+
{slot}
+ + + ) +} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/@children/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/@children/page.tsx new file mode 100644 index 00000000000000..d1e7f0d816dfa0 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/@children/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
Hello from nested @children page
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/layout.tsx b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/layout.tsx new file mode 100644 index 00000000000000..ee078e0b328e13 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/app/nested/layout.tsx @@ -0,0 +1,3 @@ +export default function Page({ children }) { + return
{children}
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/next.config.js b/test/e2e/app-dir/parallel-routes-catchall-children-slot/next.config.js new file mode 100644 index 00000000000000..807126e4cf0bf5 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/next.config.js @@ -0,0 +1,6 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = {} + +module.exports = nextConfig diff --git a/test/e2e/app-dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts b/test/e2e/app-dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts new file mode 100644 index 00000000000000..cc5115fc1d6795 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-children-slot/parallel-routes-catchall-children-slot.test.ts @@ -0,0 +1,28 @@ +import { createNextDescribe } from 'e2e-utils' + +createNextDescribe( + 'parallel-routes-catchall-children-slot', + { + files: __dirname, + }, + ({ next }) => { + it('should match the @children slot for a page before attempting to match the catchall', async () => { + let browser = await next.browser('/') + await expect(browser.elementById('children').text()).resolves.toBe( + 'Hello from @children/page' + ) + await expect(browser.elementById('slot').text()).resolves.toBe( + '@slot content' + ) + + browser = await next.browser('/nested') + + await expect(browser.elementById('children').text()).resolves.toBe( + 'Hello from nested @children page' + ) + await expect(browser.elementById('slot').text()).resolves.toBe( + 'Default @slot' + ) + }) + } +)