rsnext/packages/next/build/webpack/loaders/next-middleware-loader.ts

63 lines
1.9 KiB
TypeScript
Raw Normal View History

import type { MiddlewareMatcher } from '../../analysis/get-page-static-info'
Refactor Page Paths utils and Middleware Plugin (#36576) This PR brings some significant refactoring in preparation for upcoming middleware changes. Each commit can be reviewed independently, here is a summary of what each one does and the reasoning behind it: - [Move pagesDir to next-dev-server](https://github.com/javivelasco/next.js/pull/12/commits/f2fe154c007379f71c14960ddc553eaaaf786ffa) simply moves the `pagesDir` property to the dev server which is the only place where it is needed. Having it for every server is misleading. - [Move (de)normalize page path utils to a file page-path-utils.ts](https://github.com/javivelasco/next.js/pull/12/commits/27cedf087187b9632ef82a34b3af9cc4fe05d98b) Moves the functions to normalize and denormalize page paths to a single file that is intended to hold every utility function that transforms page paths. Since those are complementary it makes sense to have them together. I also added explanatory comments on why they are not idempotent and examples for input -> output that I find very useful. - [Extract removePagePathTail](https://github.com/javivelasco/next.js/pull/12/commits/6b121332aa9d3e50bd0f28b691fb7faea1b95f51) This extracts a function to remove the tail on a page path (absolute or relative). I'm sure there will be other contexts where we can use it. - [Extract getPagePaths and refactor findPageFile](https://github.com/javivelasco/next.js/pull/12/commits/cf2c7b842eebd8c02f23e79345681a794516b646) This extracts a function `getPagePaths` that is used to generate an array of paths to inspect when looking for a page file from `findPageFile`. Then it refactors such function to use it parallelizing lookups. This will allow us to print every path we look at when looking for a file which can be useful for debugging. It also adds a `flatten` helper. - [Refactor onDemandEntryHandler](https://github.com/javivelasco/next.js/pull/12/commits/4be685c37e3d1b797e929ea4f31495ed7b00e1cc) I've found this one quite difficult to understand so it is refactored to use some of the previously mentioned functions and make it easier to read. - [Extract absolutePagePath util](https://github.com/javivelasco/next.js/pull/12/commits/3bc078347426c73491a076d54ef4de977d9da073) Extracts yet another util from the `next-dev-server` that transforms an absolute path into a page name. Of course it adds comments, parameters and examples. - [Refactor MiddlewarePlugin](https://github.com/javivelasco/next.js/pull/12/commits/c595a2cc629b358cc61861a8a4848b7890d0a15b) This is the most significant change. The logic here was very hard to understand so it is totally redistributed with comments. This also removes a global variable `ssrEntries` that was deprecated in favour of module metadata added to Webpack from loaders keeping less dependencies. It also adds types and makes a clear distinction between phases where we statically analyze the code, find metadata and generate the manifest file cc @shuding @huozhi EDIT: - [Split page path utils](https://github.com/vercel/next.js/pull/36576/commits/158fb002d02887d7ce4be6747cf550a825a426eb) After seeing one of the utils was being used by the client while it was defined originally in the server, with this PR we are splitting the util into multiple files and moving it to `shared/lib` in order to make explicit that those can be also imported from client.
2022-04-30 13:19:27 +02:00
import { getModuleBuildInfo } from './get-module-build-info'
import { stringifyRequest } from '../stringify-request'
import { MIDDLEWARE_LOCATION_REGEXP } from '../../../lib/constants'
export type MiddlewareLoaderOptions = {
absolutePagePath: string
page: string
feat(edge): allows configuring Dynamic code execution guard (#39539) ### 📖 What's in there? Dynamic code evaluation (`eval()`, `new Function()`, ...) is not supported on the edge runtime, hence why we fail the build when detecting such statement in the middleware or `experimental-edge` routes at build time. However, there could be false positives, which static analysis and tree-shaking can not exclude: - `qs` through these dependencies (get-intrinsic: [source](https://github.com/ljharb/get-intrinsic/blob/main/index.js#L12)) - `function-bind` ([source](https://github.com/Raynos/function-bind/blob/master/implementation.js#L42)) - `has` ([source](https://github.com/tarruda/has/blob/master/src/index.js#L5)) This PR leverages the existing `config` export to let user allow some of their files. it’s meant for allowing users to import 3rd party modules who embed dynamic code evaluation, but do not use it (because or code paths), and can't be tree-shaked. By default, it’s keeping the existing behavior: warn in dev, fails to build. If users allow dynamic code, and that code is reached at runtime, their app stills breaks. ### 🧪 How to test? - (existing) integration tests for disallowing dynamic code evaluation: `pnpm testheadless --testPathPattern=runtime-dynamic` - (new) integration tests for allowing dynamic code evaluation: `pnpm testheadless --testPathPattern=runtime-configurable` - (amended) production tests for validating the new configuration keys: `pnpm testheadless --testPathPattern=config-validations` To try it live, you could have an application such as: ```js // lib/index.js /* eslint-disable no-eval */ export function hasUnusedDynamic() { if ((() => false)()) { eval('100') } } export function hasDynamic() { eval('100') } // pages/index.jsx export default function Page({ edgeRoute }) { return <p>{edgeRoute}</p> } export const getServerSideProps = async (req) => { const res = await fetch(`http://localhost:3000/api/route`) const data = await res.json() return { props: { edgeRoute: data.ok ? `Hi from the edge route` : '' } } } // pages/api/route.js import { hasDynamic } from '../../lib' export default async function handle() { hasDynamic() return Response.json({ ok: true }) } export const config = { runtime: 'experimental-edge' , allowDynamic: '/lib/**' } ``` Playing with `config.allowDynamic`, you should be able to: - build the app even if it uses `eval()` (it will obviously fail at runtime) - build the app that _imports but does not use_ `eval()` - run the app in dev, even if it uses `eval()` with no warning ### 🆙 Notes to reviewers Before adding documentation and telemetry, I'd like to collect comments on a couple of points: - the overall design for this feature: is a list of globs useful and easy enough? - should the globs be relative to the application root (current implementation) to to the edge route/middleware file? - (especially to @sokra) is the implementation idiomatic enough? I've leverage loaders to read the _entry point_ configuration once, then the ModuleGraph to get it back during the parsing phase. I couldn't re-use the existing `getExtractMetadata()` facility since it's happening late after the parsing. - there's a glitch with `import { ServerRuntime } from '../../types'` in `get-page-static-info.ts` ([here](https://github.com/vercel/next.js/pull/39539/files#diff-cb7ac6392c3dd707c5edab159c3144ec114eafea92dad5d98f4eedfc612174d2L12)). I had to use `next/types` because it was failing during lint. Any clue why? ### ☑️ Checklist - [ ] 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` - [x] Integration tests added - [x] Documentation added - [x] Telemetry added. In case of a feature if it's used or not. - [x] Errors have helpful link attached, see `contributing.md`
2022-09-13 00:01:00 +02:00
rootDir: string
matchers?: string
}
// matchers can have special characters that break the loader params
// parsing so we base64 encode/decode the string
export function encodeMatchers(matchers: MiddlewareMatcher[]) {
return Buffer.from(JSON.stringify(matchers)).toString('base64')
}
export function decodeMatchers(encodedMatchers: string) {
return JSON.parse(
Buffer.from(encodedMatchers, 'base64').toString()
) as MiddlewareMatcher[]
}
export default function middlewareLoader(this: any) {
const {
absolutePagePath,
page,
feat(edge): allows configuring Dynamic code execution guard (#39539) ### 📖 What's in there? Dynamic code evaluation (`eval()`, `new Function()`, ...) is not supported on the edge runtime, hence why we fail the build when detecting such statement in the middleware or `experimental-edge` routes at build time. However, there could be false positives, which static analysis and tree-shaking can not exclude: - `qs` through these dependencies (get-intrinsic: [source](https://github.com/ljharb/get-intrinsic/blob/main/index.js#L12)) - `function-bind` ([source](https://github.com/Raynos/function-bind/blob/master/implementation.js#L42)) - `has` ([source](https://github.com/tarruda/has/blob/master/src/index.js#L5)) This PR leverages the existing `config` export to let user allow some of their files. it’s meant for allowing users to import 3rd party modules who embed dynamic code evaluation, but do not use it (because or code paths), and can't be tree-shaked. By default, it’s keeping the existing behavior: warn in dev, fails to build. If users allow dynamic code, and that code is reached at runtime, their app stills breaks. ### 🧪 How to test? - (existing) integration tests for disallowing dynamic code evaluation: `pnpm testheadless --testPathPattern=runtime-dynamic` - (new) integration tests for allowing dynamic code evaluation: `pnpm testheadless --testPathPattern=runtime-configurable` - (amended) production tests for validating the new configuration keys: `pnpm testheadless --testPathPattern=config-validations` To try it live, you could have an application such as: ```js // lib/index.js /* eslint-disable no-eval */ export function hasUnusedDynamic() { if ((() => false)()) { eval('100') } } export function hasDynamic() { eval('100') } // pages/index.jsx export default function Page({ edgeRoute }) { return <p>{edgeRoute}</p> } export const getServerSideProps = async (req) => { const res = await fetch(`http://localhost:3000/api/route`) const data = await res.json() return { props: { edgeRoute: data.ok ? `Hi from the edge route` : '' } } } // pages/api/route.js import { hasDynamic } from '../../lib' export default async function handle() { hasDynamic() return Response.json({ ok: true }) } export const config = { runtime: 'experimental-edge' , allowDynamic: '/lib/**' } ``` Playing with `config.allowDynamic`, you should be able to: - build the app even if it uses `eval()` (it will obviously fail at runtime) - build the app that _imports but does not use_ `eval()` - run the app in dev, even if it uses `eval()` with no warning ### 🆙 Notes to reviewers Before adding documentation and telemetry, I'd like to collect comments on a couple of points: - the overall design for this feature: is a list of globs useful and easy enough? - should the globs be relative to the application root (current implementation) to to the edge route/middleware file? - (especially to @sokra) is the implementation idiomatic enough? I've leverage loaders to read the _entry point_ configuration once, then the ModuleGraph to get it back during the parsing phase. I couldn't re-use the existing `getExtractMetadata()` facility since it's happening late after the parsing. - there's a glitch with `import { ServerRuntime } from '../../types'` in `get-page-static-info.ts` ([here](https://github.com/vercel/next.js/pull/39539/files#diff-cb7ac6392c3dd707c5edab159c3144ec114eafea92dad5d98f4eedfc612174d2L12)). I had to use `next/types` because it was failing during lint. Any clue why? ### ☑️ Checklist - [ ] 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` - [x] Integration tests added - [x] Documentation added - [x] Telemetry added. In case of a feature if it's used or not. - [x] Errors have helpful link attached, see `contributing.md`
2022-09-13 00:01:00 +02:00
rootDir,
matchers: encodedMatchers,
}: MiddlewareLoaderOptions = this.getOptions()
const matchers = encodedMatchers ? decodeMatchers(encodedMatchers) : undefined
const stringifiedPagePath = stringifyRequest(this, absolutePagePath)
Refactor Page Paths utils and Middleware Plugin (#36576) This PR brings some significant refactoring in preparation for upcoming middleware changes. Each commit can be reviewed independently, here is a summary of what each one does and the reasoning behind it: - [Move pagesDir to next-dev-server](https://github.com/javivelasco/next.js/pull/12/commits/f2fe154c007379f71c14960ddc553eaaaf786ffa) simply moves the `pagesDir` property to the dev server which is the only place where it is needed. Having it for every server is misleading. - [Move (de)normalize page path utils to a file page-path-utils.ts](https://github.com/javivelasco/next.js/pull/12/commits/27cedf087187b9632ef82a34b3af9cc4fe05d98b) Moves the functions to normalize and denormalize page paths to a single file that is intended to hold every utility function that transforms page paths. Since those are complementary it makes sense to have them together. I also added explanatory comments on why they are not idempotent and examples for input -> output that I find very useful. - [Extract removePagePathTail](https://github.com/javivelasco/next.js/pull/12/commits/6b121332aa9d3e50bd0f28b691fb7faea1b95f51) This extracts a function to remove the tail on a page path (absolute or relative). I'm sure there will be other contexts where we can use it. - [Extract getPagePaths and refactor findPageFile](https://github.com/javivelasco/next.js/pull/12/commits/cf2c7b842eebd8c02f23e79345681a794516b646) This extracts a function `getPagePaths` that is used to generate an array of paths to inspect when looking for a page file from `findPageFile`. Then it refactors such function to use it parallelizing lookups. This will allow us to print every path we look at when looking for a file which can be useful for debugging. It also adds a `flatten` helper. - [Refactor onDemandEntryHandler](https://github.com/javivelasco/next.js/pull/12/commits/4be685c37e3d1b797e929ea4f31495ed7b00e1cc) I've found this one quite difficult to understand so it is refactored to use some of the previously mentioned functions and make it easier to read. - [Extract absolutePagePath util](https://github.com/javivelasco/next.js/pull/12/commits/3bc078347426c73491a076d54ef4de977d9da073) Extracts yet another util from the `next-dev-server` that transforms an absolute path into a page name. Of course it adds comments, parameters and examples. - [Refactor MiddlewarePlugin](https://github.com/javivelasco/next.js/pull/12/commits/c595a2cc629b358cc61861a8a4848b7890d0a15b) This is the most significant change. The logic here was very hard to understand so it is totally redistributed with comments. This also removes a global variable `ssrEntries` that was deprecated in favour of module metadata added to Webpack from loaders keeping less dependencies. It also adds types and makes a clear distinction between phases where we statically analyze the code, find metadata and generate the manifest file cc @shuding @huozhi EDIT: - [Split page path utils](https://github.com/vercel/next.js/pull/36576/commits/158fb002d02887d7ce4be6747cf550a825a426eb) After seeing one of the utils was being used by the client while it was defined originally in the server, with this PR we are splitting the util into multiple files and moving it to `shared/lib` in order to make explicit that those can be also imported from client.
2022-04-30 13:19:27 +02:00
const buildInfo = getModuleBuildInfo(this._module)
buildInfo.nextEdgeMiddleware = {
matchers,
page:
page.replace(new RegExp(`/${MIDDLEWARE_LOCATION_REGEXP}$`), '') || '/',
Refactor Page Paths utils and Middleware Plugin (#36576) This PR brings some significant refactoring in preparation for upcoming middleware changes. Each commit can be reviewed independently, here is a summary of what each one does and the reasoning behind it: - [Move pagesDir to next-dev-server](https://github.com/javivelasco/next.js/pull/12/commits/f2fe154c007379f71c14960ddc553eaaaf786ffa) simply moves the `pagesDir` property to the dev server which is the only place where it is needed. Having it for every server is misleading. - [Move (de)normalize page path utils to a file page-path-utils.ts](https://github.com/javivelasco/next.js/pull/12/commits/27cedf087187b9632ef82a34b3af9cc4fe05d98b) Moves the functions to normalize and denormalize page paths to a single file that is intended to hold every utility function that transforms page paths. Since those are complementary it makes sense to have them together. I also added explanatory comments on why they are not idempotent and examples for input -> output that I find very useful. - [Extract removePagePathTail](https://github.com/javivelasco/next.js/pull/12/commits/6b121332aa9d3e50bd0f28b691fb7faea1b95f51) This extracts a function to remove the tail on a page path (absolute or relative). I'm sure there will be other contexts where we can use it. - [Extract getPagePaths and refactor findPageFile](https://github.com/javivelasco/next.js/pull/12/commits/cf2c7b842eebd8c02f23e79345681a794516b646) This extracts a function `getPagePaths` that is used to generate an array of paths to inspect when looking for a page file from `findPageFile`. Then it refactors such function to use it parallelizing lookups. This will allow us to print every path we look at when looking for a file which can be useful for debugging. It also adds a `flatten` helper. - [Refactor onDemandEntryHandler](https://github.com/javivelasco/next.js/pull/12/commits/4be685c37e3d1b797e929ea4f31495ed7b00e1cc) I've found this one quite difficult to understand so it is refactored to use some of the previously mentioned functions and make it easier to read. - [Extract absolutePagePath util](https://github.com/javivelasco/next.js/pull/12/commits/3bc078347426c73491a076d54ef4de977d9da073) Extracts yet another util from the `next-dev-server` that transforms an absolute path into a page name. Of course it adds comments, parameters and examples. - [Refactor MiddlewarePlugin](https://github.com/javivelasco/next.js/pull/12/commits/c595a2cc629b358cc61861a8a4848b7890d0a15b) This is the most significant change. The logic here was very hard to understand so it is totally redistributed with comments. This also removes a global variable `ssrEntries` that was deprecated in favour of module metadata added to Webpack from loaders keeping less dependencies. It also adds types and makes a clear distinction between phases where we statically analyze the code, find metadata and generate the manifest file cc @shuding @huozhi EDIT: - [Split page path utils](https://github.com/vercel/next.js/pull/36576/commits/158fb002d02887d7ce4be6747cf550a825a426eb) After seeing one of the utils was being used by the client while it was defined originally in the server, with this PR we are splitting the util into multiple files and moving it to `shared/lib` in order to make explicit that those can be also imported from client.
2022-04-30 13:19:27 +02:00
}
feat(edge): allows configuring Dynamic code execution guard (#39539) ### 📖 What's in there? Dynamic code evaluation (`eval()`, `new Function()`, ...) is not supported on the edge runtime, hence why we fail the build when detecting such statement in the middleware or `experimental-edge` routes at build time. However, there could be false positives, which static analysis and tree-shaking can not exclude: - `qs` through these dependencies (get-intrinsic: [source](https://github.com/ljharb/get-intrinsic/blob/main/index.js#L12)) - `function-bind` ([source](https://github.com/Raynos/function-bind/blob/master/implementation.js#L42)) - `has` ([source](https://github.com/tarruda/has/blob/master/src/index.js#L5)) This PR leverages the existing `config` export to let user allow some of their files. it’s meant for allowing users to import 3rd party modules who embed dynamic code evaluation, but do not use it (because or code paths), and can't be tree-shaked. By default, it’s keeping the existing behavior: warn in dev, fails to build. If users allow dynamic code, and that code is reached at runtime, their app stills breaks. ### 🧪 How to test? - (existing) integration tests for disallowing dynamic code evaluation: `pnpm testheadless --testPathPattern=runtime-dynamic` - (new) integration tests for allowing dynamic code evaluation: `pnpm testheadless --testPathPattern=runtime-configurable` - (amended) production tests for validating the new configuration keys: `pnpm testheadless --testPathPattern=config-validations` To try it live, you could have an application such as: ```js // lib/index.js /* eslint-disable no-eval */ export function hasUnusedDynamic() { if ((() => false)()) { eval('100') } } export function hasDynamic() { eval('100') } // pages/index.jsx export default function Page({ edgeRoute }) { return <p>{edgeRoute}</p> } export const getServerSideProps = async (req) => { const res = await fetch(`http://localhost:3000/api/route`) const data = await res.json() return { props: { edgeRoute: data.ok ? `Hi from the edge route` : '' } } } // pages/api/route.js import { hasDynamic } from '../../lib' export default async function handle() { hasDynamic() return Response.json({ ok: true }) } export const config = { runtime: 'experimental-edge' , allowDynamic: '/lib/**' } ``` Playing with `config.allowDynamic`, you should be able to: - build the app even if it uses `eval()` (it will obviously fail at runtime) - build the app that _imports but does not use_ `eval()` - run the app in dev, even if it uses `eval()` with no warning ### 🆙 Notes to reviewers Before adding documentation and telemetry, I'd like to collect comments on a couple of points: - the overall design for this feature: is a list of globs useful and easy enough? - should the globs be relative to the application root (current implementation) to to the edge route/middleware file? - (especially to @sokra) is the implementation idiomatic enough? I've leverage loaders to read the _entry point_ configuration once, then the ModuleGraph to get it back during the parsing phase. I couldn't re-use the existing `getExtractMetadata()` facility since it's happening late after the parsing. - there's a glitch with `import { ServerRuntime } from '../../types'` in `get-page-static-info.ts` ([here](https://github.com/vercel/next.js/pull/39539/files#diff-cb7ac6392c3dd707c5edab159c3144ec114eafea92dad5d98f4eedfc612174d2L12)). I had to use `next/types` because it was failing during lint. Any clue why? ### ☑️ Checklist - [ ] 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` - [x] Integration tests added - [x] Documentation added - [x] Telemetry added. In case of a feature if it's used or not. - [x] Errors have helpful link attached, see `contributing.md`
2022-09-13 00:01:00 +02:00
buildInfo.rootDir = rootDir
return `
fix(edge-runtime): undefined global in edge runtime. (#38769) ## How to reproduce 1. create a next.js app with a middleware (or an edge route) that imports a node.js module: ```js // middleware.js import { NextResponse } from 'next/server' import { basename } from 'path' export default async function middleware() { basename() return NextResponse.next() } ``` 2. deploy it to vercel with `vc` 3. go to the your function logs in Vercel Front (https://vercel.com/$user/$project/$deployment/functions) 4. in another tab, query your application > it results in a 500 page: <img width="517" alt="image" src="https://user-images.githubusercontent.com/186268/179557102-72568ca9-bcfd-49e2-9b9c-c51c3064f2d7.png"> > in the logs you should see: <img width="1220" alt="image" src="https://user-images.githubusercontent.com/186268/179557266-498f3290-b7df-46ac-8816-7bb396821245.png"> ## Expected behavior The route should fail indeed in a 500, because Edge runtime **does not support node.js modules**. However the error in logs should be completely different: ```shell error - Error: The edge runtime does not support Node.js 'path' module. Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime ``` ## Notes to reviewers I introduced this issue in #38234. Prior to the PR above, the app would not even build, as we were checking imported node.js module during the build with AST analysis. Since #38234, the app would build and should fail at runtime, with an appropriate error. The mistake was to declare `__import_unsupported` function in the sandbox's context, that is only used in `next dev` and `next start`, but not shipped to Vercel platform. By loading it inside webpack loaders (both middleware and edge route), we ensure it will be defined on Vercel as well. The existing test suite (`pnpm testheadless --testPathPattern=runtime-module-error`) covers them.
2022-07-20 16:53:27 +02:00
import { adapter, blockUnallowedResponse, enhanceGlobals } from 'next/dist/server/web/adapter'
fix(edge-runtime): undefined global in edge runtime. (#38769) ## How to reproduce 1. create a next.js app with a middleware (or an edge route) that imports a node.js module: ```js // middleware.js import { NextResponse } from 'next/server' import { basename } from 'path' export default async function middleware() { basename() return NextResponse.next() } ``` 2. deploy it to vercel with `vc` 3. go to the your function logs in Vercel Front (https://vercel.com/$user/$project/$deployment/functions) 4. in another tab, query your application > it results in a 500 page: <img width="517" alt="image" src="https://user-images.githubusercontent.com/186268/179557102-72568ca9-bcfd-49e2-9b9c-c51c3064f2d7.png"> > in the logs you should see: <img width="1220" alt="image" src="https://user-images.githubusercontent.com/186268/179557266-498f3290-b7df-46ac-8816-7bb396821245.png"> ## Expected behavior The route should fail indeed in a 500, because Edge runtime **does not support node.js modules**. However the error in logs should be completely different: ```shell error - Error: The edge runtime does not support Node.js 'path' module. Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime ``` ## Notes to reviewers I introduced this issue in #38234. Prior to the PR above, the app would not even build, as we were checking imported node.js module during the build with AST analysis. Since #38234, the app would build and should fail at runtime, with an appropriate error. The mistake was to declare `__import_unsupported` function in the sandbox's context, that is only used in `next dev` and `next start`, but not shipped to Vercel platform. By loading it inside webpack loaders (both middleware and edge route), we ensure it will be defined on Vercel as well. The existing test suite (`pnpm testheadless --testPathPattern=runtime-module-error`) covers them.
2022-07-20 16:53:27 +02:00
enhanceGlobals()
var mod = require(${stringifiedPagePath})
var handler = mod.middleware || mod.default;
if (typeof handler !== 'function') {
throw new Error('The Middleware "pages${page}" must export a \`middleware\` or a \`default\` function');
}
export default function (opts) {
feat(middleware)!: forbids middleware response body (#36835) _Hello Next.js team! First PR here, I hope I've followed the right practices._ ### What's in there? It has been decided to only support the following uses cases in Next.js' middleware: - rewrite the URL (`x-middleware-rewrite` response header) - redirect to another URL (`Location` response header) - pass on to the next piece in the request pipeline (`x-middleware-next` response header) 1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`). 2. at build time, this warning becomes an error. 3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error. All returned/thrown errors contain a link to the documentation. This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more. ### How to try it? - runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core` - build behavior : `yarn jest test/integration/middleware/build-errors` - development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings` ### Notes to reviewers The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~ `Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142 Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers. About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code. Such cases are easy to detect: ```js new Response('a text value') new Response(JSON.stringify({ /* whatever */ }) ``` But these are false-positive cases: ```js function returnNull() { return null } new Response(returnNull()) function doesNothing() {} new Response(doesNothing()) ``` However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body. ## Feature - [x] 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` - [x] Integration tests added - [x] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [x] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
return blockUnallowedResponse(adapter({
...opts,
page: ${JSON.stringify(page)},
handler,
feat(middleware)!: forbids middleware response body (#36835) _Hello Next.js team! First PR here, I hope I've followed the right practices._ ### What's in there? It has been decided to only support the following uses cases in Next.js' middleware: - rewrite the URL (`x-middleware-rewrite` response header) - redirect to another URL (`Location` response header) - pass on to the next piece in the request pipeline (`x-middleware-next` response header) 1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`). 2. at build time, this warning becomes an error. 3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error. All returned/thrown errors contain a link to the documentation. This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more. ### How to try it? - runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core` - build behavior : `yarn jest test/integration/middleware/build-errors` - development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings` ### Notes to reviewers The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~ `Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142 Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers. About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code. Such cases are easy to detect: ```js new Response('a text value') new Response(JSON.stringify({ /* whatever */ }) ``` But these are false-positive cases: ```js function returnNull() { return null } new Response(returnNull()) function doesNothing() {} new Response(doesNothing()) ``` However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body. ## Feature - [x] 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` - [x] Integration tests added - [x] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [x] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
}))
}
`
}