Deprecate nested middleware

Author: javivelasco

errors/manifest.json

@@ -626,6 +626,10 @@
           "title": "middleware-relative-urls",
           "path": "/errors/middleware-relative-urls.md"
         },
+        {
+          "title": "nested-middleware",
+          "path": "/errors/nested-middleware.md"
+        },
         {
           "title": "deleting-query-params-in-middlewares",
           "path": "/errors/deleting-query-params-in-middlewares.md"

errors/nested-middleware.md

@@ -0,0 +1,27 @@
+# Nested Middleware Deprecated
+
+#### Why This Error Occurred
+
+You are defining a middleware file in a location different from `pages/_middleware` which is _deprecated_.
+
+Declaring a middleware file under specific pages brought the illusion that a middleware would be executed _only_ when pages below its declaration were matched but this wasn't the case.
+Supporting this API also comes with other consequences like allowing to nest multiple middleware or dragging effects between different middleware executions.
+
+This makes the execution model for middleware **really complex** and hard to reason about so this API is _deprecated_ in favour of a simpler model with a single root middleware.
+
+#### Possible Ways to Fix It
+
+To fix this error you must declare your middleware in the root pages folder and leverage `NextRequest` parsed URL to decide wether or not to execute the middleware code.
+For example, a middleware declared under `pages/about/_middleware.js` can be moved to `pages/_middleware` and updated so that it only executes when `about/*` matches:
+
+```typescript
+import type { NextRequest } from 'next/server'
+
+export function middleware(request: NextRequest) {
+  if (request.nextUrl.pathname.startsWith('/about')) {
+    // Execute pages/about/_middleware.js
+  }
+}
+```
+
+This also means that if you have more than one middleware you will need to reunite them in a single file and model their execution depending on the request.

packages/next/build/entries.ts

@@ -102,10 +102,6 @@ export function createPagesMapping({
     {}
   )
 
-  // In development we always alias these to allow Webpack to fallback to
-  // the correct source file so that HMR can work properly when a file is
-  // added or removed.
-
   if (isViews) {
     return pages
   }
@@ -117,7 +113,11 @@ export function createPagesMapping({
     delete pages['/_document']
   }
 
+  // In development we always alias these to allow Webpack to fallback to
+  // the correct source file so that HMR can work properly when a file is
+  // added or removed.
   const root = isDev ? PAGES_DIR_ALIAS : 'next/dist/pages'
+
   return {
     '/_app': `${root}/_app`,
     '/_error': `${root}/_error`,
@@ -396,6 +396,17 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
         return require.resolve(absolutePagePath)
       })()
 
+      /**
+       * When we find a middleware that is declared in the pages/ root we fail.
+       * There is no need to check on `dev` as this should only happen when
+       * building for production.
+       */
+      if (/[\\\\/]_middleware$/.test(page) && page !== '/_middleware') {
+        throw new Error(
+          `nested Middleware is deprecated (found pages${page}) - https://nextjs.org/docs/messages/nested-middleware`
+        )
+      }
+
       runDependingOnPageType({
         page,
         pageRuntime: await getPageRuntime(pageFilePath, config, isDev),

packages/next/server/dev/next-dev-server.ts

@@ -249,10 +249,6 @@ export default class DevServer extends Server {
       return
     }
 
-    const regexMiddleware = new RegExp(
-      `[\\\\/](_middleware.(?:${this.nextConfig.pageExtensions.join('|')}))$`
-    )
-
     const regexPageExtension = new RegExp(
       `\\.+(?:${this.nextConfig.pageExtensions.join('|')})$`
     )
@@ -280,7 +276,7 @@ export default class DevServer extends Server {
       wp.watch([], toWatch, 0)
 
       wp.on('aggregated', async () => {
-        const routedMiddleware = []
+        const routedMiddleware: string[] = []
         const routedPages: string[] = []
         const knownFiles = wp.getTimeInfoEntries()
         const viewPaths: Record<string, string> = {}
@@ -318,11 +314,18 @@ export default class DevServer extends Server {
             pageName = pageName.replace(/\/index$/, '') || '/'
           }
 
-          if (regexMiddleware.test(fileName)) {
-            routedMiddleware.push(
-              `/${relative(this.pagesDir, fileName).replace(/\\+/g, '/')}`
-                .replace(/^\/+/g, '/')
-                .replace(regexMiddleware, '/')
+          if (pageName === '/_middleware') {
+            routedMiddleware.push(`/`)
+            continue
+          }
+
+          /**
+           * If there is a middleware that is not declared in the root we will
+           * warn without adding it so it doesn't make its way into the system.
+           */
+          if (/[\\\\/]_middleware$/.test(pageName)) {
+            Log.warn(
+              `nested Middleware is deprecated (found pages${pageName}) - https://nextjs.org/docs/messages/nested-middleware`
             )
             continue
           }

test/integration/middleware-module-errors/pages/about/_middleware.js

@@ -0,0 +1 @@
+export function middleware() {}

test/integration/middleware-module-errors/pages/about/index.js

@@ -0,0 +1,7 @@
+export default function About() {
+  return (
+    <div>
+      <h1 className="title">About Page</h1>
+    </div>
+  )
+}

test/integration/middleware-module-errors/test/index.test.js

@@ -13,13 +13,16 @@ import {
   waitFor,
 } from 'next-test-utils'
 
-const context = {}
-const WEBPACK_BREAKING_CHANGE = 'BREAKING CHANGE:'
-
 jest.setTimeout(1000 * 60 * 2)
-context.appDir = join(__dirname, '../')
-context.middleware = new File(join(__dirname, '../pages/_middleware.js'))
-context.page = new File(join(__dirname, '../pages/index.js'))
+
+const WEBPACK_BREAKING_CHANGE = 'BREAKING CHANGE:'
+const context = {
+  appDir: join(__dirname, '../'),
+  buildLogs: { output: '', stdout: '', stderr: '' },
+  logs: { output: '', stdout: '', stderr: '' },
+  middleware: new File(join(__dirname, '../pages/_middleware.js')),
+  page: new File(join(__dirname, '../pages/index.js')),
+}
 
 describe('Middleware importing Node.js modules', () => {
   function getModuleNotFound(name) {
@@ -39,20 +42,20 @@ describe('Middleware importing Node.js modules', () => {
   })
 
   describe('dev mode', () => {
-    let output = ''
-
     // restart the app for every test since the latest error is not shown sometimes
     // See https://github.com/vercel/next.js/issues/36575
     beforeEach(async () => {
-      output = ''
+      context.logs = { output: '', stdout: '', stderr: '' }
       context.appPort = await findPort()
       context.app = await launchApp(context.appDir, context.appPort, {
         env: { __NEXT_TEST_WITH_DEVTOOL: 1 },
         onStdout(msg) {
-          output += msg
+          context.logs.output += msg
+          context.logs.stdout += msg
         },
         onStderr(msg) {
-          output += msg
+          context.logs.output += msg
+          context.logs.stderr += msg
         },
       })
     })
@@ -72,11 +75,13 @@ describe('Middleware importing Node.js modules', () => {
       await waitFor(500)
       const msg = getNodeBuiltinModuleNotSupportedInEdgeRuntimeMessage('path')
       expect(res.status).toBe(500)
-      expect(output).toContain(getModuleNotFound('path'))
-      expect(output).toContain(msg)
+      expect(context.logs.output).toContain(getModuleNotFound('path'))
+      expect(context.logs.output).toContain(msg)
       expect(text).toContain(escapeLF(msg))
-      expect(stripAnsi(output)).toContain("import { basename } from 'path'")
-      expect(output).not.toContain(WEBPACK_BREAKING_CHANGE)
+      expect(stripAnsi(context.logs.output)).toContain(
+        "import { basename } from 'path'"
+      )
+      expect(context.logs.output).not.toContain(WEBPACK_BREAKING_CHANGE)
     })
 
     it('shows the right error when importing `child_process` on middleware', async () => {
@@ -95,13 +100,13 @@ describe('Middleware importing Node.js modules', () => {
       const msg =
         getNodeBuiltinModuleNotSupportedInEdgeRuntimeMessage('child_process')
       expect(res.status).toBe(500)
-      expect(output).toContain(getModuleNotFound('child_process'))
-      expect(output).toContain(msg)
+      expect(context.logs.output).toContain(getModuleNotFound('child_process'))
+      expect(context.logs.output).toContain(msg)
       expect(text).toContain(escapeLF(msg))
-      expect(stripAnsi(output)).toContain(
+      expect(stripAnsi(context.logs.output)).toContain(
         "import { spawn } from 'child_process'"
       )
-      expect(output).not.toContain(WEBPACK_BREAKING_CHANGE)
+      expect(context.logs.output).not.toContain(WEBPACK_BREAKING_CHANGE)
     })
 
     it('shows the right error when importing a non-node-builtin module on middleware', async () => {
@@ -121,8 +126,8 @@ describe('Middleware importing Node.js modules', () => {
       await waitFor(500)
       const msg =
         getNodeBuiltinModuleNotSupportedInEdgeRuntimeMessage('not-exist')
-      expect(output).toContain(getModuleNotFound('not-exist'))
-      expect(output).not.toContain(msg)
+      expect(context.logs.output).toContain(getModuleNotFound('not-exist'))
+      expect(context.logs.output).not.toContain(msg)
       expect(text).not.toContain(escapeLF(msg))
     })
 
@@ -146,10 +151,25 @@ describe('Middleware importing Node.js modules', () => {
       await waitFor(500)
       const msg =
         getNodeBuiltinModuleNotSupportedInEdgeRuntimeMessage('child_process')
-      expect(output).toContain(getModuleNotFound('child_process'))
-      expect(output).not.toContain(msg)
+      expect(context.logs.output).toContain(getModuleNotFound('child_process'))
+      expect(context.logs.output).not.toContain(msg)
       expect(text).not.toContain(escapeLF(msg))
     })
+
+    it('warns about nested middleware being deprecated', async () => {
+      const file = new File(join(__dirname, '../pages/about/_middleware.js'))
+      file.write(`export function middleware() {}`)
+      try {
+        const res = await fetchViaHTTP(context.appPort, '/about')
+        console.log(context.logs.stderr)
+        expect(context.logs.stderr).toContain(
+          'nested Middleware is deprecated (found pages/about/_middleware) - https://nextjs.org/docs/messages/nested-middleware'
+        )
+        expect(res.status).toBe(200)
+      } finally {
+        file.delete()
+      }
+    })
   })
 
   describe('production mode', () => {
@@ -194,5 +214,19 @@ describe('Middleware importing Node.js modules', () => {
         getNodeBuiltinModuleNotSupportedInEdgeRuntimeMessage('child_process')
       )
     })
+
+    it('fails when there is a deprecated middleware', async () => {
+      const file = new File(join(__dirname, '../pages/about/_middleware.js'))
+      file.write(`export function middleware() {}`)
+      const buildResult = await nextBuild(context.appDir, undefined, {
+        stderr: true,
+        stdout: true,
+      })
+      console.log(buildResult.stdout)
+
+      expect(buildResult.stderr).toContain(
+        'nested Middleware is deprecated (found pages/about/_middleware) - https://nextjs.org/docs/messages/nested-middleware'
+      )
+    })
   })
 })

test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts

@@ -26,7 +26,7 @@ describe('dependencies can use env vars in middlewares', () => {
         `,
 
         // The actual middleware code
-        'pages/api/_middleware.js': `
+        'pages/_middleware.js': `
           import customPackage from 'my-custom-package';
           export default function middleware(_req) {
             return new Response(JSON.stringify({
@@ -59,7 +59,7 @@ describe('dependencies can use env vars in middlewares', () => {
       '.next/server/middleware-manifest.json'
     )
     const manifest = await readJson(manifestPath)
-    const envVars = manifest?.middleware?.['/api']?.env
+    const envVars = manifest?.middleware?.['/']?.env
 
     expect(envVars).toHaveLength(2)
     expect(envVars).toContain('ENV_VAR_USED_IN_MIDDLEWARE')