From 6da71321df76381491cfd812445e57d2eb0e98b6 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 21 Apr 2022 09:58:43 -0500 Subject: [PATCH] Ensure beforeFiles client-side resolving handles dynamic routes (#36317) 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: https://github.com/vercel/next.js/issues/35402 --- .../lib/router/utils/resolve-rewrites.ts | 2 +- test/integration/custom-routes/next.config.js | 4 + .../pages/_sport/[slug]/index.js | 13 ++++ .../custom-routes/pages/_sport/[slug]/test.js | 13 ++++ test/integration/custom-routes/pages/nav.js | 9 +++ .../custom-routes/test/index.test.js | 75 +++++++++++++++++++ 6 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 test/integration/custom-routes/pages/_sport/[slug]/index.js create mode 100644 test/integration/custom-routes/pages/_sport/[slug]/test.js diff --git a/packages/next/shared/lib/router/utils/resolve-rewrites.ts b/packages/next/shared/lib/router/utils/resolve-rewrites.ts index 30fe1a35a4f56a8..c69bac0091a2d8e 100644 --- a/packages/next/shared/lib/router/utils/resolve-rewrites.ts +++ b/packages/next/shared/lib/router/utils/resolve-rewrites.ts @@ -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) diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index 7780bda50eebdff..70814483c17eb92 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -224,6 +224,10 @@ module.exports = { source: '/overridden', destination: 'https://vercel.com', }, + { + source: '/nfl/:path*', + destination: '/_sport/nfl/:path*', + }, ], } }, diff --git a/test/integration/custom-routes/pages/_sport/[slug]/index.js b/test/integration/custom-routes/pages/_sport/[slug]/index.js new file mode 100644 index 000000000000000..5c0e0dfbda5d2c4 --- /dev/null +++ b/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 ( + <> +

/_sport/[slug]

+

{JSON.stringify(router.query)}

+

{router.asPath}

+

{router.pathname}

+ + ) +} diff --git a/test/integration/custom-routes/pages/_sport/[slug]/test.js b/test/integration/custom-routes/pages/_sport/[slug]/test.js new file mode 100644 index 000000000000000..ccbe746692eed75 --- /dev/null +++ b/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 ( + <> +

/_sport/[slug]/test

+

{JSON.stringify(router.query)}

+

{router.asPath}

+

{router.pathname}

+ + ) +} diff --git a/test/integration/custom-routes/pages/nav.js b/test/integration/custom-routes/pages/nav.js index 1b6663537c4b173..e2acbb4d22fde30 100644 --- a/test/integration/custom-routes/pages/nav.js +++ b/test/integration/custom-routes/pages/nav.js @@ -44,6 +44,15 @@ const Page = () => ( to /overridden
+ + to /nfl + +
+ + to /nfl/test + +
) + export default Page diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 404d5f7f977dac8..ae28cee1f73186f 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -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, @@ -1720,6 +1772,13 @@ const runTests = (isDev = false) => { regex: normalizeRegEx('^\\/overridden(?:\\/)?$'), source: '/overridden', }, + { + destination: '/_sport/nfl/:path*', + regex: normalizeRegEx( + '^\\/nfl(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?(?:\\/)?$' + ), + source: '/nfl/:path*', + }, ], afterFiles: [ { @@ -1973,6 +2032,22 @@ const runTests = (isDev = false) => { fallback: [], }, dynamicRoutes: [ + { + namedRegex: '^/_sport/(?[^/]+?)(?:/)?$', + page: '/_sport/[slug]', + regex: normalizeRegEx('^\\/_sport\\/([^\\/]+?)(?:\\/)?$'), + routeKeys: { + slug: 'slug', + }, + }, + { + namedRegex: '^/_sport/(?[^/]+?)/test(?:/)?$', + page: '/_sport/[slug]/test', + regex: normalizeRegEx('^\\/_sport\\/([^\\/]+?)\\/test(?:\\/)?$'), + routeKeys: { + slug: 'slug', + }, + }, { namedRegex: '^/another/(?[^/]+?)(?:/)?$', page: '/another/[id]',