From 83fb9ebc07317a1953d5daec7d6940c8aa6b89f4 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Thu, 4 Jan 2024 13:03:25 -0800 Subject: [PATCH] ensure more specific `default` segments match parallel routes before greedier catch-all segments --- packages/next/src/build/index.ts | 4 +- .../build/normalize-catchall-routes.test.ts | 23 ++++++++++ .../src/build/normalize-catchall-routes.ts | 7 ++- packages/next/src/export/index.ts | 9 +++- packages/next/src/lib/is-default-route.ts | 3 ++ .../dev-app-page-route-matcher-provider.ts | 6 ++- .../next/src/server/lib/find-page-file.ts | 5 +++ .../app/[locale]/[[...catchAll]]/page.tsx | 3 ++ .../[foo]/[bar]/@slot/[baz]/default.tsx | 3 ++ .../nested/[foo]/[bar]/@slot/[baz]/page.tsx | 3 ++ .../nested/[foo]/[bar]/@slot/default.tsx | 3 ++ .../nested/[foo]/[bar]/@slot/page.tsx | 3 ++ .../[locale]/nested/[foo]/[bar]/default.tsx | 3 ++ .../[locale]/nested/[foo]/[bar]/layout.tsx | 8 ++++ .../app/layout.tsx | 11 +++++ .../app/page.tsx | 3 ++ .../next.config.js | 6 +++ .../parallel-routes-catchall-default.test.ts | 44 +++++++++++++++++++ .../app/@slot/default.tsx | 7 --- 19 files changed, 140 insertions(+), 14 deletions(-) create mode 100644 packages/next/src/lib/is-default-route.ts create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/[[...catchAll]]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/default.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/default.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/default.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/layout.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/layout.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/app/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/next.config.js create mode 100644 test/e2e/app-dir/parallel-routes-catchall-default/parallel-routes-catchall-default.test.ts delete mode 100644 test/e2e/app-dir/parallel-routes-catchall/app/@slot/default.tsx diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index f19fde79196f5..442d0b664fb5e 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -902,7 +902,9 @@ export default async function build( validFileMatcher.isAppRouterPage(absolutePath) || // For now we only collect the root /not-found page in the app // directory as the 404 fallback - validFileMatcher.isRootNotFound(absolutePath), + validFileMatcher.isRootNotFound(absolutePath) || + // Default slots are also valid pages, and need to be considered during path normalization + validFileMatcher.isDefaultSlot(absolutePath), ignorePartFilter: (part) => part.startsWith('_'), }) ) diff --git a/packages/next/src/build/normalize-catchall-routes.test.ts b/packages/next/src/build/normalize-catchall-routes.test.ts index 1ae0c771d883a..636da8b6966fb 100644 --- a/packages/next/src/build/normalize-catchall-routes.test.ts +++ b/packages/next/src/build/normalize-catchall-routes.test.ts @@ -96,4 +96,27 @@ describe('normalizeCatchallRoutes', () => { ], }) }) + + it('should not add the catch-all route to segments that have a more specific default', () => { + const appPaths = { + '/': ['/page'], + '/[[...catchAll]]': ['/[[...catchAll]]/page'], + '/nested/[foo]/[bar]/default': [ + '/nested/[foo]/[bar]/default', + '/nested/[foo]/[bar]/@slot/default', + ], + '/nested/[foo]/[bar]': ['/nested/[foo]/[bar]/@slot/page'], + '/nested/[foo]/[bar]/[baz]/default': [ + '/nested/[foo]/[bar]/@slot/[baz]/default', + '/[[...catchAll]]/page', + ], + '/nested/[foo]/[bar]/[baz]': ['/nested/[foo]/[bar]/@slot/[baz]/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 f24f95897e77b..f84c62db21694 100644 --- a/packages/next/src/build/normalize-catchall-routes.ts +++ b/packages/next/src/build/normalize-catchall-routes.ts @@ -36,12 +36,15 @@ export function normalizeCatchAllRoutes( 0, normalizedCatchAllRoute.search(catchAllRouteRegex) ) - if ( // check if the appPath could match the catch-all appPath.startsWith(normalizedCatchAllRouteBasePath) && // check if there's not already a slot value that could match the catch-all - !appPaths[appPath].some((path) => hasMatchedSlots(path, catchAllRoute)) + !appPaths[appPath].some((path) => + hasMatchedSlots(path, catchAllRoute) + ) && + // check if the catch-all is not already matched by a default route + !appPaths[`${appPath}/default`] ) { appPaths[appPath].push(catchAllRoute) } diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts index eb949220ffb19..d99993f196faa 100644 --- a/packages/next/src/export/index.ts +++ b/packages/next/src/export/index.ts @@ -55,6 +55,7 @@ import isError from '../lib/is-error' import { needsExperimentalReact } from '../lib/needs-experimental-react' import { formatManifest } from '../build/manifests/formatter/format-manifest' import { validateRevalidate } from '../server/lib/patch-fetch' +import { isDefaultRoute } from '../lib/is-default-route' function divideSegments(number: number, segments: number): number[] { const result = [] @@ -559,9 +560,13 @@ export async function exportAppImpl( ] const filteredPaths = exportPaths.filter( - // Remove API routes (route) => - exportPathMap[route]._isAppDir || !isAPIRoute(exportPathMap[route].page) + // Remove default routes -- they don't need to be exported + // and are only used for parallel route normalization + !isDefaultRoute(exportPathMap[route].page) && + (exportPathMap[route]._isAppDir || + // Remove API routes + !isAPIRoute(exportPathMap[route].page)) ) if (filteredPaths.length !== exportPaths.length) { diff --git a/packages/next/src/lib/is-default-route.ts b/packages/next/src/lib/is-default-route.ts new file mode 100644 index 0000000000000..adbd42d508f7f --- /dev/null +++ b/packages/next/src/lib/is-default-route.ts @@ -0,0 +1,3 @@ +export function isDefaultRoute(value?: string) { + return value?.endsWith('/default') +} diff --git a/packages/next/src/server/future/route-matcher-providers/dev/dev-app-page-route-matcher-provider.ts b/packages/next/src/server/future/route-matcher-providers/dev/dev-app-page-route-matcher-provider.ts index 2a5b0f05be2fc..bb7ef7cc97587 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/dev-app-page-route-matcher-provider.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/dev-app-page-route-matcher-provider.ts @@ -19,9 +19,11 @@ export class DevAppPageRouteMatcherProvider extends FileCacheRouteMatcherProvide this.normalizers = new DevAppNormalizers(appDir, extensions) - // Match any page file that ends with `/page.${extension}` under the app + // Match any page file that ends with `/page.${extension}` or `/default.${extension}` under the app // directory. - this.expression = new RegExp(`[/\\\\]page\\.(?:${extensions.join('|')})$`) + this.expression = new RegExp( + `[/\\\\](page|default)\\.(?:${extensions.join('|')})$` + ) } protected async transform( diff --git a/packages/next/src/server/lib/find-page-file.ts b/packages/next/src/server/lib/find-page-file.ts index d51935a761d17..42b046e8efc54 100644 --- a/packages/next/src/server/lib/find-page-file.ts +++ b/packages/next/src/server/lib/find-page-file.ts @@ -127,6 +127,10 @@ export function createValidFileMatcher( return validExtensionFileRegex.test(filePath) || isMetadataFile(filePath) } + function isDefaultSlot(filePath: string) { + return filePath.endsWith(`default.${pageExtensions[0]}`) + } + function isRootNotFound(filePath: string) { if (!appDirPath) { return false @@ -143,5 +147,6 @@ export function createValidFileMatcher( isAppRouterPage, isMetadataFile, isRootNotFound, + isDefaultSlot, } } diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/[[...catchAll]]/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/[[...catchAll]]/page.tsx new file mode 100644 index 0000000000000..32aad01038734 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/[[...catchAll]]/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
/[locale]/[[...catchAll]]/page.tsx
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/default.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/default.tsx new file mode 100644 index 0000000000000..335a52a0d00de --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/default.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
/[locale]/nested/[foo]/[bar]/@slot/[baz]/default.tsx
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx new file mode 100644 index 0000000000000..6078f8edee1d1 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx @@ -0,0 +1,3 @@ +export default function Page({ params }) { + return
/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/default.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/default.tsx new file mode 100644 index 0000000000000..5a33f15b80591 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/default.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
/[locale]/nested/[foo]/[bar]/@slot/default.tsx
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/page.tsx new file mode 100644 index 0000000000000..32afde0e4032a --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/@slot/page.tsx @@ -0,0 +1,3 @@ +export default function Foo() { + return
/[locale]/nested/[foo]/[bar]/@slot/page.tsx
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/default.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/default.tsx new file mode 100644 index 0000000000000..b99ced4ae3d61 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/default.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
/[locale]/nested/[foo]/[bar]/default.tsx
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/layout.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/layout.tsx new file mode 100644 index 0000000000000..f2619e0b884b9 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/[locale]/nested/[foo]/[bar]/layout.tsx @@ -0,0 +1,8 @@ +export default function Layout({ children, slot }) { + return ( + <> + Children:
{children}
+ Slot:
{slot}
+ + ) +} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/layout.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/layout.tsx new file mode 100644 index 0000000000000..98b2ba6e286e8 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/layout.tsx @@ -0,0 +1,11 @@ +import React from 'react' + +export default function Root({ children }: { children: React.ReactNode }) { + return ( + + + Children:
{children}
+ + + ) +} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/app/page.tsx b/test/e2e/app-dir/parallel-routes-catchall-default/app/page.tsx new file mode 100644 index 0000000000000..0e3edd2c011b1 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/app/page.tsx @@ -0,0 +1,3 @@ +export default async function Home() { + return
Root Page
+} diff --git a/test/e2e/app-dir/parallel-routes-catchall-default/next.config.js b/test/e2e/app-dir/parallel-routes-catchall-default/next.config.js new file mode 100644 index 0000000000000..807126e4cf0bf --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/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-default/parallel-routes-catchall-default.test.ts b/test/e2e/app-dir/parallel-routes-catchall-default/parallel-routes-catchall-default.test.ts new file mode 100644 index 0000000000000..7c4fd86b93aad --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-catchall-default/parallel-routes-catchall-default.test.ts @@ -0,0 +1,44 @@ +import { createNextDescribe } from 'e2e-utils' + +createNextDescribe( + 'parallel-routes-catchall-default', + { + files: __dirname, + }, + ({ next }) => { + it('should match default paths before catch-all', async () => { + let browser = await next.browser('/en/nested') + + // we have a top-level catch-all but the /nested dir doesn't have a default/page until the /[foo]/[bar] segment + // so we expect the top-level catch-all to render + expect(await browser.elementById('children').text()).toBe( + '/[locale]/[[...catchAll]]/page.tsx' + ) + + browser = await next.browser('/en/nested/foo/bar') + + // we're now at the /[foo]/[bar] segment, so we expect the matched page to be the default (since there's no page defined) + expect(await browser.elementById('nested-children').text()).toBe( + '/[locale]/nested/[foo]/[bar]/default.tsx' + ) + + // we expect the slot to match since there's a page defined at this segment + expect(await browser.elementById('slot').text()).toBe( + '/[locale]/nested/[foo]/[bar]/@slot/page.tsx' + ) + + browser = await next.browser('/en/nested/foo/bar/baz') + + // the page slot should still be the one matched at the /[foo]/[bar] segment because it's the default and we + // didn't define a page at the /[foo]/[bar]/[baz] segment + expect(await browser.elementById('nested-children').text()).toBe( + '/[locale]/nested/[foo]/[bar]/default.tsx' + ) + + // however we do have a slot for the `[baz]` segment and so we expect that to no match + expect(await browser.elementById('slot').text()).toBe( + '/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx' + ) + }) + } +) diff --git a/test/e2e/app-dir/parallel-routes-catchall/app/@slot/default.tsx b/test/e2e/app-dir/parallel-routes-catchall/app/@slot/default.tsx deleted file mode 100644 index 129f875a30b3a..0000000000000 --- a/test/e2e/app-dir/parallel-routes-catchall/app/@slot/default.tsx +++ /dev/null @@ -1,7 +0,0 @@ -export default function Default() { - return ( -
-
Default
-
- ) -}