From e0d7ee01df95aa07fbe42bf1a37e09228409a255 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Sun, 14 Aug 2022 23:51:11 +0200 Subject: [PATCH] Fix Edge SSR routes (#39594) 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) --- packages/next/server/dev/next-dev-server.ts | 5 ++- packages/next/server/next-server.ts | 34 +++++++---------- test/e2e/switchable-runtime/index.test.ts | 37 +++++++++++++++++++ .../e2e/switchable-runtime/pages/edge/[id].js | 13 +++++++ test/e2e/switchable-runtime/pages/edge/foo.js | 13 +++++++ 5 files changed, 80 insertions(+), 22 deletions(-) create mode 100644 test/e2e/switchable-runtime/pages/edge/[id].js create mode 100644 test/e2e/switchable-runtime/pages/edge/foo.js diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index 80a9d385bae27e3..a43f65c1ed3fa0a 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -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) { diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index 0eb6747391f5c03..b58ffafead4248b 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -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' @@ -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[] { @@ -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 } } diff --git a/test/e2e/switchable-runtime/index.test.ts b/test/e2e/switchable-runtime/index.test.ts index a1868f18d0ebdc5..7ec464455d62e87 100644 --- a/test/e2e/switchable-runtime/index.test.ts +++ b/test/e2e/switchable-runtime/index.test.ts @@ -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 diff --git a/test/e2e/switchable-runtime/pages/edge/[id].js b/test/e2e/switchable-runtime/pages/edge/[id].js new file mode 100644 index 000000000000000..8e91214e16c7b06 --- /dev/null +++ b/test/e2e/switchable-runtime/pages/edge/[id].js @@ -0,0 +1,13 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ to /edge/foo +
+ ) +} + +export const config = { + runtime: 'experimental-edge', +} diff --git a/test/e2e/switchable-runtime/pages/edge/foo.js b/test/e2e/switchable-runtime/pages/edge/foo.js new file mode 100644 index 000000000000000..b14bc44d881f3a6 --- /dev/null +++ b/test/e2e/switchable-runtime/pages/edge/foo.js @@ -0,0 +1,13 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ to /edge/[id] +
+ ) +} + +export const config = { + runtime: 'experimental-edge', +}