From f5bb340270f656069efb84fe2abc3b7a91f37844 Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 27 Dec 2024 18:18:15 +0100 Subject: [PATCH 1/3] fix: keep __nextDataReq query param working --- src/run/handlers/server.ts | 9 +++++++++ tests/e2e/edge-middleware.test.ts | 20 +++++++++++++++++++ tests/fixtures/middleware-pages/next-env.d.ts | 2 +- .../fixtures/middleware-pages/next.config.js | 1 + tests/fixtures/middleware-pages/package.json | 4 +++- tests/fixtures/middleware-pages/tsconfig.json | 3 ++- tests/utils/create-e2e-fixture.ts | 1 + 7 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/run/handlers/server.ts b/src/run/handlers/server.ts index 436411c812..51cfb0290d 100644 --- a/src/run/handlers/server.ts +++ b/src/run/handlers/server.ts @@ -83,6 +83,15 @@ export default async (request: Request) => { }, }) + if (new URL(request.url).searchParams.get('__nextDataReq')) { + const NEXT_REQUEST_META = Symbol.for('NextInternalRequestMeta') + // @ts-expect-error NEXT_REQUEST_META doesn't exist in IncomingMessage type + const meta = req[NEXT_REQUEST_META] ?? {} + meta.isNextDataReq = true + // @ts-expect-error NEXT_REQUEST_META doesn't exist in IncomingMessage type + req[NEXT_REQUEST_META] = meta + } + disableFaultyTransferEncodingHandling(res as unknown as ComputeJsOutgoingMessage) const requestContext = getRequestContext() ?? createRequestContext() diff --git a/tests/e2e/edge-middleware.test.ts b/tests/e2e/edge-middleware.test.ts index 0eda11b710..998eb8aff4 100644 --- a/tests/e2e/edge-middleware.test.ts +++ b/tests/e2e/edge-middleware.test.ts @@ -52,3 +52,23 @@ 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?__nextDataReq=1`, + { + 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/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 4289aa284f6eb58fa3ff9ab2633a16b9ab1f9444 Mon Sep 17 00:00:00 2001 From: pieh Date: Thu, 2 Jan 2025 19:32:59 +0100 Subject: [PATCH 2/3] fix: don't attempt to keep old query param working --- edge-runtime/lib/response.ts | 16 ++++++++++------ src/run/handlers/server.ts | 9 --------- tests/e2e/edge-middleware.test.ts | 11 ++++------- 3 files changed, 14 insertions(+), 22 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/src/run/handlers/server.ts b/src/run/handlers/server.ts index 51cfb0290d..436411c812 100644 --- a/src/run/handlers/server.ts +++ b/src/run/handlers/server.ts @@ -83,15 +83,6 @@ export default async (request: Request) => { }, }) - if (new URL(request.url).searchParams.get('__nextDataReq')) { - const NEXT_REQUEST_META = Symbol.for('NextInternalRequestMeta') - // @ts-expect-error NEXT_REQUEST_META doesn't exist in IncomingMessage type - const meta = req[NEXT_REQUEST_META] ?? {} - meta.isNextDataReq = true - // @ts-expect-error NEXT_REQUEST_META doesn't exist in IncomingMessage type - req[NEXT_REQUEST_META] = meta - } - disableFaultyTransferEncodingHandling(res as unknown as ComputeJsOutgoingMessage) const requestContext = getRequestContext() ?? createRequestContext() diff --git a/tests/e2e/edge-middleware.test.ts b/tests/e2e/edge-middleware.test.ts index 998eb8aff4..8b32fbaa10 100644 --- a/tests/e2e/edge-middleware.test.ts +++ b/tests/e2e/edge-middleware.test.ts @@ -54,14 +54,11 @@ test('it should render OpenGraph image meta tag correctly', async ({ page, middl }) test('json data rewrite works', async ({ middlewarePages }) => { - const response = await fetch( - `${middlewarePages.url}/_next/data/build-id/sha.json?__nextDataReq=1`, - { - headers: { - 'x-nextjs-data': '1', - }, + 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() From a1d859dfa6868a668a6b84e117467a18c748439b Mon Sep 17 00:00:00 2001 From: pieh Date: Fri, 3 Jan 2025 11:38:30 +0100 Subject: [PATCH 3/3] test: remove tests that don't make sense anymore and update some other ones --- tests/integration/edge-handler.test.ts | 11 ++++------- tests/integration/page-router.test.ts | 16 ---------------- 2 files changed, 4 insertions(+), 23 deletions(-) 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) => {