Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure external beforeFiles rewrites are handled with next/link #33888

Merged
merged 2 commits into from Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion packages/next/shared/lib/router/utils/resolve-rewrites.ts
Expand Up @@ -25,8 +25,10 @@ export default function resolveRewrites(
parsedAs: ReturnType<typeof parseRelativeUrl>
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
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -139,5 +142,6 @@ export default function resolveRewrites(
parsedAs,
matchedPage,
resolvedHref,
externalDest,
}
}
4 changes: 4 additions & 0 deletions test/integration/custom-routes/next.config.js
Expand Up @@ -220,6 +220,10 @@ module.exports = {
source: '/old-blog/:path*',
destination: '/blog/:path*',
},
{
source: '/overridden',
destination: 'https://example.com',
},
],
}
},
Expand Down
4 changes: 4 additions & 0 deletions test/integration/custom-routes/pages/nav.js
Expand Up @@ -40,6 +40,10 @@ const Page = () => (
<a id="to-old-blog">to /old-blog/post-1</a>
</Link>
<br />
<Link href="/overridden">
<a id="to-before-files-overridden">to /overridden</a>
</Link>
<br />
</>
)
export default Page
3 changes: 3 additions & 0 deletions test/integration/custom-routes/pages/overridden.js
@@ -0,0 +1,3 @@
export default function Page() {
return <p>this page is overridden by a beforeFiles rewrite</p>
}
24 changes: 24 additions & 0 deletions test/integration/custom-routes/test/index.test.js
Expand Up @@ -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 [
{
Expand Down Expand Up @@ -1697,6 +1710,11 @@ const runTests = (isDev = false) => {
),
source: '/old-blog/:path*',
},
{
destination: 'https://example.com',
regex: normalizeRegEx('^\\/overridden(?:\\/)?$'),
source: '/overridden',
},
],
afterFiles: [
{
Expand Down Expand Up @@ -2028,6 +2046,12 @@ const runTests = (isDev = false) => {
regex: '^/nav(?:/)?$',
routeKeys: {},
},
{
namedRegex: '^/overridden(?:/)?$',
page: '/overridden',
regex: '^/overridden(?:/)?$',
routeKeys: {},
},
{
namedRegex: '^/redirect\\-override(?:/)?$',
page: '/redirect-override',
Expand Down