Skip to content

Commit

Permalink
ensure more specific default segments match parallel routes before …
Browse files Browse the repository at this point in the history
…greedier catch-all segments
  • Loading branch information
ztanner committed Jan 5, 2024
1 parent 91dba7d commit 83fb9eb
Show file tree
Hide file tree
Showing 19 changed files with 140 additions and 14 deletions.
4 changes: 3 additions & 1 deletion packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('_'),
})
)
Expand Down
23 changes: 23 additions & 0 deletions packages/next/src/build/normalize-catchall-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
7 changes: 5 additions & 2 deletions packages/next/src/build/normalize-catchall-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
9 changes: 7 additions & 2 deletions packages/next/src/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions packages/next/src/lib/is-default-route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function isDefaultRoute(value?: string) {
return value?.endsWith('/default')
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions packages/next/src/server/lib/find-page-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -143,5 +147,6 @@ export function createValidFileMatcher(
isAppRouterPage,
isMetadataFile,
isRootNotFound,
isDefaultSlot,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>/[locale]/[[...catchAll]]/page.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>/[locale]/nested/[foo]/[bar]/@slot/[baz]/default.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page({ params }) {
return <div>/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>/[locale]/nested/[foo]/[bar]/@slot/default.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Foo() {
return <div>/[locale]/nested/[foo]/[bar]/@slot/page.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>/[locale]/nested/[foo]/[bar]/default.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function Layout({ children, slot }) {
return (
<>
Children: <div id="nested-children">{children}</div>
Slot: <div id="slot">{slot}</div>
</>
)
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/parallel-routes-catchall-default/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from 'react'

export default function Root({ children }: { children: React.ReactNode }) {
return (
<html>
<body>
Children: <div id="children">{children}</div>
</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default async function Home() {
return <div>Root Page</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -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'
)
})
}
)

This file was deleted.

0 comments on commit 83fb9eb

Please sign in to comment.