Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix parallel route top-level catch-all normalization logic to support nested explicit (non-catchall) slot routes #60776

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
74a2fb4
checks if appPath ends in a dynamic route, preventing catchall from t…
williamli Jan 17, 2024
4a865c5
add support for segments that ends with a more specific [dynamicRoute]
williamli Jan 17, 2024
0e68b4b
Merge branch 'canary' into fix-parallel-routes-top-level-catchall-but…
williamli Jan 17, 2024
2af4569
added new e2e test for parallel-routes-catchall-dynamic-segment
williamli Jan 17, 2024
c9be4c4
Merge branch 'canary' into fix-parallel-routes-top-level-catchall-but…
williamli Jan 19, 2024
b04d877
revise isMoreSpecific, pathname is more specific even when having sam…
williamli Jan 19, 2024
64ff919
better function names and comments
williamli Jan 19, 2024
43f86c4
added a more complex test for [dynamicRoute], mixing in more slots an…
williamli Jan 19, 2024
f3af8e7
more comprehensive test involving nested [dynamicRoutes], activate e2…
williamli Jan 19, 2024
a5a4e35
Merge branch 'canary' into fix-parallel-routes-top-level-catchall-but…
williamli Jan 19, 2024
3d1825e
Merge branch 'canary' into fix-parallel-routes-top-level-catchall-but…
williamli Jan 19, 2024
affd5bb
fix typo in a related test: parallel-routes-catchall-default.test
williamli Jan 19, 2024
519950a
revise page name
williamli Jan 19, 2024
17e8687
expand normalize function to support nested non-catchall routes
williamli Jan 19, 2024
8c3335a
added e2e test for top-level catchall with parallel non-catchall rout…
williamli Jan 19, 2024
fb93863
update comment
williamli Jan 19, 2024
e0d7750
isMoreSpecific function should use > instead of >=
williamli Jan 19, 2024
3fe3ed3
add parallel-routes-catchall-slotted-non-catchalls to test manifest a…
williamli Jan 19, 2024
4f368f5
fix test case naming
williamli Jan 19, 2024
897e2d1
uncomment test case
williamli Jan 19, 2024
41cdffd
update test describe
williamli Jan 20, 2024
a4ace6c
Update test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/para…
williamli Jan 23, 2024
26678e1
Update test/e2e/app-dir/parallel-routes-catchall-dynamic-segment/para…
williamli Jan 23, 2024
0c35ee2
Update test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchall…
williamli Jan 23, 2024
9385dde
Merge branch 'canary' into fix-parallel-routes-top-level-catchall-but…
williamli Jan 23, 2024
c2143d4
remove test
williamli Jan 23, 2024
27d096d
remove test/e2e/app-dir/parallel-routes-catchall-slotted-non-catchall…
williamli Jan 23, 2024
21ffa68
Merge branch 'canary' into fix-parallel-routes-top-level-catchall-but…
ztanner Jan 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 55 additions & 0 deletions packages/next/src/build/normalize-catchall-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,61 @@ describe('normalizeCatchallRoutes', () => {
expect(appPaths).toMatchObject(initialAppPaths)
})

it('should not add the catch-all route to segments that have a more specific [dynamicRoute]', () => {
const appPaths = {
'/': ['/page'],
'/[[...catchAll]]': ['/[[...catchAll]]/page'],
'/nested/[foo]/[bar]/default': [
'/nested/[foo]/[bar]/default',
'/nested/[foo]/[bar]/@slot0/default',
'/nested/[foo]/[bar]/@slot2/default',
],
'/nested/[foo]/[bar]': [
'/nested/[foo]/[bar]/@slot0/page',
'/nested/[foo]/[bar]/@slot1/page',
],
'/nested/[foo]/[bar]/[baz]': [
'/nested/[foo]/[bar]/@slot0/[baz]/page',
'/nested/[foo]/[bar]/@slot1/[baz]/page',
],
'/[locale]/nested/[foo]/[bar]/[baz]/[qux]': [
'/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page',
],
}

const initialAppPaths = JSON.parse(JSON.stringify(appPaths))

// normalize appPaths against catchAlls
normalizeCatchAllRoutes(appPaths)

// ensure values are correct after normalizing
expect(appPaths).toMatchObject(initialAppPaths)
})

it('should not add the catch-all route to non-catchall segments that are more specific', () => {
const appPaths = {
'/': ['/page'],
'/[locale]/[[...catchAll]]': ['/[locale]/[[...catchAll]]/page'],
'/[locale]/nested/default': [
'/[locale]/nested/default',
'/[locale]/nested/@slot0/default',
'/[locale]/nested/@slot1/default',
],
'/[locale]/nested': ['/[locale]/nested/page'],
'/[locale]/nested/bar': ['/[locale]/nested/@slot0/bar/page'],
'/[locale]/nested/foo': ['/[locale]/nested/@slot0/foo/page'],
'/[locale]/nested/baz': ['/[locale]/nested/@slot1/baz/page'],
}

const initialAppPaths = JSON.parse(JSON.stringify(appPaths))

// normalize appPaths against catchAlls
normalizeCatchAllRoutes(appPaths)

// ensure values are correct after normalizing
expect(appPaths).toMatchObject(initialAppPaths)
})

