Skip to content

Commit b8b9f67

Browse files
committed
Fixes the issue by ensuring that when building a loader tree for a specific route like /payment/whoops, only the exact matching page is included, not catch-all routes that happen to be in the same directory structure.
1 parent c3e6680 commit b8b9f67

File tree

13 files changed

+175
-8
lines changed

13 files changed

+175
-8
lines changed

crates/next-core/src/next_app/mod.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,16 +258,19 @@ impl AppPage {
258258
}
259259

260260
pub fn is_catchall(&self) -> bool {
261-
let segment = if self.is_complete() {
262-
// The `PageType` is the last segment for completed pages.
263-
self.0.iter().nth_back(1)
264-
} else {
265-
self.0.last()
266-
};
261+
// The `PageType` is the last segment for completed pages. We need to find the last segment
262+
// that is not a `PageType`, `Group`, or `Parallel` segment, because these are not leaf
263+
// nodes.
264+
let last_path_segment = self.0.iter().rev().find(|segment| {
265+
!matches!(
266+
segment,
267+
PageSegment::PageType(_) | PageSegment::Group(_) | PageSegment::Parallel(_)
268+
)
269+
});
267270

268271
matches!(
269-
segment,
270-
Some(PageSegment::CatchAll(..) | PageSegment::OptionalCatchAll(..))
272+
last_path_segment,
273+
Some(PageSegment::CatchAll(_)) | Some(PageSegment::OptionalCatchAll(_))
271274
)
272275
}
273276

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import React from 'react'
2+
3+
export default function Page({ params }: { params: { slug: string[] } }) {
4+
return <h1 id="catch-all">anotherRoute catch-all: {params.slug.join('/')}</h1>
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import React from 'react'
2+
3+
export default function Page() {
4+
return <h1 id="specific-page">anotherRoute whoops</h1>
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import React from 'react'
2+
3+
export default function Page() {
4+
return <h1 id="parallel-page">parallel page</h1>
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import React from 'react'
2+
3+
export default function Layout({
4+
children,
5+
parallel,
6+
}: {
7+
children: React.ReactNode
8+
parallel: React.ReactNode
9+
}) {
10+
return (
11+
<>
12+
{children}
13+
{parallel}
14+
</>
15+
)
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import React from 'react'
2+
3+
export default function Page({ params }: { params: { slug: string[] } }) {
4+
return <h1 id="catch-all">payment catch-all: {params.slug.join('/')}</h1>
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import React from 'react'
2+
3+
export default function Page() {
4+
return <h1 id="specific-page">payment whoops</h1>
5+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import React from 'react'
2+
3+
export default function RootLayout({
4+
children,
5+
}: {
6+
children: React.ReactNode
7+
}) {
8+
return (
9+
<html>
10+
<head>
11+
<title>Test App</title>
12+
</head>
13+
<body>{children}</body>
14+
</html>
15+
)
16+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import React from 'react'
2+
3+
export default function Page() {
4+
return <h1>Root Page</h1>
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { nextTestSetup } from 'e2e-utils'
2+
3+
describe('app-dir - middleware rewrite with catch-all and parallel routes', () => {
4+
const { next } = nextTestSetup({
5+
files: __dirname,
6+
})
7+
8+
describe('payment route', () => {
9+
it('should rewrite to the specific page instead of the catch-all with parallel route', async () => {
10+
const browser = await next.browser('/payment/test')
11+
const text = await browser.elementByCss('#specific-page').text()
12+
13+
expect(text).toBe('payment whoops')
14+
expect(await browser.url()).toBe(next.url + '/payment/test')
15+
})
16+
17+
it('should redirect to the specific page instead of the catch-all with parallel route', async () => {
18+
const browser = await next.browser('/payment/test?redirect=true')
19+
const text = await browser.elementByCss('#specific-page').text()
20+
21+
expect(text).toBe('payment whoops')
22+
expect(await browser.url()).toBe(next.url + '/payment/whoops')
23+
})
24+
})
25+
26+
describe('anotherRoute', () => {
27+
it('should rewrite to the specific page instead of the catch-all without parallel route', async () => {
28+
const browser = await next.browser('/anotherRoute/test')
29+
const text = await browser.elementByCss('#specific-page').text()
30+
31+
expect(text).toBe('anotherRoute whoops')
32+
expect(await browser.url()).toBe(next.url + '/anotherRoute/test')
33+
})
34+
35+
it('should redirect to the specific page instead of the catch-all without parallel route', async () => {
36+
const browser = await next.browser('/anotherRoute/test?redirect=true')
37+
const text = await browser.elementByCss('#specific-page').text()
38+
39+
expect(text).toBe('anotherRoute whoops')
40+
expect(await browser.url()).toBe(next.url + '/anotherRoute/whoops')
41+
})
42+
})
43+
})

0 commit comments

Comments
 (0)