Skip to content

Commit

Permalink
Fix Edge SSR routes (#39594)
Browse files Browse the repository at this point in the history
Currently Edge SSR routes are added to both `routedPages` (catch-all page render routes) and `edgeRoutesSet` (catch-all edge function routes). This causes the request to be handled by both and results in an error (the Node server can't execute the Edge SSR route as a regular page). 

Another fix is to make sure Edge Function routes are sorted too, so `/foo` can be caught before dynamic ones like `/[id]`.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
  • Loading branch information
shuding committed Aug 14, 2022
1 parent 6791e75 commit e0d7ee0
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 22 deletions.
5 changes: 3 additions & 2 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -382,12 +382,13 @@ export default class DevServer extends Server {
page: pageName,
pageRuntime: staticInfo.runtime,
onClient: () => {},
onServer: () => {},
onServer: () => {
routedPages.push(pageName)
},
onEdgeServer: () => {
edgeRoutesSet.add(pageName)
},
})
routedPages.push(pageName)
}

if (envChange) {
Expand Down
34 changes: 14 additions & 20 deletions packages/next/server/next-server.ts
Expand Up @@ -84,10 +84,10 @@ import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-
import { getNextPathnameInfo } from '../shared/lib/router/utils/get-next-pathname-info'
import { bodyStreamToNodeStream, getClonableBody } from './body-streams'
import { checkIsManualRevalidate } from './api-utils'
import { isDynamicRoute } from '../shared/lib/router/utils'
import { shouldUseReactRoot } from './utils'
import ResponseCache from './response-cache'
import { IncrementalCache } from './lib/incremental-cache'
import { getSortedRoutes } from '../shared/lib/router/utils/sorted-routes'

if (shouldUseReactRoot) {
;(process.env as any).__NEXT_REACT_ROOT = 'true'
Expand Down Expand Up @@ -1151,10 +1151,13 @@ export default class NextNodeServer extends BaseServer {
return []
}

return Object.keys(manifest.functions).map((page) => ({
match: getMiddlewareMatcher(manifest.functions[page]),
page,
}))
// Make sure to sort function routes too.
return getSortedRoutes(Object.keys(manifest.functions)).map((page) => {
return {
match: getMiddlewareMatcher(manifest.functions[page]),
page,
}
})
}

protected getEdgeRoutes(): RoutingItem[] {
Expand Down Expand Up @@ -1371,22 +1374,13 @@ export default class NextNodeServer extends BaseServer {
const normalizedPathname = removeTrailingSlash(pathname || '')
let page = normalizedPathname
let params: Params | undefined = undefined
let pageFound = !isDynamicRoute(page)

if (this.dynamicRoutes) {
for (const dynamicRoute of this.dynamicRoutes) {
params = dynamicRoute.match(normalizedPathname) || undefined
if (params) {
page = dynamicRoute.page
pageFound = true
break
}
}
}

if (!pageFound) {
return {
finished: false,
for (const edgeFunction of edgeFunctions) {
const matched = edgeFunction.match(page)
if (matched) {
params = matched
page = edgeFunction.page
break
}
}

Expand Down
37 changes: 37 additions & 0 deletions test/e2e/switchable-runtime/index.test.ts
Expand Up @@ -70,6 +70,43 @@ describe('Switchable runtime', () => {
expect(devMiddlewareManifest).toEqual({})
})

it('should sort edge SSR routes correctly', async () => {
const res = await fetchViaHTTP(next.url, `/edge/foo`)
const html = await res.text()

// /edge/foo should be caught before /edge/[id]
expect(html).toContain(`to /edge/[id]`)
})

it('should be able to navigate between edge SSR routes without any errors', async () => {
const res = await fetchViaHTTP(next.url, `/edge/foo`)
const html = await res.text()

// /edge/foo should be caught before /edge/[id]
expect(html).toContain(`to /edge/[id]`)

const browser = await webdriver(context.appPort, '/edge/foo')

await browser.waitForElementByCss('a').click()

// on /edge/[id]
await check(
() => browser.eval('document.documentElement.innerHTML'),
/to \/edge\/foo/
)

await browser.waitForElementByCss('a').click()

// on /edge/foo
await check(
() => browser.eval('document.documentElement.innerHTML'),
/to \/edge\/\[id\]/
)

expect(context.stdout).not.toContain('self is not defined')
expect(context.stderr).not.toContain('self is not defined')
})

it.skip('should support client side navigation to ssr rsc pages', async () => {
let flightRequest = null

Expand Down
13 changes: 13 additions & 0 deletions test/e2e/switchable-runtime/pages/edge/[id].js
@@ -0,0 +1,13 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<Link href="/edge/foo">to /edge/foo</Link>
</div>
)
}

export const config = {
runtime: 'experimental-edge',
}
13 changes: 13 additions & 0 deletions test/e2e/switchable-runtime/pages/edge/foo.js
@@ -0,0 +1,13 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<Link href="/edge/123">to /edge/[id]</Link>
</div>
)
}

export const config = {
runtime: 'experimental-edge',
}

0 comments on commit e0d7ee0

Please sign in to comment.