Skip to content

Commit

Permalink
Fix: Router.query contains _next when using middleware with dynamic r…
Browse files Browse the repository at this point in the history
…outes (#48753)

Fixes: #43598

This was a tricky one to find! Not sure why more people aren't
complaining about this issue, was super annoying in my use case since
links had the wrong URL.

## What?
This issue only occurred when basepath was defined and middleware and
dynamic pages are being used.
Example from the reproduction repo mentioned in the issue tagged:
<img width="686" alt="Screenshot 2023-04-23 at 9 32 55 PM"
src="https://user-images.githubusercontent.com/11258286/233850968-e14f6b49-858b-410e-b8f9-93c90447090a.png">


## Why?
`nextConfig` wasn't passed to `getNextPathnameInfo` function, hence the
basePath wasn't removed from a intermediate variable and that trickled
down to cause this issue.

Added test case based on the issue reproduction repo

---------

Co-authored-by: Jimmy Lai <laijimmy0@gmail.com>
  • Loading branch information
darshkpatel and feedthejim committed May 9, 2023
1 parent 761c293 commit 81f5ed7
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 1 deletion.
7 changes: 6 additions & 1 deletion packages/next/src/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,12 @@ function getMiddlewareData<T extends FetchDataOutput>(
) {
const parsedSource = getNextPathnameInfo(
parseRelativeUrl(source).pathname,
{ parseData: true }
{
nextConfig: process.env.__NEXT_HAS_REWRITES
? undefined
: nextConfig,
parseData: true,
}
)

as = addBasePath(parsedSource.pathname)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { NextResponse } from 'next/server'

export default function middleware(_) {
const res = NextResponse.next()
res.headers.set('X-From-Middleware', 'true')
return res
}

export const config = { matcher: '/random' }
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
basePath: '/base',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import Link from 'next/link'
import { useRouter } from 'next/router'

const Index = ({ value }) => {
const router = useRouter()

return (
<section>
<h1>{value}</h1>
<p id="router-path">
<strong>{router.query.path}</strong>
</p>
<Link href="/another-page" id="linkelement">
Link to another page
</Link>
</section>
)
}

export async function getStaticPaths() {
return {
paths: [{ params: { path: 'another-page', pages: null } }],
fallback: true,
}
}

export async function getStaticProps() {
return {
props: {
value: 'Hello',
},
}
}

export default Index
55 changes: 55 additions & 0 deletions test/e2e/middleware-dynamic-basepath-matcher/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/* eslint-env jest */
/* eslint-disable jest/no-standalone-expect */

import { join } from 'path'
import webdriver from 'next-webdriver'
import { fetchViaHTTP } from 'next-test-utils'
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'

const itif = (condition: boolean) => (condition ? it : it.skip)

const isModeDeploy = process.env.NEXT_TEST_MODE === 'deploy'

describe('Middleware custom matchers basePath', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: new FileRef(join(__dirname, '../app')),
})
})
afterAll(() => next.destroy())

// FIXME
// See https://linear.app/vercel/issue/EC-170/middleware-rewrite-of-nextjs-with-basepath-does-not-work-on-vercel
itif(!isModeDeploy)('should match', async () => {
for (const path of [
'/base/default',
`/base/_next/data/${next.buildId}/default.json`,
]) {
const res = await fetchViaHTTP(next.url, path)
expect(res.status).toBe(200)
expect(res.headers.get('x-from-middleware')).toBeDefined()
}
})

it.each(['/default', '/invalid/base/default'])(
'should not match',
async (path) => {
const res = await fetchViaHTTP(next.url, path)
expect(res.status).toBe(404)
}
)

// FIXME:
// See https://linear.app/vercel/issue/EC-160/header-value-set-on-middleware-is-not-propagated-on-client-request-of
itif(!isModeDeploy)('should match query path', async () => {
const browser = await webdriver(next.url, '/base/random')
const currentPath = await browser.elementById('router-path').text()
expect(currentPath).toBe('random')
await browser.elementById('linkelement').click()
const anotherPagePath = await browser.elementById('router-path').text()
expect(anotherPagePath).toBe('another-page')
})
})

0 comments on commit 81f5ed7

Please sign in to comment.