Skip to content

Commit

Permalink
Ensure navigating with middleware parses route params correctly (#37704)
Browse files Browse the repository at this point in the history
  • Loading branch information
ijjk committed Jun 15, 2022
1 parent 007d186 commit bf7bf82
Show file tree
Hide file tree
Showing 8 changed files with 213 additions and 17 deletions.
5 changes: 1 addition & 4 deletions packages/next/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1239,10 +1239,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
`${query.__nextLocale ? `/${query.__nextLocale}` : ''}${pathname}`
)
// return empty JSON when not an SSG/SSP page and not an error
if (
!(isSSG || hasServerProps) &&
(!res.statusCode || res.statusCode === 200 || res.statusCode === 404)
) {
if (!(isSSG || hasServerProps)) {
res.setHeader('content-type', 'application/json')
res.body('{}')
res.send()
Expand Down
9 changes: 9 additions & 0 deletions packages/next/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,15 @@ export default class NextNodeServer extends BaseServer {
for (const [key, value] of Object.entries(
toNodeHeaders(result.response.headers)
)) {
if (
[
'x-middleware-rewrite',
'x-middleware-redirect',
'x-middleware-refresh',
].includes(key)
) {
continue
}
if (key !== 'content-encoding' && value !== undefined) {
res.setHeader(key, value)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/next/server/web/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export async function adapter(params: {
*/
if (isDataReq) {
response.headers.set(
'x-nextjs-matched-path',
'x-nextjs-rewrite',
relativizeURL(String(rewriteUrl), String(requestUrl))
)
}
Expand Down
87 changes: 75 additions & 12 deletions packages/next/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ export type CompletePrivateRouteInfo = {
err?: Error
error?: any
route?: string
resolvedAs?: string
}

export type AppProps = Pick<CompletePrivateRouteInfo, 'Component' | 'err'> & {
Expand Down Expand Up @@ -1234,8 +1235,32 @@ export default class Router implements BaseRouter {
isPreview: nextState.isPreview,
})

if ('route' in routeInfo) {
if ('route' in routeInfo && routeInfo.route !== route) {
pathname = routeInfo.route || route

if (isDynamicRoute(pathname)) {
const prefixedAs =
routeInfo.resolvedAs ||
addBasePath(addLocale(as, nextState.locale), true)

let rewriteAs = prefixedAs

if (hasBasePath(rewriteAs)) {
rewriteAs = removeBasePath(rewriteAs)
}

if (process.env.__NEXT_I18N_SUPPORT) {
const localeResult = normalizeLocalePath(rewriteAs, this.locales)
nextState.locale = localeResult.detectedLocale || nextState.locale
rewriteAs = localeResult.pathname
}
const routeRegex = getRouteRegex(pathname)
const routeMatch = getRouteMatcher(routeRegex)(rewriteAs)

if (routeMatch) {
Object.assign(query, routeMatch)
}
}
}

// If the routeInfo brings a redirect we simply apply it.
Expand Down Expand Up @@ -1703,6 +1728,7 @@ export default class Router implements BaseRouter {

routeInfo.props = props
routeInfo.route = route
routeInfo.resolvedAs = resolvedAs
this.components[route] = routeInfo
return routeInfo
} catch (err) {
Expand Down Expand Up @@ -2127,9 +2153,9 @@ function getMiddlewareData<T extends FetchDataOutput>(
trailingSlash: Boolean(process.env.__NEXT_TRAILING_SLASH),
}

// TODO: ensure x-nextjs-matched-path is always present instead of both
// variants
let rewriteTarget = response.headers.get('x-nextjs-matched-path')
let rewriteTarget =
response.headers.get('x-nextjs-rewrite') ||
response.headers.get('x-nextjs-matched-path')

const matchedPath = response.headers.get('x-matched-path')

Expand All @@ -2145,17 +2171,54 @@ function getMiddlewareData<T extends FetchDataOutput>(
parseData: true,
})

parsedRewriteTarget.pathname = pathnameInfo.pathname
const fsPathname = removeTrailingSlash(pathnameInfo.pathname)
return Promise.resolve(options.router.pageLoader.getPageList()).then(
(pages) => ({
return Promise.all([
options.router.pageLoader.getPageList(),
getClientBuildManifest(),
]).then(([pages, { __rewrites: rewrites }]: any) => {
let as = parsedRewriteTarget.pathname

if (isDynamicRoute(as)) {
const parsedSource = getNextPathnameInfo(
parseRelativeUrl(source).pathname,
{ parseData: true }
)

as = addBasePath(parsedSource.pathname)
}

if (process.env.__NEXT_HAS_REWRITES) {
const result = resolveRewrites(
as,
pages,
rewrites,
parsedRewriteTarget.query,
(path: string) => resolveDynamicRoute(path, pages),
options.router.locales
)

if (result.matchedPage) {
parsedRewriteTarget.pathname = result.parsedAs.pathname
parsedRewriteTarget.query = result.parsedAs.query
}
}

const resolvedHref = !pages.includes(fsPathname)
? resolveDynamicRoute(
normalizeLocalePath(
removeBasePath(parsedRewriteTarget.pathname),
options.router.locales
).pathname,
pages
)
: fsPathname

return {
type: 'rewrite' as const,
parsedAs: parsedRewriteTarget,
resolvedHref: !pages.includes(fsPathname)
? resolveDynamicRoute(fsPathname, pages)
: fsPathname,
})
)
resolvedHref,
}
})
}

const src = parsePath(source)
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/middleware-general/app/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ export async function middleware(request) {
}
}

if (url.pathname === '/rewrite-to-dynamic') {
url.pathname = '/blog/from-middleware'
return NextResponse.rewrite(url)
}

if (url.pathname === '/rewrite-to-config-rewrite') {
url.pathname = '/rewrite-3'
return NextResponse.rewrite(url)
}

if (url.pathname.startsWith('/fetch-user-agent-crypto')) {
try {
const apiRoute = new URL(url)
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/middleware-general/app/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ module.exports = {
source: '/rewrite-2',
destination: '/about/a',
},
{
source: '/rewrite-3',
destination: '/blog/middleware-rewrite',
},
]
},
}
23 changes: 23 additions & 0 deletions test/e2e/middleware-general/app/pages/blog/[slug].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { useRouter } from 'next/router'

export default function Page(props) {
const router = useRouter()
return (
<>
<p id="blog">/blog/[slug]</p>
<p id="query">{JSON.stringify(router.query)}</p>
<p id="pathname">{router.pathname}</p>
<p id="as-path">{router.asPath}</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

export function getServerSideProps({ params }) {
return {
props: {
now: Date.now(),
params,
},
}
}
90 changes: 90 additions & 0 deletions test/e2e/middleware-general/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,96 @@ describe('Middleware Runtime', () => {
})
}

it('should have correct dynamic route params on client-transition to dynamic route', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.beforeNav = 1')
await browser.eval('window.next.router.push("/blog/first")')
await browser.waitForElementByCss('#blog')

expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'first',
})
expect(
JSON.parse(await browser.elementByCss('#props').text()).params
).toEqual({
slug: 'first',
})
expect(await browser.elementByCss('#pathname').text()).toBe('/blog/[slug]')
expect(await browser.elementByCss('#as-path').text()).toBe('/blog/first')

await browser.eval('window.next.router.push("/blog/second")')
await check(() => browser.elementByCss('body').text(), /"slug":"second"/)

expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'second',
})
expect(
JSON.parse(await browser.elementByCss('#props').text()).params
).toEqual({
slug: 'second',
})
expect(await browser.elementByCss('#pathname').text()).toBe('/blog/[slug]')
expect(await browser.elementByCss('#as-path').text()).toBe('/blog/second')
})

it('should have correct dynamic route params for middleware rewrite to dynamic route', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.beforeNav = 1')
await browser.eval('window.next.router.push("/rewrite-to-dynamic")')
await browser.waitForElementByCss('#blog')

expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'from-middleware',
})
expect(
JSON.parse(await browser.elementByCss('#props').text()).params
).toEqual({
slug: 'from-middleware',
})
expect(await browser.elementByCss('#pathname').text()).toBe('/blog/[slug]')
expect(await browser.elementByCss('#as-path').text()).toBe(
'/rewrite-to-dynamic'
)
})

