From 5a602c30af16a3d91272f03ebb53b3a49c57f728 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 7 Jan 2025 08:46:56 +0100 Subject: [PATCH 1/2] fix: adjust middleware json data rewrite to work with recent next@canary (#2734) * fix: keep __nextDataReq query param working * fix: don't attempt to keep old query param working * test: remove tests that don't make sense anymore and update some other ones --- edge-runtime/lib/response.ts | 16 ++++++++++------ tests/e2e/edge-middleware.test.ts | 17 +++++++++++++++++ tests/fixtures/middleware-pages/next-env.d.ts | 2 +- tests/fixtures/middleware-pages/next.config.js | 1 + tests/fixtures/middleware-pages/package.json | 4 +++- tests/fixtures/middleware-pages/tsconfig.json | 3 ++- tests/integration/edge-handler.test.ts | 11 ++++------- tests/integration/page-router.test.ts | 16 ---------------- tests/utils/create-e2e-fixture.ts | 1 + 9 files changed, 39 insertions(+), 32 deletions(-) diff --git a/edge-runtime/lib/response.ts b/edge-runtime/lib/response.ts index 8faf0d906a..ec0730e8ce 100644 --- a/edge-runtime/lib/response.ts +++ b/edge-runtime/lib/response.ts @@ -14,6 +14,8 @@ import { normalizeLocalePath, normalizeTrailingSlash, relativizeURL, + removeBasePath, + rewriteDataPath, } from './util.ts' export interface FetchEventResult { @@ -180,14 +182,16 @@ export const buildResponse = async ({ } if (isDataReq) { - // The rewrite target is a data request, but a middleware rewrite target is always for the page route, - // so we need to tell the server this is a data request. Setting the `x-nextjs-data` header is not enough. 🤷 - rewriteUrl.searchParams.set('__nextDataReq', '1') + rewriteUrl.pathname = rewriteDataPath({ + dataUrl: new URL(request.url).pathname, + newRoute: removeBasePath(rewriteUrl.pathname, nextConfig?.basePath), + basePath: nextConfig?.basePath, + }) + } else { + // respect trailing slash rules to prevent 308s + rewriteUrl.pathname = normalizeTrailingSlash(rewriteUrl.pathname, nextConfig?.trailingSlash) } - // respect trailing slash rules to prevent 308s - rewriteUrl.pathname = normalizeTrailingSlash(rewriteUrl.pathname, nextConfig?.trailingSlash) - const target = normalizeLocalizedTarget({ target: rewriteUrl.toString(), request, nextConfig }) if (target === request.url) { logger.withFields({ rewrite_url: rewrite }).debug('Rewrite url is same as original url') diff --git a/tests/e2e/edge-middleware.test.ts b/tests/e2e/edge-middleware.test.ts index 0eda11b710..8b32fbaa10 100644 --- a/tests/e2e/edge-middleware.test.ts +++ b/tests/e2e/edge-middleware.test.ts @@ -52,3 +52,20 @@ test('it should render OpenGraph image meta tag correctly', async ({ page, middl const size = await getImageSize(Buffer.from(imageBuffer), 'png') expect([size.width, size.height]).toEqual([1200, 630]) }) + +test('json data rewrite works', async ({ middlewarePages }) => { + const response = await fetch(`${middlewarePages.url}/_next/data/build-id/sha.json`, { + headers: { + 'x-nextjs-data': '1', + }, + }) + + expect(response.ok).toBe(true) + const body = await response.text() + + expect(body).toMatch(/^{"pageProps":/) + + const data = JSON.parse(body) + + expect(data.pageProps.message).toBeDefined() +}) diff --git a/tests/fixtures/middleware-pages/next-env.d.ts b/tests/fixtures/middleware-pages/next-env.d.ts index 4f11a03dc6..52e831b434 100644 --- a/tests/fixtures/middleware-pages/next-env.d.ts +++ b/tests/fixtures/middleware-pages/next-env.d.ts @@ -2,4 +2,4 @@ /// // NOTE: This file should not be edited -// see https://nextjs.org/docs/basic-features/typescript for more information. +// see https://nextjs.org/docs/pages/api-reference/config/typescript for more information. diff --git a/tests/fixtures/middleware-pages/next.config.js b/tests/fixtures/middleware-pages/next.config.js index 3cc07b88d1..c69795688a 100644 --- a/tests/fixtures/middleware-pages/next.config.js +++ b/tests/fixtures/middleware-pages/next.config.js @@ -21,6 +21,7 @@ if (platform === 'win32') { } } +/** @type {import('next').NextConfig} */ module.exports = { trailingSlash: true, output: 'standalone', diff --git a/tests/fixtures/middleware-pages/package.json b/tests/fixtures/middleware-pages/package.json index 5708c88b50..4f57aa1121 100644 --- a/tests/fixtures/middleware-pages/package.json +++ b/tests/fixtures/middleware-pages/package.json @@ -13,6 +13,8 @@ "react-dom": "18.2.0" }, "devDependencies": { - "@types/react": "18.2.47" + "@types/node": "^20.10.6", + "@types/react": "18.2.47", + "typescript": "^5.3.3" } } diff --git a/tests/fixtures/middleware-pages/tsconfig.json b/tests/fixtures/middleware-pages/tsconfig.json index 1c4f693a99..d88efa188d 100644 --- a/tests/fixtures/middleware-pages/tsconfig.json +++ b/tests/fixtures/middleware-pages/tsconfig.json @@ -11,7 +11,8 @@ "moduleResolution": "node", "resolveJsonModule": true, "isolatedModules": true, - "jsx": "preserve" + "jsx": "preserve", + "target": "ES2017" }, "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"], "exclude": ["node_modules"] diff --git a/tests/integration/edge-handler.test.ts b/tests/integration/edge-handler.test.ts index f938a1dc12..825ed6fac1 100644 --- a/tests/integration/edge-handler.test.ts +++ b/tests/integration/edge-handler.test.ts @@ -387,8 +387,7 @@ describe('page router', () => { }) const res = await response.json() const url = new URL(res.url, 'http://n/') - expect(url.pathname).toBe('/ssr-page-2/') - expect(url.searchParams.get('__nextDataReq')).toBe('1') + expect(url.pathname).toBe('/_next/data/build-id/ssr-page-2.json') expect(res.headers['x-nextjs-data']).toBe('1') expect(response.headers.get('x-nextjs-rewrite')).toBe('/ssr-page-2/') expect(response.status).toBe(200) @@ -420,7 +419,7 @@ describe('page router', () => { expect(response.status).toBe(200) }) - test('should rewrite un-rewritten data requests to page route', async (ctx) => { + test('should NOT rewrite un-rewritten data requests to page route', async (ctx) => { await createFixture('middleware-pages', ctx) await runPlugin(ctx) const origin = await LocalServer.run(async (req, res) => { @@ -443,8 +442,7 @@ describe('page router', () => { }) const res = await response.json() const url = new URL(res.url, 'http://n/') - expect(url.pathname).toBe('/ssg/hello/') - expect(url.searchParams.get('__nextDataReq')).toBe('1') + expect(url.pathname).toBe('/_next/data/build-id/ssg/hello.json') expect(res.headers['x-nextjs-data']).toBe('1') expect(response.status).toBe(200) }) @@ -472,8 +470,7 @@ describe('page router', () => { }) const res = await response.json() const url = new URL(res.url, 'http://n/') - expect(url.pathname).toBe('/blog/first/') - expect(url.searchParams.get('__nextDataReq')).toBe('1') + expect(url.pathname).toBe('/_next/data/build-id/blog/first.json') expect(url.searchParams.get('slug')).toBe('first') expect(res.headers['x-nextjs-data']).toBe('1') expect(response.status).toBe(200) diff --git a/tests/integration/page-router.test.ts b/tests/integration/page-router.test.ts index c246a47748..eb039eb79d 100644 --- a/tests/integration/page-router.test.ts +++ b/tests/integration/page-router.test.ts @@ -95,22 +95,6 @@ test('Should revalidate path with On-demand Revalidation', a expect(dateCacheInitial).not.toBe(dateCacheRevalidated) }) -test('Should return JSON for data req to page route', async (ctx) => { - await createFixture('page-router', ctx) - await runPlugin(ctx) - - const response = await invokeFunction(ctx, { - url: '/static/revalidate-manual?__nextDataReq=1', - headers: { 'x-nextjs-data': '1' }, - }) - - expect(response.body).toMatch(/^{"pageProps":/) - - const data = JSON.parse(response.body) - - expect(data.pageProps.show).toBeDefined() -}) - test.skipIf(platform === 'win32')( 'Should set permanent "netlify-cdn-cache-control" header on fully static pages"', async (ctx) => { diff --git a/tests/utils/create-e2e-fixture.ts b/tests/utils/create-e2e-fixture.ts index 097bd4ac0d..31cf3496e1 100644 --- a/tests/utils/create-e2e-fixture.ts +++ b/tests/utils/create-e2e-fixture.ts @@ -325,6 +325,7 @@ export const fixtureFactories = { bun: () => createE2EFixture('simple', { packageManger: 'bun' }), middleware: () => createE2EFixture('middleware'), middlewareOg: () => createE2EFixture('middleware-og'), + middlewarePages: () => createE2EFixture('middleware-pages'), pageRouter: () => createE2EFixture('page-router'), pageRouterBasePathI18n: () => createE2EFixture('page-router-base-path-i18n'), turborepo: () => From 7e932bcb118b505113c3713fa6f02ed36ac41961 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 7 Jan 2025 11:47:25 +0100 Subject: [PATCH 2/2] chore(main): release 5.9.3 (#2737) --- .release-please-manifest.json | 2 +- CHANGELOG.md | 7 +++++++ package-lock.json | 4 ++-- package.json | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 0eebf4fc35..d4c049982b 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "5.9.2" + ".": "5.9.3" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ab7cdf164..4054eb08a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## [5.9.3](https://github.com/opennextjs/opennextjs-netlify/compare/v5.9.2...v5.9.3) (2025-01-07) + + +### Bug Fixes + +* adjust middleware json data rewrite to work with recent next@canary ([#2734](https://github.com/opennextjs/opennextjs-netlify/issues/2734)) ([5a602c3](https://github.com/opennextjs/opennextjs-netlify/commit/5a602c30af16a3d91272f03ebb53b3a49c57f728)) + ## [5.9.2](https://github.com/opennextjs/opennextjs-netlify/compare/v5.9.1...v5.9.2) (2024-12-20) diff --git a/package-lock.json b/package-lock.json index f3a23902ff..b21ab6fcbe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@netlify/plugin-nextjs", - "version": "5.9.2", + "version": "5.9.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@netlify/plugin-nextjs", - "version": "5.9.2", + "version": "5.9.3", "license": "MIT", "devDependencies": { "@fastly/http-compute-js": "1.1.4", diff --git a/package.json b/package.json index 18bf026a07..127185738f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@netlify/plugin-nextjs", - "version": "5.9.2", + "version": "5.9.3", "description": "Run Next.js seamlessly on Netlify", "main": "./dist/index.js", "type": "module",