Skip to content

Commit

Permalink
Ensure beforeFiles client-side resolving handles dynamic routes (#36317)
Browse files Browse the repository at this point in the history
This fixes resolving `beforeFiles` rewrites with `next/link` as previously we weren't resolving the destination to a dynamic route since the resolving was being marked as finished when it shouldn't be during `beforeFiles`. 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`

Fixes: #35402
  • Loading branch information
ijjk committed Apr 21, 2022
1 parent 864d401 commit 6da7132
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/next/shared/lib/router/utils/resolve-rewrites.ts
Expand Up @@ -106,7 +106,7 @@ export default function resolveRewrites(
for (let i = 0; i < rewrites.beforeFiles.length; i++) {
// we don't end after match in beforeFiles to allow
// continuing through all beforeFiles rewrites
finished = handleRewrite(rewrites.beforeFiles[i]) || false
handleRewrite(rewrites.beforeFiles[i])
}
matchedPage = pages.includes(fsPathname)

Expand Down
4 changes: 4 additions & 0 deletions test/integration/custom-routes/next.config.js
Expand Up @@ -224,6 +224,10 @@ module.exports = {
source: '/overridden',
destination: 'https://vercel.com',
},
{
source: '/nfl/:path*',
destination: '/_sport/nfl/:path*',
},
],
}
},
Expand Down
13 changes: 13 additions & 0 deletions test/integration/custom-routes/pages/_sport/[slug]/index.js
@@ -0,0 +1,13 @@
import { useRouter } from 'next/router'

export default function Page() {
const router = useRouter()
return (
<>
<p>/_sport/[slug]</p>
<p id="query">{JSON.stringify(router.query)}</p>
<p id="as-path">{router.asPath}</p>
<p id="pathname">{router.pathname}</p>
</>
)
}
13 changes: 13 additions & 0 deletions test/integration/custom-routes/pages/_sport/[slug]/test.js
@@ -0,0 +1,13 @@
import { useRouter } from 'next/router'

export default function Page() {
const router = useRouter()
return (
<>
<p>/_sport/[slug]/test</p>
<p id="query">{JSON.stringify(router.query)}</p>
<p id="as-path">{router.asPath}</p>
<p id="pathname">{router.pathname}</p>
</>
)
}
9 changes: 9 additions & 0 deletions test/integration/custom-routes/pages/nav.js
Expand Up @@ -44,6 +44,15 @@ const Page = () => (
<a id="to-before-files-overridden">to /overridden</a>
</Link>
<br />
<Link href="/nfl">
<a id="to-before-files-dynamic">to /nfl</a>
</Link>
<br />
<Link href="/nfl/test">
<a id="to-before-files-dynamic-again">to /nfl/test</a>
</Link>
<br />
</>
)

export default Page
75 changes: 75 additions & 0 deletions test/integration/custom-routes/test/index.test.js
Expand Up @@ -99,6 +99,58 @@ const runTests = (isDev = false) => {
)
})

it('should handle beforeFiles rewrite to dynamic route correctly', async () => {
const res = await fetchViaHTTP(appPort, '/nfl')
const html = await res.text()

if (res.status !== 200) {
console.error('Invalid response', html)
}
expect(res.status).toBe(200)
expect(html).toContain('/_sport/[slug]')

const browser = await webdriver(appPort, '/nav')
await browser.eval('window.beforeNav = 1')
await browser.elementByCss('#to-before-files-dynamic').click()
await check(
() => browser.eval('document.documentElement.innerHTML'),
/_sport\/\[slug\]/
)
expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'nfl',
})
expect(await browser.elementByCss('#pathname').text()).toBe(
'/_sport/[slug]'
)
expect(await browser.eval('window.beforeNav')).toBe(1)
})

it('should handle beforeFiles rewrite to partly dynamic route correctly', async () => {
const res = await fetchViaHTTP(appPort, '/nfl')
const html = await res.text()

if (res.status !== 200) {
console.error('Invalid response', html)
}
expect(res.status).toBe(200)
expect(html).toContain('/_sport/[slug]')

const browser = await webdriver(appPort, '/nav')
await browser.eval('window.beforeNav = 1')
await browser.elementByCss('#to-before-files-dynamic-again').click()
await check(
() => browser.eval('document.documentElement.innerHTML'),
/_sport\/\[slug\]\/test/
)
expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'nfl',
})
expect(await browser.elementByCss('#pathname').text()).toBe(
'/_sport/[slug]/test'
)
expect(await browser.eval('window.beforeNav')).toBe(1)
})

it('should support long URLs for rewrites', async () => {
const res = await fetchViaHTTP(
appPort,
Expand Down Expand Up @@ -1720,6 +1772,13 @@ const runTests = (isDev = false) => {
regex: normalizeRegEx('^\\/overridden(?:\\/)?$'),
source: '/overridden',
},
{
destination: '/_sport/nfl/:path*',
regex: normalizeRegEx(
'^\\/nfl(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?(?:\\/)?$'
),
source: '/nfl/:path*',
},
],
afterFiles: [
{
Expand Down Expand Up @@ -1973,6 +2032,22 @@ const runTests = (isDev = false) => {
fallback: [],
},
dynamicRoutes: [
{
namedRegex: '^/_sport/(?<slug>[^/]+?)(?:/)?$',
page: '/_sport/[slug]',
regex: normalizeRegEx('^\\/_sport\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: {
slug: 'slug',
},
},
{
namedRegex: '^/_sport/(?<slug>[^/]+?)/test(?:/)?$',
page: '/_sport/[slug]/test',
regex: normalizeRegEx('^\\/_sport\\/([^\\/]+?)\\/test(?:\\/)?$'),
routeKeys: {
slug: 'slug',
},
},
{
namedRegex: '^/another/(?<id>[^/]+?)(?:/)?$',
page: '/another/[id]',
Expand Down

0 comments on commit 6da7132

Please sign in to comment.