From 7de3cf5f8962f28ff667aedeb2331c4b0a33ee65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 16 Aug 2022 19:05:03 +0100 Subject: [PATCH] fix(next-server): Fix priority for edge routes (#39462) Fixes #39411 Bug Related issues linked using fixes #number 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 Co-authored-by: JJ Kasper --- packages/next/server/base-server.ts | 2 +- packages/next/server/dev/next-dev-server.ts | 1 + .../server/dev/on-demand-entry-handler.ts | 9 ++ packages/next/server/next-server.ts | 124 +++++++----------- packages/next/server/router.ts | 11 +- .../index.test.ts | 33 +++++ test/lib/next-test-utils.js | 1 + 7 files changed, 98 insertions(+), 83 deletions(-) create mode 100644 test/e2e/edge-vs.-non-edge-api-route-priority/index.test.ts diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index c4e247a4e5b5..d3e89af197c7 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -127,7 +127,7 @@ export interface BaseRequestHandler { ): Promise } -type RequestContext = { +export type RequestContext = { req: BaseNextRequest res: BaseNextResponse pathname: string diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index a43f65c1ed3f..5f538ced0a2e 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -386,6 +386,7 @@ export default class DevServer extends Server { routedPages.push(pageName) }, onEdgeServer: () => { + routedPages.push(pageName) edgeRoutesSet.add(pageName) }, }) diff --git a/packages/next/server/dev/on-demand-entry-handler.ts b/packages/next/server/dev/on-demand-entry-handler.ts index da43d221be0a..f3c820da209d 100644 --- a/packages/next/server/dev/on-demand-entry-handler.ts +++ b/packages/next/server/dev/on-demand-entry-handler.ts @@ -1,4 +1,5 @@ import type ws from 'ws' +import * as Log from '../../build/output/log' import type { webpack } from 'next/dist/compiled/webpack/webpack' import type { NextConfigComplete } from '../config-shared' import { EventEmitter } from 'events' @@ -514,6 +515,13 @@ export function onDemandEntryHandler({ return { async ensurePage(page: string, clientOnly: boolean): Promise { + const stalledTime = 60 + const stalledEnsureTimeout = setTimeout(() => { + Log.warn( + `Ensuring ${page} has taken longer than ${stalledTime}s, if this continues to stall this may be a bug` + ) + }, stalledTime * 1000) + const pagePathData = await findPagePathData( rootDir, pagesDir, @@ -630,6 +638,7 @@ export function onDemandEntryHandler({ invalidator.invalidate([...added.keys()]) await Promise.all(invalidatePromises) } + clearTimeout(stalledEnsureTimeout) }, onHMR(client: ws) { diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index 333bd82e1360..f0e869773d0c 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -73,6 +73,7 @@ import BaseServer, { prepareServerlessUrl, RoutingItem, NoFallbackError, + RequestContext, } from './base-server' import { getPagePath, requireFontManifest } from './require' import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path' @@ -95,7 +96,6 @@ import { checkIsManualRevalidate } from './api-utils' import { shouldUseReactRoot, isTargetLikeServerless } 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' @@ -673,16 +673,22 @@ export default class NextNodeServer extends BaseServer { page: string, builtPagePath: string ): Promise { - const handledAsEdgeFunction = await this.runEdgeFunction({ - req, - res, - query, - params, - page, - }) + const edgeFunctions = this.getEdgeFunctions() + + for (const item of edgeFunctions) { + if (item.page === page) { + const handledAsEdgeFunction = await this.runEdgeFunction({ + req, + res, + query, + params, + page, + }) - if (handledAsEdgeFunction) { - return true + if (handledAsEdgeFunction) { + return true + } + } } const pageModule = await require(builtPagePath) @@ -801,6 +807,28 @@ export default class NextNodeServer extends BaseServer { ) } + protected async renderPageComponent( + ctx: RequestContext, + bubbleNoFallback: boolean + ) { + const edgeFunctions = this.getEdgeFunctions() + + for (const item of edgeFunctions) { + if (item.page === ctx.pathname) { + await this.runEdgeFunction({ + req: ctx.req, + res: ctx.res, + query: ctx.query, + params: ctx.renderOpts.params, + page: ctx.pathname, + }) + return null + } + } + + return super.renderPageComponent(ctx, bubbleNoFallback) + } + protected async findPageComponents( pathname: string, query: NextParsedUrlQuery = {}, @@ -1452,20 +1480,16 @@ export default class NextNodeServer extends BaseServer { return manifest } - /** - * Return a list of middleware routing items. This method exists to be later - * overridden by the development server in order to use a different source - * to get the list. - */ + /** Returns the middleware routing item if there is one. */ protected getMiddleware(): RoutingItem | undefined { const manifest = this.getMiddlewareManifest() - const rootMiddleware = manifest?.middleware?.['/'] - if (!rootMiddleware) { + const middleware = manifest?.middleware?.['/'] + if (!middleware) { return } return { - match: getMiddlewareMatcher(rootMiddleware), + match: getMiddlewareMatcher(middleware), page: '/', } } @@ -1476,13 +1500,10 @@ export default class NextNodeServer extends BaseServer { return [] } - // Make sure to sort function routes too. - return getSortedRoutes(Object.keys(manifest.functions)).map((page) => { - return { - match: getMiddlewareMatcher(manifest.functions[page]), - page, - } - }) + return Object.keys(manifest.functions).map((page) => ({ + match: getMiddlewareMatcher(manifest.functions[page]), + page, + })) } /** @@ -1857,45 +1878,6 @@ export default class NextNodeServer extends BaseServer { routes.push(middlewareCatchAllRoute) } - if (this.getEdgeFunctions().length) { - const edgeCatchAllRoute: Route = { - match: getPathMatch('/:path*'), - type: 'route', - name: 'edge functions catchall', - fn: async (req, res, _params, parsed) => { - const edgeFunctions = this.getEdgeFunctions() - if (!edgeFunctions.length) return { finished: false } - - const { query, pathname } = parsed - const normalizedPathname = removeTrailingSlash(pathname || '') - let page = normalizedPathname - let params: Params | undefined = undefined - - for (const edgeFunction of edgeFunctions) { - const matched = edgeFunction.match(page) - if (matched) { - params = matched - page = edgeFunction.page - break - } - } - - const edgeSSRResult = await this.runEdgeFunction({ - req, - res, - query, - params, - page, - }) - - return { - finished: !!edgeSSRResult, - } - }, - } - - routes.push(edgeCatchAllRoute) - } } return routes @@ -1946,15 +1928,11 @@ export default class NextNodeServer extends BaseServer { }): Promise { let middlewareInfo: ReturnType | undefined - try { - await this.ensureEdgeFunction(params.page) - middlewareInfo = this.getEdgeFunctionInfo({ - page: params.page, - middleware: false, - }) - } catch { - return null - } + await this.ensureEdgeFunction(params.page) + middlewareInfo = this.getEdgeFunctionInfo({ + page: params.page, + middleware: false, + }) if (!middlewareInfo) { return null diff --git a/packages/next/server/router.ts b/packages/next/server/router.ts index f06b4443249a..0cb309f823af 100644 --- a/packages/next/server/router.ts +++ b/packages/next/server/router.ts @@ -220,8 +220,7 @@ export default class Router { - User rewrites (checking filesystem and pages each match) */ - const [middlewareCatchAllRoute, edgeSSRCatchAllRoute] = - this.catchAllMiddleware + const [middlewareCatchAllRoute] = this.catchAllMiddleware const allRoutes = [ ...(middlewareCatchAllRoute ? this.fsRoutes @@ -244,7 +243,6 @@ export default class Router { // disabled ...(this.useFileSystemPublicRoutes ? [ - ...(edgeSSRCatchAllRoute ? [edgeSSRCatchAllRoute] : []), { type: 'route', name: 'page checker', @@ -299,12 +297,7 @@ export default class Router { // We only check the catch-all route if public page routes hasn't been // disabled - ...(this.useFileSystemPublicRoutes - ? [ - ...(edgeSSRCatchAllRoute ? [edgeSSRCatchAllRoute] : []), - this.catchAllRoute, - ] - : []), + ...(this.useFileSystemPublicRoutes ? [this.catchAllRoute] : []), ] for (const testRoute of allRoutes) { diff --git a/test/e2e/edge-vs.-non-edge-api-route-priority/index.test.ts b/test/e2e/edge-vs.-non-edge-api-route-priority/index.test.ts new file mode 100644 index 000000000000..48fe0a3e92ba --- /dev/null +++ b/test/e2e/edge-vs.-non-edge-api-route-priority/index.test.ts @@ -0,0 +1,33 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { fetchViaHTTP } from 'next-test-utils' + +describe('Edge vs. non-Edge API route priority', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: { + 'pages/api/user/login.js': ` + export default async function handler(_, res) { + res.send('from login.js') + } + `, + 'pages/api/user/[id].js': ` + export const config = { + runtime: 'experimental-edge', + } + export default async function handler() { + return new Response('from [id].js') + }`, + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('more specific route should match', async () => { + const res = await fetchViaHTTP(next.url, '/api/user/login') + expect(await res.text()).toBe('from login.js') + }) +}) diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js index 1bcb877dac75..d46441e156f3 100644 --- a/test/lib/next-test-utils.js +++ b/test/lib/next-test-utils.js @@ -114,6 +114,7 @@ export function renderViaHTTP(appPort, pathname, query, opts) { return fetchViaHTTP(appPort, pathname, query, opts).then((res) => res.text()) } +/** @return {Promise} */ export function fetchViaHTTP(appPort, pathname, query, opts) { const url = `${pathname}${ typeof query === 'string' ? query : query ? `?${qs.stringify(query)}` : ''