Skip to content

Commit

Permalink
Fix page url for edge routes in app dir (#40361)
Browse files Browse the repository at this point in the history
For edge routes in app dir, we passed the page name as url so that the
url will be incorrect:

* a non-dynamic route `/dashboard` will become `/dashboard/page`
* a dynamic route `/dynamic/[id]` with url `/dyanmic/123` will still hit
`/dynamic/page/[id]/page`

This PR uses normalized `params.page` with interoplated as correct
pathname for the page.
  • Loading branch information
huozhi committed Sep 8, 2022
1 parent a234abf commit 1858fa9
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 13 deletions.
23 changes: 10 additions & 13 deletions packages/next/server/next-server.ts
Expand Up @@ -763,7 +763,6 @@ export default class NextNodeServer extends BaseServer {
params,
page,
appPaths: null,
isAppPath: false,
})

if (handledAsEdgeFunction) {
Expand Down Expand Up @@ -913,7 +912,6 @@ export default class NextNodeServer extends BaseServer {
params: ctx.renderOpts.params,
page,
appPaths,
isAppPath,
})
return null
}
Expand Down Expand Up @@ -2031,12 +2029,12 @@ export default class NextNodeServer extends BaseServer {
params: Params | undefined
page: string
appPaths: string[] | null
isAppPath: boolean
onWarning?: (warning: Error) => void
}): Promise<FetchEventResult | null> {
let middlewareInfo: ReturnType<typeof this.getEdgeFunctionInfo> | undefined

const page = params.page
const { query, page } = params

await this.ensureEdgeFunction({ page, appPaths: params.appPaths })
middlewareInfo = this.getEdgeFunctionInfo({
page,
Expand All @@ -2048,29 +2046,28 @@ export default class NextNodeServer extends BaseServer {
}

// For middleware to "fetch" we must always provide an absolute URL
const isDataReq = !!params.query.__nextDataReq
const query = urlQueryToSearchParams(params.query).toString()
const locale = params.query.__nextLocale
// Use original pathname (without `/page`) instead of appPath for url
let normalizedPathname = params.page
const locale = query.__nextLocale
const isDataReq = !!query.__nextDataReq
const queryString = urlQueryToSearchParams(query).toString()

if (isDataReq) {
params.req.headers['x-nextjs-data'] = '1'
}

let normalizedPathname = normalizeAppPath(page)
if (isDynamicRoute(normalizedPathname)) {
const routeRegex = getNamedRouteRegex(params.page)
const routeRegex = getNamedRouteRegex(normalizedPathname)
normalizedPathname = interpolateDynamicPath(
params.page,
Object.assign({}, params.params, params.query),
normalizedPathname,
Object.assign({}, params.params, query),
routeRegex
)
}

const url = `${getRequestMeta(params.req, '_protocol')}://${
this.hostname
}:${this.port}${locale ? `/${locale}` : ''}${normalizedPathname}${
query ? `?${query}` : ''
queryString ? `?${queryString}` : ''
}`

if (!url.startsWith('http')) {
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/app-dir/rsc-basic.test.ts
Expand Up @@ -340,6 +340,20 @@ describe('app dir - react server components', () => {
expect(head).toMatch(/{color:(\s*)blue;?}/)
})

it('should stick to the url without trailing /page suffix', async () => {
const browser = await webdriver(next.url, '/edge/dynamic')
const indexUrl = await browser.url()

await browser.loadPage(`${next.url}/edge/dynamic/123`, {
disableCache: false,
beforePageLoad: null,
})

const dynamicRouteUrl = await browser.url()
expect(indexUrl).toBe(`${next.url}/edge/dynamic`)
expect(dynamicRouteUrl).toBe(`${next.url}/edge/dynamic/123`)
})

it('should support streaming for flight response', async () => {
await fetchViaHTTP(next.url, '/?__flight__=1').then(async (response) => {
const result = await resolveStreamResponse(response)
Expand Down
@@ -0,0 +1,7 @@
export default function page() {
return 'dynamic route [id] page'
}

export const config = {
runtime: 'experimental-edge',
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/rsc-basic/app/edge/dynamic/page.server.js
@@ -0,0 +1,7 @@
export default function page() {
return 'dynamic route index page'
}

export const config = {
runtime: 'experimental-edge',
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/rsc-basic/next.config.js
Expand Up @@ -7,4 +7,14 @@ module.exports = {
appDir: true,
serverComponents: true,
},
rewrites: async () => {
return {
afterFiles: [
{
source: '/rewritten-to-edge-dynamic',
destination: '/edge/dynamic',
},
],
}
},
}

0 comments on commit 1858fa9

Please sign in to comment.