From 7b1baceede9b473f634d8212a8f1c879c7a7ce1f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 2 Feb 2022 12:31:56 -0600 Subject: [PATCH] Ensure external beforeFiles rewrites are handled with next/link (#33888) This ensures we properly handle external `beforeFiles` rewrites client-side so that a different result doesn't occur client-side versus on a direct visit. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes: https://github.com/vercel/next.js/issues/32348 --- packages/next/shared/lib/router/router.ts | 9 +++++++ .../lib/router/utils/resolve-rewrites.ts | 6 ++++- test/integration/custom-routes/next.config.js | 4 ++++ test/integration/custom-routes/pages/nav.js | 4 ++++ .../custom-routes/pages/overridden.js | 3 +++ .../custom-routes/test/index.test.js | 24 +++++++++++++++++++ 6 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 test/integration/custom-routes/pages/overridden.js diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 58a7595b0b1209d..d828f29d9774359 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1104,6 +1104,11 @@ export default class Router implements BaseRouter { (p: string) => resolveDynamicRoute(p, pages), this.locales ) + + if (rewritesResult.externalDest) { + location.href = as + return true + } resolvedAs = rewritesResult.asPath if (rewritesResult.matchedPage && rewritesResult.resolvedHref) { @@ -1689,6 +1694,10 @@ export default class Router implements BaseRouter { (p: string) => resolveDynamicRoute(p, pages), this.locales ) + + if (rewritesResult.externalDest) { + return + } resolvedAs = delLocale(delBasePath(rewritesResult.asPath), this.locale) if (rewritesResult.matchedPage && rewritesResult.resolvedHref) { diff --git a/packages/next/shared/lib/router/utils/resolve-rewrites.ts b/packages/next/shared/lib/router/utils/resolve-rewrites.ts index 653a362ee0ba4b1..30fe1a35a4f56a8 100644 --- a/packages/next/shared/lib/router/utils/resolve-rewrites.ts +++ b/packages/next/shared/lib/router/utils/resolve-rewrites.ts @@ -25,8 +25,10 @@ export default function resolveRewrites( parsedAs: ReturnType asPath: string resolvedHref?: string + externalDest?: boolean } { let matchedPage = false + let externalDest = false let parsedAs = parseRelativeUrl(asPath) let fsPathname = removePathTrailingSlash( normalizeLocalePath(delBasePath(parsedAs.pathname), locales).pathname @@ -65,6 +67,7 @@ export default function resolveRewrites( if (params) { if (!rewrite.destination) { // this is a proxied rewrite which isn't handled on the client + externalDest = true return true } const destRes = prepareDestination({ @@ -103,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 - handleRewrite(rewrites.beforeFiles[i]) + finished = handleRewrite(rewrites.beforeFiles[i]) || false } matchedPage = pages.includes(fsPathname) @@ -139,5 +142,6 @@ export default function resolveRewrites( parsedAs, matchedPage, resolvedHref, + externalDest, } } diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index 61911b0cf5bc616..f9552353c24586e 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -220,6 +220,10 @@ module.exports = { source: '/old-blog/:path*', destination: '/blog/:path*', }, + { + source: '/overridden', + destination: 'https://example.com', + }, ], } }, diff --git a/test/integration/custom-routes/pages/nav.js b/test/integration/custom-routes/pages/nav.js index 2be4ac0870eac39..1b6663537c4b173 100644 --- a/test/integration/custom-routes/pages/nav.js +++ b/test/integration/custom-routes/pages/nav.js @@ -40,6 +40,10 @@ const Page = () => ( to /old-blog/post-1
+ + to /overridden + +
) export default Page diff --git a/test/integration/custom-routes/pages/overridden.js b/test/integration/custom-routes/pages/overridden.js new file mode 100644 index 000000000000000..68499a3d018f993 --- /dev/null +++ b/test/integration/custom-routes/pages/overridden.js @@ -0,0 +1,3 @@ +export default function Page() { + return

this page is overridden by a beforeFiles rewrite

+} diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index ac370167265cc75..59b19fcaf9d9cc6 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -39,6 +39,19 @@ let appPort let app const runTests = (isDev = false) => { + it('should handle external beforeFiles rewrite correctly', async () => { + const res = await fetchViaHTTP(appPort, '/overridden') + expect(res.status).toBe(200) + expect(await res.text()).toContain('Example Domain') + + const browser = await webdriver(appPort, '/nav') + await browser.elementByCss('#to-before-files-overridden').click() + await check( + () => browser.eval('document.documentElement.innerHTML'), + /Example Domain/ + ) + }) + it('should handle has query encoding correctly', async () => { for (const expected of [ { @@ -1697,6 +1710,11 @@ const runTests = (isDev = false) => { ), source: '/old-blog/:path*', }, + { + destination: 'https://example.com', + regex: normalizeRegEx('^\\/overridden(?:\\/)?$'), + source: '/overridden', + }, ], afterFiles: [ { @@ -2028,6 +2046,12 @@ const runTests = (isDev = false) => { regex: '^/nav(?:/)?$', routeKeys: {}, }, + { + namedRegex: '^/overridden(?:/)?$', + page: '/overridden', + regex: '^/overridden(?:/)?$', + routeKeys: {}, + }, { namedRegex: '^/redirect\\-override(?:/)?$', page: '/redirect-override',