it('should not add the catch-all route to a path that has a @children slot', async () => {
const appPaths = {
'/': ['/@children/page', '/@slot/page'],
Expand Down
14 changes: 12 additions & 2 deletions packages/next/src/build/normalize-catchall-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,18 @@ 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)
) &&
// check if the catch-all is not already matched by a default route
!appPaths[`${appPath}/default`]
// check if the catch-all is not already matched by a default route or page route
!appPaths[`${appPath}/default`] &&
// check if appPath is a catch-all OR is not more specific than the catch-all
(isCatchAllRoute(appPath) || !isMoreSpecific(appPath, catchAllRoute))
) {
appPaths[appPath].push(catchAllRoute)
}
Expand Down Expand Up @@ -81,3 +84,10 @@ const catchAllRouteRegex = /\[?\[\.\.\./
function isCatchAllRoute(pathname: string): boolean {
return pathname.includes('[...') || pathname.includes('[[...')
}

// test to see if a path is more specific than a catch-all route
function isMoreSpecific(pathname: string, catchAllRoute: string): boolean {
const pathnameDepth = pathname.split('/').length
const catchAllRouteDepth = catchAllRoute.split('/').length - 1
return pathnameDepth > catchAllRouteDepth
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ createNextDescribe(
'/[locale]/nested/[foo]/[bar]/default.tsx'
)

// however we do have a slot for the `[baz]` segment and so we expect that to no match
// however we do have a slot for the `[baz]` segment and so we expect that to match
expect(await browser.elementById('slot').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot/[baz]/page.tsx'
)
Expand Down
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({ params }) {
return <div>/[locale]/nested/[foo]/[bar]/@slot0/[baz]/page.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]/@slot0/default.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]/@slot0/page.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]/@slot1/[baz]/[qux]/page.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]/@slot1/[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]/@slot1/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]/@slot2/default.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,10 @@
export default function Layout({ children, slot0, slot1, slot2 }) {
return (
<>
Children: <div id="nested-children">{children}</div>
Slot0: <div id="slot0">{slot0}</div>
Slot1: <div id="slot1">{slot1}</div>
Slot2: <div id="slot2">{slot2}</div>
</>
)
}
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,87 @@
import { createNextDescribe } from 'e2e-utils'

createNextDescribe(
'parallel-routes-catchall-dynamic-segment',
{
files: __dirname,
},
({ next }) => {
it('should match default and dynamic segment 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 slot0 to match since there's a page defined at this segment
expect(await browser.elementById('slot0').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot0/page.tsx'
)

// we expect the slot1 to match since there's a page defined at this segment
expect(await browser.elementById('slot1').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot1/page.tsx'
)

// we expect the slot2 to match since there's a default page defined at this segment
expect(await browser.elementById('slot2').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot2/default.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'
)

// we do have a slot for the `[baz]` dynamic segment in slot0 and so we expect that to match
expect(await browser.elementById('slot0').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot0/[baz]/page.tsx'
)

// we do have a slot for the `[baz]` dynamic segment in slot1 and so we expect that to match
expect(await browser.elementById('slot1').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot1/[baz]/page.tsx'
)

// we do not have a slot for the `[baz]` dynamic segment in slot2 and so the default page is matched
expect(await browser.elementById('slot2').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot2/default.tsx'
)

browser = await next.browser('/en/nested/foo/bar/baz/qux')

// 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]/[qux] segment
expect(await browser.elementById('nested-children').text()).toBe(
'/[locale]/nested/[foo]/[bar]/default.tsx'
)

// we do not have a slot for the `[baz]/[qux]` dynamic segment in slot0 and so we expect the default page at `@slot0/` to be returned
expect(await browser.elementById('slot0').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot0/default.tsx'
)

// we do have a slot for the `[baz]/[qux]` dynamic segment in slot1 and so we expect that to no match
expect(await browser.elementById('slot1').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot1/[baz]/[qux]/page.tsx'
)

// we do not have a slot for the `[baz]/[qux]` dynamic segment in slot2 and so we expect the default page at `@slot2/` to be returned
expect(await browser.elementById('slot2').text()).toBe(
'/[locale]/nested/[foo]/[bar]/@slot2/default.tsx'
)
})
}
)
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({ params }) {
return <div>/[locale]/nested/@slot0/bar/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/@slot0/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/@slot0/foo/page.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/@slot1/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/@slot1/default.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>/[locale]/nested/default.tsx</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function Layout({ children, slot0, slot1 }) {
return (
<>
Children: <div id="nested-children">{children}</div>
Slot0: <div id="slot0">{slot0}</div>
Slot1: <div id="slot1">{slot1}</div>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>/[locale]/nested/page.tsx</div>
}
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