it('should have correct route params for chained rewrite from middleware to config rewrite', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.beforeNav = 1')
await browser.eval('window.next.router.push("/rewrite-to-config-rewrite")')
await browser.waitForElementByCss('#blog')

expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'middleware-rewrite',
})
expect(
JSON.parse(await browser.elementByCss('#props').text()).params
).toEqual({
slug: 'middleware-rewrite',
})
expect(await browser.elementByCss('#pathname').text()).toBe('/blog/[slug]')
expect(await browser.elementByCss('#as-path').text()).toBe(
'/rewrite-to-config-rewrite'
)
})

it('should have correct route params for rewrite from config dynamic route', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.beforeNav = 1')
await browser.eval('window.next.router.push("/rewrite-3")')
await browser.waitForElementByCss('#blog')

expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
slug: 'middleware-rewrite',
})
expect(
JSON.parse(await browser.elementByCss('#props').text()).params
).toEqual({
slug: 'middleware-rewrite',
})
expect(await browser.elementByCss('#pathname').text()).toBe('/blog/[slug]')
expect(await browser.elementByCss('#as-path').text()).toBe('/rewrite-3')
})

it('should redirect the same for direct visit and client-transition', async () => {
const res = await fetchViaHTTP(
next.url,
Expand Down

0 comments on commit bf7bf82

Please sign in to comment.