Skip to content

Commit 007c53c

Browse files
Cy-TekCy-Tek
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 007c53c

File tree

13 files changed

+170
-9
lines changed

13 files changed

+170
-9
lines changed

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -257,17 +257,22 @@ impl AppPage {
257257
matches!(self.0.last(), Some(PageSegment::PageType(..)))
258258
}
259259

260-
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-
};
260+
/// The `PageType` is the last segment for completed pages. We need to find
261+
/// the last segment that is not a `PageType`, `Group`, or `Parallel`
262+
/// segment, because these do not inform the routing structure.
263+
pub fn get_last_routing_segment(&self) -> Option<&PageSegment> {
264+
self.0.iter().rev().find(|segment| {
265+
!matches!(
266+
segment,
267+
PageSegment::PageType(_) | PageSegment::Group(_) | PageSegment::Parallel(_)
268+
)
269+
})
270+
}
267271

272+
pub fn is_catchall(&self) -> bool {
268273
matches!(
269-
segment,
270-
Some(PageSegment::CatchAll(..) | PageSegment::OptionalCatchAll(..))
274+
self.get_last_routing_segment(),
275+
Some(PageSegment::CatchAll(_) | PageSegment::OptionalCatchAll(_))
271276
)
272277
}
273278

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/(someGroup)/anotherRoute/[[...slug]]/page.jsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import React from 'react'
2+
3+
export default function Page({ params }) {
4+
return (
5+
<h1 id="catch-all">anotherRoute catch-all: {params.slug?.join('/')}</h1>
6+
)
7+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/(someGroup)/anotherRoute/whoops/page.jsx

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 id="specific-page">anotherRoute whoops</h1>
5+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/(someGroup)/payment/[[...slug]]/@parallel/page.jsx

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 id="parallel-page">parallel page</h1>
5+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/(someGroup)/payment/[[...slug]]/layout.jsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import React from 'react'
2+
3+
export default function Layout({ children, parallel }) {
4+
return (
5+
<>
6+
{children}
7+
{parallel}
8+
</>
9+
)
10+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/(someGroup)/payment/[[...slug]]/page.jsx

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({ params }) {
4+
return <h1 id="catch-all">payment catch-all: {params.slug?.join('/')}</h1>
5+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/(someGroup)/payment/whoops/page.jsx

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 id="specific-page">payment whoops</h1>
5+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/layout.jsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import React from 'react'
2+
3+
export default function RootLayout({ children }) {
4+
return (
5+
<html>
6+
<head>
7+
<title>Test App</title>
8+
</head>
9+
<body>{children}</body>
10+
</html>
11+
)
12+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/app/page.jsx

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+
}

‎test/e2e/app-dir/middleware-rewrite-catchall-priority-with-parallel-route/middleware-rewrite-catchall-priority-with-parallel-route.test.js

Lines changed: 43 additions & 0 deletions
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)