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 navigating with middleware parses route params correctly #37704

Merged
merged 1 commit into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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