Skip to content

Commit

Permalink
Deprecate nested middleware
Browse files Browse the repository at this point in the history
  • Loading branch information
javivelasco committed May 9, 2022
1 parent 84c3370 commit 3abe43f
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 39 deletions.
4 changes: 4 additions & 0 deletions errors/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
27 changes: 27 additions & 0 deletions errors/nested-middleware.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 15 additions & 4 deletions packages/next/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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`,
Expand Down Expand Up @@ -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),
Expand Down
23 changes: 13 additions & 10 deletions packages/next/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('|')})$`
)
Expand Down Expand Up @@ -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> = {}
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export function middleware() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function About() {
return (
<div>
<h1 className="title">About Page</h1>
</div>
)
}
80 changes: 57 additions & 23 deletions test/integration/middleware-module-errors/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
},
})
})
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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))
})

Expand All @@ -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', () => {
Expand Down Expand Up @@ -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'
)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 3abe43f

Please sign in to comment.