bf089562c7
_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`
112 lines
3.2 KiB
TypeScript
112 lines
3.2 KiB
TypeScript
import { createNext, FileRef } from 'e2e-utils'
|
|
import { NextInstance } from 'test/lib/next-modes/base'
|
|
import { fetchViaHTTP } from 'next-test-utils'
|
|
import path from 'path'
|
|
import fs from 'fs-extra'
|
|
|
|
function baseNextConfig(): Parameters<typeof createNext>[0] {
|
|
return {
|
|
files: {
|
|
'src/add.wasm': new FileRef(path.join(__dirname, './add.wasm')),
|
|
'src/add.js': `
|
|
import wasm from './add.wasm?module'
|
|
const instance$ = WebAssembly.instantiate(wasm);
|
|
|
|
export async function increment(a) {
|
|
const { exports } = await instance$;
|
|
return exports.add_one(a);
|
|
}
|
|
`,
|
|
'pages/index.js': `
|
|
export default function () { return <div>Hello, world!</div> }
|
|
`,
|
|
'middleware.js': `
|
|
import { increment } from './src/add.js'
|
|
export default async function middleware(request) {
|
|
const input = Number(request.nextUrl.searchParams.get('input')) || 1;
|
|
const value = await increment(input);
|
|
return new Response(null, { headers: { data: JSON.stringify({ input, value }) } });
|
|
}
|
|
`,
|
|
},
|
|
}
|
|
}
|
|
|
|
describe('middleware can use wasm files', () => {
|
|
let next: NextInstance
|
|
|
|
beforeAll(async () => {
|
|
const config = baseNextConfig()
|
|
next = await createNext(config)
|
|
})
|
|
afterAll(() => next.destroy())
|
|
|
|
it('uses the wasm file', async () => {
|
|
const response = await fetchViaHTTP(next.url, '/')
|
|
expect(extractJSON(response)).toEqual({
|
|
input: 1,
|
|
value: 2,
|
|
})
|
|
})
|
|
|
|
it('can be called twice', async () => {
|
|
const response = await fetchViaHTTP(next.url, '/', { input: 2 })
|
|
expect(extractJSON(response)).toEqual({
|
|
input: 2,
|
|
value: 3,
|
|
})
|
|
})
|
|
|
|
if (!(global as any).isNextDeploy) {
|
|
it('lists the necessary wasm bindings in the manifest', async () => {
|
|
const manifestPath = path.join(
|
|
next.testDir,
|
|
'.next/server/middleware-manifest.json'
|
|
)
|
|
const manifest = await fs.readJSON(manifestPath)
|
|
expect(manifest.middleware['/']).toMatchObject({
|
|
wasm: [
|
|
{
|
|
filePath:
|
|
'server/edge-chunks/wasm_58ccff8b2b94b5dac6ef8957082ecd8f6d34186d.wasm',
|
|
name: 'wasm_58ccff8b2b94b5dac6ef8957082ecd8f6d34186d',
|
|
},
|
|
],
|
|
})
|
|
})
|
|
}
|
|
})
|
|
|
|
describe('middleware can use wasm files with the experimental modes on', () => {
|
|
let next: NextInstance
|
|
|
|
beforeAll(async () => {
|
|
const config = baseNextConfig()
|
|
config.files['next.config.js'] = `
|
|
module.exports = {
|
|
webpack(config) {
|
|
config.output.webassemblyModuleFilename = 'static/wasm/[modulehash].wasm'
|
|
|
|
// Since Webpack 5 doesn't enable WebAssembly by default, we should do it manually
|
|
config.experiments = { ...config.experiments, asyncWebAssembly: true }
|
|
|
|
return config
|
|
},
|
|
}
|
|
`
|
|
next = await createNext(config)
|
|
})
|
|
afterAll(() => next.destroy())
|
|
|
|
it('uses the wasm file', async () => {
|
|
const response = await fetchViaHTTP(next.url, '/')
|
|
expect(extractJSON(response)).toEqual({
|
|
input: 1,
|
|
value: 2,
|
|
})
|
|
})
|
|
})
|
|
|
|
function extractJSON(response) {
|
|
return JSON.parse(response.headers.get('data') ?? '{}')
|
|
}
|