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 <jj@jjsweb.site>
vercel · Aug 16, 2022 · 7de3cf5 · 7de3cf5
1 parent 57b6eff
commit 7de3cf5
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 83 deletions.
diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -127,7 +127,7 @@ export interface BaseRequestHandler {
   ): Promise<void>
 }
 
-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
@@ -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
@@ -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<void> {
+      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
@@ -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<boolean> {
-    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<FetchEventResult | null> {
     let middlewareInfo: ReturnType<typeof this.getEdgeFunctionInfo> | 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
@@ -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
@@ -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
@@ -114,6 +114,7 @@ export function renderViaHTTP(appPort, pathname, query, opts) {
   return fetchViaHTTP(appPort, pathname, query, opts).then((res) => res.text())
 }
 
+/** @return {Promise<Response & {buffer: any} & {headers: any}>} */
 export function fetchViaHTTP(appPort, pathname, query, opts) {
   const url = `${pathname}${
     typeof query === 'string' ? query : query ? `?${qs.stringify(query)}` : ''