From 2fe2570dcf07e582b235d3405864b5b9cd15720b Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Tue, 3 Jan 2023 14:24:12 -0500 Subject: [PATCH 01/12] test: add Next 13 request header mutation to middleware --- demos/middleware/middleware.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/demos/middleware/middleware.ts b/demos/middleware/middleware.ts index 93c43a0c62..232463089d 100644 --- a/demos/middleware/middleware.ts +++ b/demos/middleware/middleware.ts @@ -8,6 +8,17 @@ export async function middleware(req: NextRequest) { let response const { pathname } = req.nextUrl + if (pathname.startsWith('/api/hello')) { + // Next 13 request header mutation functionality + const headers = new Headers(req.headers) + headers.set('x-from-middleware', 'hello-from-middleware') + return NextResponse.next({ + request: { + headers + } + }) + } + const request = new MiddlewareRequest(req) if (pathname.startsWith('/static')) { // Unlike NextResponse.next(), this actually sends the request to the origin From 1c225224318e46b98d0ba249fb79b1ea269a9b1d Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Wed, 4 Jan 2023 14:40:43 -0500 Subject: [PATCH 02/12] feat: support Next 13 request header mutation --- demos/middleware/middleware.ts | 1 + package-lock.json | 277 +++++++++++++++++- package.json | 3 + .../src/templates/edge-shared/utils.ts | 30 ++ test/mockedDenoModules/html_rewriter/index.ts | 2 + test/templates/edge-shared/utils.spec.ts | 56 ++++ 6 files changed, 367 insertions(+), 2 deletions(-) create mode 100644 test/mockedDenoModules/html_rewriter/index.ts create mode 100644 test/templates/edge-shared/utils.spec.ts diff --git a/demos/middleware/middleware.ts b/demos/middleware/middleware.ts index 232463089d..4f9e5026d5 100644 --- a/demos/middleware/middleware.ts +++ b/demos/middleware/middleware.ts @@ -11,6 +11,7 @@ export async function middleware(req: NextRequest) { if (pathname.startsWith('/api/hello')) { // Next 13 request header mutation functionality const headers = new Headers(req.headers) + debugger; headers.set('x-from-middleware', 'hello-from-middleware') return NextResponse.next({ request: { diff --git a/package-lock.json b/package-lock.json index 39f4468304..df6b87bd04 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24551,7 +24551,7 @@ }, "packages/next": { "name": "@netlify/next", - "version": "1.4.2", + "version": "1.4.3", "license": "MIT", "devDependencies": { "@netlify/edge-functions": "^2.0.0", @@ -24569,7 +24569,7 @@ }, "packages/runtime": { "name": "@netlify/plugin-nextjs", - "version": "4.29.3", + "version": "4.29.4", "license": "MIT", "dependencies": { "@netlify/esbuild": "0.14.39", @@ -24623,6 +24623,201 @@ "engines": { "node": ">=10" } + }, + "node_modules/@next/swc-android-arm-eabi": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-android-arm-eabi/-/swc-android-arm-eabi-13.0.7.tgz", + "integrity": "sha512-QTEamOK/LCwBf05GZ261rULMbZEpE3TYdjHlXfznV+nXwTztzkBNFXwP67gv2wW44BROzgi/vrR9H8oP+J5jxg==", + "cpu": [ + "arm" + ], + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-android-arm64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-android-arm64/-/swc-android-arm64-13.0.7.tgz", + "integrity": "sha512-wcy2H0Tl9ME8vKy2GnJZ7Mybwys+43F/Eh2Pvph7mSDpMbYBJ6iA0zeY62iYYXxlZhnAID3+h79FUqUEakkClw==", + "cpu": [ + "arm64" + ], + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-darwin-arm64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-darwin-arm64/-/swc-darwin-arm64-13.0.7.tgz", + "integrity": "sha512-F/mU7csN1/J2cqXJPMgTQ6MwAbc1pJ6sp6W+X0z5JEY4IFDzxKd3wRc3pCiNF7j8xW381JlNpWxhjCctnNmfaw==", + "cpu": [ + "arm64" + ], + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-darwin-x64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-darwin-x64/-/swc-darwin-x64-13.0.7.tgz", + "integrity": "sha512-636AuRQynCPnIPRVzcCk5B7OMq9XjaYam2T0HeWUCE6y7EqEO3kxiuZ4QmN81T7A6Ydb+JnivYrLelHXmgdj6A==", + "cpu": [ + "x64" + ], + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-freebsd-x64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-freebsd-x64/-/swc-freebsd-x64-13.0.7.tgz", + "integrity": "sha512-92XAMzNgQazowZ9t7uZmHRA5VdBl/SwEdrf5UybdfRovsxB4r3+yJWEvFaqYpSEp0gwndbwLokJdpz7OwFdL3Q==", + "cpu": [ + "x64" + ], + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-linux-arm-gnueabihf": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-arm-gnueabihf/-/swc-linux-arm-gnueabihf-13.0.7.tgz", + "integrity": "sha512-3r1CWl5P6I5n5Yxip8EXv/Rfu2Cp6wVmIOpvmczyUR82j+bcMkwPAcUjNkG/vMCagS4xV7NElrcdGb39iFmfLg==", + "cpu": [ + "arm" + ], + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-linux-arm64-gnu": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-arm64-gnu/-/swc-linux-arm64-gnu-13.0.7.tgz", + "integrity": "sha512-RXo8tt6ppiwyS6hpDw3JdAjKcdVewsefxnxk9xOH4mRhMyq9V2lQx0e24X/dRiZqkx3jnWReR2WRrUlgN1UkSQ==", + "cpu": [ + "arm64" + ], + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-linux-arm64-musl": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-arm64-musl/-/swc-linux-arm64-musl-13.0.7.tgz", + "integrity": "sha512-RWpnW+bmfXyxyY7iARbueYDGuIF+BEp3etLeYh/RUNHb9PhOHLDgJOG8haGSykud3a6CcyBI8hEjqOhoObaDpw==", + "cpu": [ + "arm64" + ], + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-linux-x64-gnu": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-x64-gnu/-/swc-linux-x64-gnu-13.0.7.tgz", + "integrity": "sha512-/ygUIiMMTYnbKlFs5Ba9J5k/tNxFWy8eI1bBF8UuMTvV8QJHl/aLDiA5dwsei2kk99/cu3eay62JnJXkSk3RSQ==", + "cpu": [ + "x64" + ], + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-linux-x64-musl": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-x64-musl/-/swc-linux-x64-musl-13.0.7.tgz", + "integrity": "sha512-dLzr6AL77USJN0ejgx5AS8O8SbFlbYTzs0XwAWag4oQpUG2p3ARvxwQgYQ0Z+6EP0zIRZ/XfLkN/mhsyi3m4PA==", + "cpu": [ + "x64" + ], + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-win32-arm64-msvc": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-win32-arm64-msvc/-/swc-win32-arm64-msvc-13.0.7.tgz", + "integrity": "sha512-+vFIVa82AwqFkpFClKT+n73fGxrhAZ2u1u3mDYEBdxO6c9U4Pj3S5tZFsGFK9kLT/bFvf/eeVOICSLCC7MSgJQ==", + "cpu": [ + "arm64" + ], + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-win32-ia32-msvc": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-win32-ia32-msvc/-/swc-win32-ia32-msvc-13.0.7.tgz", + "integrity": "sha512-RNLXIhp+assD39dQY9oHhDxw+/qSJRARKhOFsHfOtf8yEfCHqcKkn3X/L+ih60ntaEqK294y1WkMk6ylotsxwA==", + "cpu": [ + "ia32" + ], + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@next/swc-win32-x64-msvc": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-win32-x64-msvc/-/swc-win32-x64-msvc-13.0.7.tgz", + "integrity": "sha512-kvdnlLcrnEq72ZP0lqe2Z5NqvB9N5uSCvtXJ0PhKvNncWWd0fEG9Ec9erXgwCmVlM2ytw41k9/uuQ+SVw4Pihw==", + "cpu": [ + "x64" + ], + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } } }, "dependencies": { @@ -42882,6 +43077,84 @@ "compress-commons": "^4.1.0", "readable-stream": "^3.6.0" } + }, + "@next/swc-android-arm-eabi": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-android-arm-eabi/-/swc-android-arm-eabi-13.0.7.tgz", + "integrity": "sha512-QTEamOK/LCwBf05GZ261rULMbZEpE3TYdjHlXfznV+nXwTztzkBNFXwP67gv2wW44BROzgi/vrR9H8oP+J5jxg==", + "optional": true + }, + "@next/swc-android-arm64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-android-arm64/-/swc-android-arm64-13.0.7.tgz", + "integrity": "sha512-wcy2H0Tl9ME8vKy2GnJZ7Mybwys+43F/Eh2Pvph7mSDpMbYBJ6iA0zeY62iYYXxlZhnAID3+h79FUqUEakkClw==", + "optional": true + }, + "@next/swc-darwin-arm64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-darwin-arm64/-/swc-darwin-arm64-13.0.7.tgz", + "integrity": "sha512-F/mU7csN1/J2cqXJPMgTQ6MwAbc1pJ6sp6W+X0z5JEY4IFDzxKd3wRc3pCiNF7j8xW381JlNpWxhjCctnNmfaw==", + "optional": true + }, + "@next/swc-darwin-x64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-darwin-x64/-/swc-darwin-x64-13.0.7.tgz", + "integrity": "sha512-636AuRQynCPnIPRVzcCk5B7OMq9XjaYam2T0HeWUCE6y7EqEO3kxiuZ4QmN81T7A6Ydb+JnivYrLelHXmgdj6A==", + "optional": true + }, + "@next/swc-freebsd-x64": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-freebsd-x64/-/swc-freebsd-x64-13.0.7.tgz", + "integrity": "sha512-92XAMzNgQazowZ9t7uZmHRA5VdBl/SwEdrf5UybdfRovsxB4r3+yJWEvFaqYpSEp0gwndbwLokJdpz7OwFdL3Q==", + "optional": true + }, + "@next/swc-linux-arm-gnueabihf": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-arm-gnueabihf/-/swc-linux-arm-gnueabihf-13.0.7.tgz", + "integrity": "sha512-3r1CWl5P6I5n5Yxip8EXv/Rfu2Cp6wVmIOpvmczyUR82j+bcMkwPAcUjNkG/vMCagS4xV7NElrcdGb39iFmfLg==", + "optional": true + }, + "@next/swc-linux-arm64-gnu": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-arm64-gnu/-/swc-linux-arm64-gnu-13.0.7.tgz", + "integrity": "sha512-RXo8tt6ppiwyS6hpDw3JdAjKcdVewsefxnxk9xOH4mRhMyq9V2lQx0e24X/dRiZqkx3jnWReR2WRrUlgN1UkSQ==", + "optional": true + }, + "@next/swc-linux-arm64-musl": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-arm64-musl/-/swc-linux-arm64-musl-13.0.7.tgz", + "integrity": "sha512-RWpnW+bmfXyxyY7iARbueYDGuIF+BEp3etLeYh/RUNHb9PhOHLDgJOG8haGSykud3a6CcyBI8hEjqOhoObaDpw==", + "optional": true + }, + "@next/swc-linux-x64-gnu": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-x64-gnu/-/swc-linux-x64-gnu-13.0.7.tgz", + "integrity": "sha512-/ygUIiMMTYnbKlFs5Ba9J5k/tNxFWy8eI1bBF8UuMTvV8QJHl/aLDiA5dwsei2kk99/cu3eay62JnJXkSk3RSQ==", + "optional": true + }, + "@next/swc-linux-x64-musl": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-linux-x64-musl/-/swc-linux-x64-musl-13.0.7.tgz", + "integrity": "sha512-dLzr6AL77USJN0ejgx5AS8O8SbFlbYTzs0XwAWag4oQpUG2p3ARvxwQgYQ0Z+6EP0zIRZ/XfLkN/mhsyi3m4PA==", + "optional": true + }, + "@next/swc-win32-arm64-msvc": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-win32-arm64-msvc/-/swc-win32-arm64-msvc-13.0.7.tgz", + "integrity": "sha512-+vFIVa82AwqFkpFClKT+n73fGxrhAZ2u1u3mDYEBdxO6c9U4Pj3S5tZFsGFK9kLT/bFvf/eeVOICSLCC7MSgJQ==", + "optional": true + }, + "@next/swc-win32-ia32-msvc": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-win32-ia32-msvc/-/swc-win32-ia32-msvc-13.0.7.tgz", + "integrity": "sha512-RNLXIhp+assD39dQY9oHhDxw+/qSJRARKhOFsHfOtf8yEfCHqcKkn3X/L+ih60ntaEqK294y1WkMk6ylotsxwA==", + "optional": true + }, + "@next/swc-win32-x64-msvc": { + "version": "13.0.7", + "resolved": "https://registry.npmjs.org/@next/swc-win32-x64-msvc/-/swc-win32-x64-msvc-13.0.7.tgz", + "integrity": "sha512-kvdnlLcrnEq72ZP0lqe2Z5NqvB9N5uSCvtXJ0PhKvNncWWd0fEG9Ec9erXgwCmVlM2ytw41k9/uuQ+SVw4Pihw==", + "optional": true } } } diff --git a/package.json b/package.json index 115b76605e..a03dd1f7b1 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,9 @@ "!**/test/fixtures/**", "!**/test/sample/**" ], + "moduleNameMapper": { + "https://deno.land/x/html_rewriter@v0.1.0-pre.17": "/test/mockedDenoModules/html_rewriter" + }, "transform": { "\\.[jt]sx?$": "babel-jest" }, diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts index 4d9b95cde6..9a438da267 100644 --- a/packages/runtime/src/templates/edge-shared/utils.ts +++ b/packages/runtime/src/templates/edge-shared/utils.ts @@ -59,6 +59,34 @@ function isMiddlewareResponse(response: Response | MiddlewareResponse): response return 'dataTransforms' in response } +// Next 13 supports request header mutations and has the side effect of prepending header values with 'x-middleware-request' +// as part of invoking NextResponse.next() in the middleware. We need to remove that before sending the response the user +// as the code that removes it in Next isn't run based on how we handle the middleware +// +// Related Next.js code: +// * https://github.com/vercel/next.js/blob/68d06fe015b28d8f81da52ca107a5f4bd72ab37c/packages/next/server/next-server.ts#L1918-L1928 +// * https://github.com/vercel/next.js/blob/43c9d8940dc42337dd2f7d66aa90e6abf952278e/packages/next/server/web/spec-extension/response.ts#L10-L27 +export function updateModifiedHeaders(response: Response) { + const overriddenHeaders = response.headers.get('x-middleware-override-headers') || '' + + if (!overriddenHeaders) { + return response + } + + const headersToUpdate = overriddenHeaders.split(',') + + for (const header of headersToUpdate) { + const oldHeaderKey = 'x-middleware-request-' + header + const headerValue = response.headers.get(oldHeaderKey) || '' + + response.headers.set(header, headerValue) + response.headers.delete(oldHeaderKey) + } + response.headers.delete('x-middleware-override-headers') + + return response +} + export const buildResponse = async ({ result, request, @@ -68,6 +96,8 @@ export const buildResponse = async ({ request: Request context: Context }) => { + result.response = updateModifiedHeaders(result.response) + // They've returned the MiddlewareRequest directly, so we'll call `next()` for them. if (isMiddlewareRequest(result.response)) { result.response = await result.response.next() diff --git a/test/mockedDenoModules/html_rewriter/index.ts b/test/mockedDenoModules/html_rewriter/index.ts new file mode 100644 index 0000000000..e3f0172226 --- /dev/null +++ b/test/mockedDenoModules/html_rewriter/index.ts @@ -0,0 +1,2 @@ +export const ElementHandlers = jest.fn() +export const HTMLRewriter = jest.fn() diff --git a/test/templates/edge-shared/utils.spec.ts b/test/templates/edge-shared/utils.spec.ts new file mode 100644 index 0000000000..0fa4787da7 --- /dev/null +++ b/test/templates/edge-shared/utils.spec.ts @@ -0,0 +1,56 @@ +import {updateModifiedHeaders, FetchEventResult} from '../../../packages/runtime/src/templates/edge-shared/utils' + +describe('updateModifiedHeaders', () => { + it('does not modify the headers if \'x-middleware-override-headers\' is not found', () => { + const mockHeaders = new Headers() + // There shouldn't be a case where x-middleware-override-headers is not set and a header has + // been modified with 'x-middleware-request' added to it, this is more to confirm the test case + mockHeaders.set('x-middleware-request-foo', 'bar') + + let mockResult: FetchEventResult = { + response: new Response('', {headers: mockHeaders}), + waitUntil: Promise.resolve() + } + + mockResult.response = updateModifiedHeaders(mockResult.response) + + expect(mockResult.response.headers.get('x-middleware-request-foo')).toEqual('bar') + }) + + describe('when the \'x-middleware-override-headers\' headers is present', () => { + let mockHeaders + let mockResult: FetchEventResult + + beforeEach(() => { + mockHeaders = new Headers() + mockHeaders.set('foo', 'bar') + mockHeaders.set('x-middleware-request-hello', 'world') + mockHeaders.set('x-middleware-request-test', '123') + mockHeaders.set('x-middleware-override-headers', 'hello,test') + + mockResult = { + response: new Response('', {headers: mockHeaders}), + waitUntil: Promise.resolve() + } + + mockResult.response = updateModifiedHeaders(mockResult.response) + + }) + + it('does not modify headers that are missing \'x-middleware-request\' in the name', () => { + expect(mockResult.response.headers.get('foo')).toEqual('bar') + }) + + it('removes \'x-middleware-request-\' from headers', () => { + expect(mockResult.response.headers.get('x-middleware-request-hello')).toBe(null) + expect(mockResult.response.headers.get('x-middleware-request-test')).toBe(null) + + expect(mockResult.response.headers.get('hello')).toEqual('world') + expect(mockResult.response.headers.get('test')).toEqual('123') + }) + + it('removes \'x-middleware-override-headers\' after cleaning headers', () => { + expect(mockResult.response.headers.get('x-middleware-override-headers')).toBe(null) + }) + }) +}) From 0aaa3ac0f339521aa3dea16fcc28eef9b55f1d8f Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Wed, 4 Jan 2023 14:46:04 -0500 Subject: [PATCH 03/12] refactor: remove duplicate route conditional --- demos/middleware/middleware.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/demos/middleware/middleware.ts b/demos/middleware/middleware.ts index 4f9e5026d5..d9b307677b 100644 --- a/demos/middleware/middleware.ts +++ b/demos/middleware/middleware.ts @@ -12,7 +12,7 @@ export async function middleware(req: NextRequest) { // Next 13 request header mutation functionality const headers = new Headers(req.headers) debugger; - headers.set('x-from-middleware', 'hello-from-middleware') + headers.set('x-hello', 'world') return NextResponse.next({ request: { headers @@ -36,12 +36,6 @@ export async function middleware(req: NextRequest) { return res } - if (pathname.startsWith('/api/hello')) { - // Add a header to the request - req.headers.set('x-hello', 'world') - return request.next() - } - if (pathname.startsWith('/api/geo')) { req.headers.set('x-geo-country', req.geo.country) req.headers.set('x-geo-region', req.geo.region) From 20f9464fd2bc9370bf4fe9de921311fd42da4f63 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Wed, 4 Jan 2023 15:21:28 -0500 Subject: [PATCH 04/12] refactor: remove debugger --- demos/middleware/middleware.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demos/middleware/middleware.ts b/demos/middleware/middleware.ts index d9b307677b..4ef5240597 100644 --- a/demos/middleware/middleware.ts +++ b/demos/middleware/middleware.ts @@ -11,7 +11,7 @@ export async function middleware(req: NextRequest) { if (pathname.startsWith('/api/hello')) { // Next 13 request header mutation functionality const headers = new Headers(req.headers) - debugger; + headers.set('x-hello', 'world') return NextResponse.next({ request: { From 52326fc98c01b304b65625118522ec4f74d924c0 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Thu, 5 Jan 2023 13:34:59 -0500 Subject: [PATCH 05/12] test: update test to use Deno style of testing include github workflow --- .github/workflows/test-deno.yml | 21 ++++++++++++++++ package.json | 9 +++---- .../src/templates/edge-shared/utils.test.ts | 25 ++++++++++++------- .../src/templates/edge-shared/utils.ts | 4 +-- test/mockedDenoModules/html_rewriter/index.ts | 2 -- 5 files changed, 43 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/test-deno.yml rename test/templates/edge-shared/utils.spec.ts => packages/runtime/src/templates/edge-shared/utils.test.ts (64%) delete mode 100644 test/mockedDenoModules/html_rewriter/index.ts diff --git a/.github/workflows/test-deno.yml b/.github/workflows/test-deno.yml new file mode 100644 index 0000000000..6c15856206 --- /dev/null +++ b/.github/workflows/test-deno.yml @@ -0,0 +1,21 @@ +name: Deno tests + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Git Checkout Deno Module + uses: actions/checkout@v2 + - name: Use Deno Version ${{ matrix.deno-version }} + uses: denolib/setup-deno@v2 + with: + deno-version: vx.x.x + - name: Test Deno + run: deno test packages/runtime/src/templates/edge-shared/ diff --git a/package.json b/package.json index a03dd1f7b1..3a7c805faa 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,8 @@ "build": "npm run build -w packages/next -w packages/runtime", "postinstall": "run-s build install-husky", "install-husky": "if-env CI=1 || husky install node_modules/@netlify/eslint-config-node/.husky", - "test": "run-s build:demo test:jest", + "test": "run-s build:demo test:jest test:deno", + "test:deno": "deno test packages/runtime/src/templates/edge-shared/", "test:next": "jest -c test/e2e/jest.config.js", "test:next:disabled": "RUN_SKIPPED_TESTS=1 jest -c test/e2e/jest.config.disabled.js", "test:next:all": "RUN_SKIPPED_TESTS=1 jest -c test/e2e/jest.config.all.js", @@ -101,11 +102,9 @@ "**/test/**/*.spec.ts", "!**/test/e2e/**", "!**/test/fixtures/**", - "!**/test/sample/**" + "!**/test/sample/**", + "!**/test/templates/edge-shared/**" ], - "moduleNameMapper": { - "https://deno.land/x/html_rewriter@v0.1.0-pre.17": "/test/mockedDenoModules/html_rewriter" - }, "transform": { "\\.[jt]sx?$": "babel-jest" }, diff --git a/test/templates/edge-shared/utils.spec.ts b/packages/runtime/src/templates/edge-shared/utils.test.ts similarity index 64% rename from test/templates/edge-shared/utils.spec.ts rename to packages/runtime/src/templates/edge-shared/utils.test.ts index 0fa4787da7..03187fa183 100644 --- a/test/templates/edge-shared/utils.spec.ts +++ b/packages/runtime/src/templates/edge-shared/utils.test.ts @@ -1,4 +1,11 @@ -import {updateModifiedHeaders, FetchEventResult} from '../../../packages/runtime/src/templates/edge-shared/utils' +import { assertEquals } from 'https://deno.land/std@0.167.0/testing/asserts.ts' +import { + beforeEach, + describe, + it, +} from "https://deno.land/std@0.167.0/testing/bdd.ts"; +import {updateModifiedHeaders, FetchEventResult} from './utils.ts' + describe('updateModifiedHeaders', () => { it('does not modify the headers if \'x-middleware-override-headers\' is not found', () => { @@ -7,14 +14,14 @@ describe('updateModifiedHeaders', () => { // been modified with 'x-middleware-request' added to it, this is more to confirm the test case mockHeaders.set('x-middleware-request-foo', 'bar') - let mockResult: FetchEventResult = { + const mockResult: FetchEventResult = { response: new Response('', {headers: mockHeaders}), waitUntil: Promise.resolve() } mockResult.response = updateModifiedHeaders(mockResult.response) - expect(mockResult.response.headers.get('x-middleware-request-foo')).toEqual('bar') + assertEquals(mockResult.response.headers.get('x-middleware-request-foo'), 'bar') }) describe('when the \'x-middleware-override-headers\' headers is present', () => { @@ -38,19 +45,19 @@ describe('updateModifiedHeaders', () => { }) it('does not modify headers that are missing \'x-middleware-request\' in the name', () => { - expect(mockResult.response.headers.get('foo')).toEqual('bar') + assertEquals(mockResult.response.headers.get('foo'), 'bar') }) it('removes \'x-middleware-request-\' from headers', () => { - expect(mockResult.response.headers.get('x-middleware-request-hello')).toBe(null) - expect(mockResult.response.headers.get('x-middleware-request-test')).toBe(null) + assertEquals(mockResult.response.headers.get('x-middleware-request-hello'), null) + assertEquals(mockResult.response.headers.get('x-middleware-request-test'), null) - expect(mockResult.response.headers.get('hello')).toEqual('world') - expect(mockResult.response.headers.get('test')).toEqual('123') + assertEquals(mockResult.response.headers.get('hello'), 'world') + assertEquals(mockResult.response.headers.get('test'), '123') }) it('removes \'x-middleware-override-headers\' after cleaning headers', () => { - expect(mockResult.response.headers.get('x-middleware-override-headers')).toBe(null) + assertEquals(mockResult.response.headers.get('x-middleware-override-headers'), null) }) }) }) diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts index 9a438da267..12022a1380 100644 --- a/packages/runtime/src/templates/edge-shared/utils.ts +++ b/packages/runtime/src/templates/edge-shared/utils.ts @@ -67,13 +67,13 @@ function isMiddlewareResponse(response: Response | MiddlewareResponse): response // * https://github.com/vercel/next.js/blob/68d06fe015b28d8f81da52ca107a5f4bd72ab37c/packages/next/server/next-server.ts#L1918-L1928 // * https://github.com/vercel/next.js/blob/43c9d8940dc42337dd2f7d66aa90e6abf952278e/packages/next/server/web/spec-extension/response.ts#L10-L27 export function updateModifiedHeaders(response: Response) { - const overriddenHeaders = response.headers.get('x-middleware-override-headers') || '' + const overriddenHeaders = response.headers.get('x-middleware-override-headers') if (!overriddenHeaders) { return response } - const headersToUpdate = overriddenHeaders.split(',') + const headersToUpdate = overriddenHeaders.split(',').map(header => header.trim()) for (const header of headersToUpdate) { const oldHeaderKey = 'x-middleware-request-' + header diff --git a/test/mockedDenoModules/html_rewriter/index.ts b/test/mockedDenoModules/html_rewriter/index.ts deleted file mode 100644 index e3f0172226..0000000000 --- a/test/mockedDenoModules/html_rewriter/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export const ElementHandlers = jest.fn() -export const HTMLRewriter = jest.fn() From d3221788cad00fca1ab9d49713dbdf37e54f81c9 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Thu, 5 Jan 2023 13:53:57 -0500 Subject: [PATCH 06/12] style: lint --- .../src/templates/edge-shared/utils.test.ts | 36 ++++++++----------- .../src/templates/edge-shared/utils.ts | 3 +- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/packages/runtime/src/templates/edge-shared/utils.test.ts b/packages/runtime/src/templates/edge-shared/utils.test.ts index 03187fa183..2614beda6e 100644 --- a/packages/runtime/src/templates/edge-shared/utils.test.ts +++ b/packages/runtime/src/templates/edge-shared/utils.test.ts @@ -1,22 +1,17 @@ import { assertEquals } from 'https://deno.land/std@0.167.0/testing/asserts.ts' -import { - beforeEach, - describe, - it, -} from "https://deno.land/std@0.167.0/testing/bdd.ts"; -import {updateModifiedHeaders, FetchEventResult} from './utils.ts' - +import { beforeEach, describe, it } from 'https://deno.land/std@0.167.0/testing/bdd.ts' +import { updateModifiedHeaders, FetchEventResult } from './utils.ts' describe('updateModifiedHeaders', () => { - it('does not modify the headers if \'x-middleware-override-headers\' is not found', () => { + it("does not modify the headers if 'x-middleware-override-headers' is not found", () => { const mockHeaders = new Headers() // There shouldn't be a case where x-middleware-override-headers is not set and a header has // been modified with 'x-middleware-request' added to it, this is more to confirm the test case mockHeaders.set('x-middleware-request-foo', 'bar') const mockResult: FetchEventResult = { - response: new Response('', {headers: mockHeaders}), - waitUntil: Promise.resolve() + response: new Response('', { headers: mockHeaders }), + waitUntil: Promise.resolve(), } mockResult.response = updateModifiedHeaders(mockResult.response) @@ -24,7 +19,7 @@ describe('updateModifiedHeaders', () => { assertEquals(mockResult.response.headers.get('x-middleware-request-foo'), 'bar') }) - describe('when the \'x-middleware-override-headers\' headers is present', () => { + describe("when the 'x-middleware-override-headers' headers is present", () => { let mockHeaders let mockResult: FetchEventResult @@ -34,29 +29,28 @@ describe('updateModifiedHeaders', () => { mockHeaders.set('x-middleware-request-hello', 'world') mockHeaders.set('x-middleware-request-test', '123') mockHeaders.set('x-middleware-override-headers', 'hello,test') - + mockResult = { - response: new Response('', {headers: mockHeaders}), - waitUntil: Promise.resolve() + response: new Response('', { headers: mockHeaders }), + waitUntil: Promise.resolve(), } - + mockResult.response = updateModifiedHeaders(mockResult.response) - }) - it('does not modify headers that are missing \'x-middleware-request\' in the name', () => { + it("does not modify headers that are missing 'x-middleware-request' in the name", () => { assertEquals(mockResult.response.headers.get('foo'), 'bar') }) - - it('removes \'x-middleware-request-\' from headers', () => { + + it("removes 'x-middleware-request-' from headers", () => { assertEquals(mockResult.response.headers.get('x-middleware-request-hello'), null) assertEquals(mockResult.response.headers.get('x-middleware-request-test'), null) assertEquals(mockResult.response.headers.get('hello'), 'world') assertEquals(mockResult.response.headers.get('test'), '123') }) - - it('removes \'x-middleware-override-headers\' after cleaning headers', () => { + + it("removes 'x-middleware-override-headers' after cleaning headers", () => { assertEquals(mockResult.response.headers.get('x-middleware-override-headers'), null) }) }) diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts index 12022a1380..0b3edf50b2 100644 --- a/packages/runtime/src/templates/edge-shared/utils.ts +++ b/packages/runtime/src/templates/edge-shared/utils.ts @@ -73,7 +73,7 @@ export function updateModifiedHeaders(response: Response) { return response } - const headersToUpdate = overriddenHeaders.split(',').map(header => header.trim()) + const headersToUpdate = overriddenHeaders.split(',').map((header) => header.trim()) for (const header of headersToUpdate) { const oldHeaderKey = 'x-middleware-request-' + header @@ -96,6 +96,7 @@ export const buildResponse = async ({ request: Request context: Context }) => { + debugger result.response = updateModifiedHeaders(result.response) // They've returned the MiddlewareRequest directly, so we'll call `next()` for them. From 8f38808e161728c601fe52f0ab30431deacb4004 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Thu, 5 Jan 2023 16:00:07 -0500 Subject: [PATCH 07/12] test: update test command --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3a7c805faa..b2cff5f02b 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "build": "npm run build -w packages/next -w packages/runtime", "postinstall": "run-s build install-husky", "install-husky": "if-env CI=1 || husky install node_modules/@netlify/eslint-config-node/.husky", - "test": "run-s build:demo test:jest test:deno", + "test": "run-s build:demo test:jest", "test:deno": "deno test packages/runtime/src/templates/edge-shared/", "test:next": "jest -c test/e2e/jest.config.js", "test:next:disabled": "RUN_SKIPPED_TESTS=1 jest -c test/e2e/jest.config.disabled.js", From 124d15ba7700bf309aa0d95a057252e004ad6783 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Fri, 6 Jan 2023 10:30:25 -0500 Subject: [PATCH 08/12] refactor: remove debugger statement that snuck in --- packages/runtime/src/templates/edge-shared/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts index 0b3edf50b2..c4b1ebd0fa 100644 --- a/packages/runtime/src/templates/edge-shared/utils.ts +++ b/packages/runtime/src/templates/edge-shared/utils.ts @@ -96,7 +96,6 @@ export const buildResponse = async ({ request: Request context: Context }) => { - debugger result.response = updateModifiedHeaders(result.response) // They've returned the MiddlewareRequest directly, so we'll call `next()` for them. From 4526461578472ca43f49a0a0e2155db7538c04ee Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Fri, 6 Jan 2023 11:50:50 -0500 Subject: [PATCH 09/12] fix: add the modified middleware headers to the request, not response update the related tests --- .../src/templates/edge-shared/utils.test.ts | 41 +++++++++++-------- .../src/templates/edge-shared/utils.ts | 19 ++++----- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/packages/runtime/src/templates/edge-shared/utils.test.ts b/packages/runtime/src/templates/edge-shared/utils.test.ts index 2614beda6e..f515fa5839 100644 --- a/packages/runtime/src/templates/edge-shared/utils.test.ts +++ b/packages/runtime/src/templates/edge-shared/utils.test.ts @@ -9,19 +9,23 @@ describe('updateModifiedHeaders', () => { // been modified with 'x-middleware-request' added to it, this is more to confirm the test case mockHeaders.set('x-middleware-request-foo', 'bar') - const mockResult: FetchEventResult = { - response: new Response('', { headers: mockHeaders }), - waitUntil: Promise.resolve(), + const mockResponse = { + headers: mockHeaders } - mockResult.response = updateModifiedHeaders(mockResult.response) + const mockRequest = { + headers: new Headers() + } + + updateModifiedHeaders(mockRequest.headers, mockResponse.headers) - assertEquals(mockResult.response.headers.get('x-middleware-request-foo'), 'bar') + assertEquals(mockRequest.headers.get('x-middleware-request-foo'), null) }) describe("when the 'x-middleware-override-headers' headers is present", () => { let mockHeaders - let mockResult: FetchEventResult + let mockRequest: { headers: Headers } + let mockResponse: { headers: Headers } beforeEach(() => { mockHeaders = new Headers() @@ -30,28 +34,31 @@ describe('updateModifiedHeaders', () => { mockHeaders.set('x-middleware-request-test', '123') mockHeaders.set('x-middleware-override-headers', 'hello,test') - mockResult = { - response: new Response('', { headers: mockHeaders }), - waitUntil: Promise.resolve(), + mockRequest = { + headers: new Headers() + } + + mockResponse = { + headers: mockHeaders } - mockResult.response = updateModifiedHeaders(mockResult.response) + updateModifiedHeaders(mockRequest.headers, mockResponse.headers) }) - it("does not modify headers that are missing 'x-middleware-request' in the name", () => { - assertEquals(mockResult.response.headers.get('foo'), 'bar') + it("does not modify or add headers that are missing 'x-middleware-request' in the name", () => { + assertEquals(mockRequest.headers.get('foo'), null) }) it("removes 'x-middleware-request-' from headers", () => { - assertEquals(mockResult.response.headers.get('x-middleware-request-hello'), null) - assertEquals(mockResult.response.headers.get('x-middleware-request-test'), null) + assertEquals(mockRequest.headers.get('x-middleware-request-hello'), null) + assertEquals(mockRequest.headers.get('x-middleware-request-test'), null) - assertEquals(mockResult.response.headers.get('hello'), 'world') - assertEquals(mockResult.response.headers.get('test'), '123') + assertEquals(mockRequest.headers.get('hello'), 'world') + assertEquals(mockRequest.headers.get('test'), '123') }) it("removes 'x-middleware-override-headers' after cleaning headers", () => { - assertEquals(mockResult.response.headers.get('x-middleware-override-headers'), null) + assertEquals(mockRequest.headers.get('x-middleware-override-headers'), null) }) }) }) diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts index c4b1ebd0fa..394c018156 100644 --- a/packages/runtime/src/templates/edge-shared/utils.ts +++ b/packages/runtime/src/templates/edge-shared/utils.ts @@ -66,25 +66,23 @@ function isMiddlewareResponse(response: Response | MiddlewareResponse): response // Related Next.js code: // * https://github.com/vercel/next.js/blob/68d06fe015b28d8f81da52ca107a5f4bd72ab37c/packages/next/server/next-server.ts#L1918-L1928 // * https://github.com/vercel/next.js/blob/43c9d8940dc42337dd2f7d66aa90e6abf952278e/packages/next/server/web/spec-extension/response.ts#L10-L27 -export function updateModifiedHeaders(response: Response) { - const overriddenHeaders = response.headers.get('x-middleware-override-headers') +export function updateModifiedHeaders(requestHeaders: Headers, responseHeaders: Headers) { + const overriddenHeaders = responseHeaders.get('x-middleware-override-headers') if (!overriddenHeaders) { - return response + return } const headersToUpdate = overriddenHeaders.split(',').map((header) => header.trim()) for (const header of headersToUpdate) { const oldHeaderKey = 'x-middleware-request-' + header - const headerValue = response.headers.get(oldHeaderKey) || '' + const headerValue = responseHeaders.get(oldHeaderKey) || '' - response.headers.set(header, headerValue) - response.headers.delete(oldHeaderKey) + requestHeaders.set(header, headerValue) + responseHeaders.delete(oldHeaderKey) } - response.headers.delete('x-middleware-override-headers') - - return response + responseHeaders.delete('x-middleware-override-headers') } export const buildResponse = async ({ @@ -96,7 +94,8 @@ export const buildResponse = async ({ request: Request context: Context }) => { - result.response = updateModifiedHeaders(result.response) + debugger + updateModifiedHeaders(request.headers, result.response.headers) // They've returned the MiddlewareRequest directly, so we'll call `next()` for them. if (isMiddlewareRequest(result.response)) { From 93441ff302bd4945588ba5cc1825087ec12dafb8 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Fri, 6 Jan 2023 11:51:24 -0500 Subject: [PATCH 10/12] style: lint --- packages/runtime/src/templates/edge-shared/utils.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/runtime/src/templates/edge-shared/utils.test.ts b/packages/runtime/src/templates/edge-shared/utils.test.ts index f515fa5839..9c988f6816 100644 --- a/packages/runtime/src/templates/edge-shared/utils.test.ts +++ b/packages/runtime/src/templates/edge-shared/utils.test.ts @@ -10,11 +10,11 @@ describe('updateModifiedHeaders', () => { mockHeaders.set('x-middleware-request-foo', 'bar') const mockResponse = { - headers: mockHeaders + headers: mockHeaders, } const mockRequest = { - headers: new Headers() + headers: new Headers(), } updateModifiedHeaders(mockRequest.headers, mockResponse.headers) @@ -35,11 +35,11 @@ describe('updateModifiedHeaders', () => { mockHeaders.set('x-middleware-override-headers', 'hello,test') mockRequest = { - headers: new Headers() + headers: new Headers(), } mockResponse = { - headers: mockHeaders + headers: mockHeaders, } updateModifiedHeaders(mockRequest.headers, mockResponse.headers) From 5355a44a8f2b8c536affe153494b451dc4e24bdd Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Fri, 6 Jan 2023 12:40:58 -0500 Subject: [PATCH 11/12] fix: remove debugger --- packages/runtime/src/templates/edge-shared/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/runtime/src/templates/edge-shared/utils.ts b/packages/runtime/src/templates/edge-shared/utils.ts index 394c018156..1dc9153cc8 100644 --- a/packages/runtime/src/templates/edge-shared/utils.ts +++ b/packages/runtime/src/templates/edge-shared/utils.ts @@ -94,7 +94,6 @@ export const buildResponse = async ({ request: Request context: Context }) => { - debugger updateModifiedHeaders(request.headers, result.response.headers) // They've returned the MiddlewareRequest directly, so we'll call `next()` for them. From ccfc06da9561c227120b20dd3dd2ac3d23b8cf31 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Mon, 9 Jan 2023 08:59:44 -0500 Subject: [PATCH 12/12] test: move tests for request headers into standard --- cypress/integration/middleware/enhanced.spec.ts | 12 ------------ cypress/integration/middleware/standard.spec.ts | 12 ++++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cypress/integration/middleware/enhanced.spec.ts b/cypress/integration/middleware/enhanced.spec.ts index 738353e16a..4d5f1f0b87 100644 --- a/cypress/integration/middleware/enhanced.spec.ts +++ b/cypress/integration/middleware/enhanced.spec.ts @@ -1,16 +1,4 @@ describe('Enhanced middleware', () => { - it('adds request headers', () => { - cy.request('/api/hello').then((response) => { - expect(response.body).to.have.nested.property('headers.x-hello', 'world') - }) - }) - - it('adds request headers to a rewrite', () => { - cy.request('/headers').then((response) => { - expect(response.body).to.have.nested.property('headers.x-hello', 'world') - }) - }) - it('rewrites the response body', () => { cy.visit('/static') cy.get('#message').contains('This was static but has been transformed in') diff --git a/cypress/integration/middleware/standard.spec.ts b/cypress/integration/middleware/standard.spec.ts index f97392b165..97387b7ca0 100644 --- a/cypress/integration/middleware/standard.spec.ts +++ b/cypress/integration/middleware/standard.spec.ts @@ -1,4 +1,16 @@ describe('Standard middleware', () => { + it('adds request headers', () => { + cy.request('/api/hello').then((response) => { + expect(response.body).to.have.nested.property('headers.x-hello', 'world') + }) + }) + + it('adds request headers to a rewrite', () => { + cy.request('/headers').then((response) => { + expect(response.body).to.have.nested.property('headers.x-hello', 'world') + }) + }) + it('rewrites to internal page', () => { // preview mode is off by default cy.visit('/shows/rewriteme')