Skip to content

Commit 5a602c3

Browse files
authored
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
1 parent c3e328c commit 5a602c3

File tree

9 files changed

+38
-31
lines changed

9 files changed

+38
-31
lines changed

edge-runtime/lib/response.ts

+9-5
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -14,6 +14,8 @@ import {
14
normalizeLocalePath,
14
normalizeLocalePath,
15
normalizeTrailingSlash,
15
normalizeTrailingSlash,
16
relativizeURL,
16
relativizeURL,
17+
removeBasePath,
18+
rewriteDataPath,
17
} from './util.ts'
19
} from './util.ts'
18

20

19
export interface FetchEventResult {
21
export interface FetchEventResult {
@@ -180,13 +182,15 @@ export const buildResponse = async ({
180
}
182
}
181

183

182
if (isDataReq) {
184
if (isDataReq) {
183-
// The rewrite target is a data request, but a middleware rewrite target is always for the page route,
185+
rewriteUrl.pathname = rewriteDataPath({
184-
// so we need to tell the server this is a data request. Setting the `x-nextjs-data` header is not enough. 🤷
186+
dataUrl: new URL(request.url).pathname,
185-
rewriteUrl.searchParams.set('__nextDataReq', '1')
187+
newRoute: removeBasePath(rewriteUrl.pathname, nextConfig?.basePath),
186-
}
188+
basePath: nextConfig?.basePath,
187-
189+
})
190+
} else {
188
// respect trailing slash rules to prevent 308s
191
// respect trailing slash rules to prevent 308s
189
rewriteUrl.pathname = normalizeTrailingSlash(rewriteUrl.pathname, nextConfig?.trailingSlash)
192
rewriteUrl.pathname = normalizeTrailingSlash(rewriteUrl.pathname, nextConfig?.trailingSlash)
193+
}
190

194

191
const target = normalizeLocalizedTarget({ target: rewriteUrl.toString(), request, nextConfig })
195
const target = normalizeLocalizedTarget({ target: rewriteUrl.toString(), request, nextConfig })
192
if (target === request.url) {
196
if (target === request.url) {

tests/e2e/edge-middleware.test.ts

+17
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -52,3 +52,20 @@ test('it should render OpenGraph image meta tag correctly', async ({ page, middl
52
const size = await getImageSize(Buffer.from(imageBuffer), 'png')
52
const size = await getImageSize(Buffer.from(imageBuffer), 'png')
53
expect([size.width, size.height]).toEqual([1200, 630])
53
expect([size.width, size.height]).toEqual([1200, 630])
54
})
54
})
55+
56+
test('json data rewrite works', async ({ middlewarePages }) => {
57+
const response = await fetch(`${middlewarePages.url}/_next/data/build-id/sha.json`, {
58+
headers: {
59+
'x-nextjs-data': '1',
60+
},
61+
})
62+
63+
expect(response.ok).toBe(true)
64+
const body = await response.text()
65+
66+
expect(body).toMatch(/^{"pageProps":/)
67+
68+
const data = JSON.parse(body)
69+
70+
expect(data.pageProps.message).toBeDefined()
71+
})

tests/fixtures/middleware-pages/next-env.d.ts

+1-1
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -2,4 +2,4 @@
2
/// <reference types="next/image-types/global" />
2
/// <reference types="next/image-types/global" />
3

3

4
// NOTE: This file should not be edited
4
// NOTE: This file should not be edited
5-
// see https://nextjs.org/docs/basic-features/typescript for more information.
5+
// see https://nextjs.org/docs/pages/api-reference/config/typescript for more information.

tests/fixtures/middleware-pages/next.config.js

+1
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -21,6 +21,7 @@ if (platform === 'win32') {
21
}
21
}
22
}
22
}
23

23

24+
/** @type {import('next').NextConfig} */
24
module.exports = {
25
module.exports = {
25
trailingSlash: true,
26
trailingSlash: true,
26
output: 'standalone',
27
output: 'standalone',

tests/fixtures/middleware-pages/package.json

+3-1
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -13,6 +13,8 @@
13
"react-dom": "18.2.0"
13
"react-dom": "18.2.0"
14
},
14
},
15
"devDependencies": {
15
"devDependencies": {
16-
"@types/react": "18.2.47"
16+
"@types/node": "^20.10.6",
17+
"@types/react": "18.2.47",
18+
"typescript": "^5.3.3"
17
}
19
}
18
}
20
}

tests/fixtures/middleware-pages/tsconfig.json

+2-1
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -11,7 +11,8 @@
11
"moduleResolution": "node",
11
"moduleResolution": "node",
12
"resolveJsonModule": true,
12
"resolveJsonModule": true,
13
"isolatedModules": true,
13
"isolatedModules": true,
14-
"jsx": "preserve"
14+
"jsx": "preserve",
15+
"target": "ES2017"
15
},
16
},
16
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
17
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
17
"exclude": ["node_modules"]
18
"exclude": ["node_modules"]

tests/integration/edge-handler.test.ts

+4-7
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -387,8 +387,7 @@ describe('page router', () => {
387
})
387
})
388
const res = await response.json()
388
const res = await response.json()
389
const url = new URL(res.url, 'http://n/')
389
const url = new URL(res.url, 'http://n/')
390-
expect(url.pathname).toBe('/ssr-page-2/')
390+
expect(url.pathname).toBe('/_next/data/build-id/ssr-page-2.json')
391-
expect(url.searchParams.get('__nextDataReq')).toBe('1')
392
expect(res.headers['x-nextjs-data']).toBe('1')
391
expect(res.headers['x-nextjs-data']).toBe('1')
393
expect(response.headers.get('x-nextjs-rewrite')).toBe('/ssr-page-2/')
392
expect(response.headers.get('x-nextjs-rewrite')).toBe('/ssr-page-2/')
394
expect(response.status).toBe(200)
393
expect(response.status).toBe(200)
@@ -420,7 +419,7 @@ describe('page router', () => {
420
expect(response.status).toBe(200)
419
expect(response.status).toBe(200)
421
})
420
})
422

421

423-
test<FixtureTestContext>('should rewrite un-rewritten data requests to page route', async (ctx) => {
422+
test<FixtureTestContext>('should NOT rewrite un-rewritten data requests to page route', async (ctx) => {
424
await createFixture('middleware-pages', ctx)
423
await createFixture('middleware-pages', ctx)
425
await runPlugin(ctx)
424
await runPlugin(ctx)
426
const origin = await LocalServer.run(async (req, res) => {
425
const origin = await LocalServer.run(async (req, res) => {
@@ -443,8 +442,7 @@ describe('page router', () => {
443
})
442
})
444
const res = await response.json()
443
const res = await response.json()
445
const url = new URL(res.url, 'http://n/')
444
const url = new URL(res.url, 'http://n/')
446-
expect(url.pathname).toBe('/ssg/hello/')
445+
expect(url.pathname).toBe('/_next/data/build-id/ssg/hello.json')
447-
expect(url.searchParams.get('__nextDataReq')).toBe('1')
448
expect(res.headers['x-nextjs-data']).toBe('1')
446
expect(res.headers['x-nextjs-data']).toBe('1')
449
expect(response.status).toBe(200)
447
expect(response.status).toBe(200)
450
})
448
})
@@ -472,8 +470,7 @@ describe('page router', () => {
472
})
470
})
473
const res = await response.json()
471
const res = await response.json()
474
const url = new URL(res.url, 'http://n/')
472
const url = new URL(res.url, 'http://n/')
475-
expect(url.pathname).toBe('/blog/first/')
473+
expect(url.pathname).toBe('/_next/data/build-id/blog/first.json')
476-
expect(url.searchParams.get('__nextDataReq')).toBe('1')
477
expect(url.searchParams.get('slug')).toBe('first')
474
expect(url.searchParams.get('slug')).toBe('first')
478
expect(res.headers['x-nextjs-data']).toBe('1')
475
expect(res.headers['x-nextjs-data']).toBe('1')
479
expect(response.status).toBe(200)
476
expect(response.status).toBe(200)

tests/integration/page-router.test.ts

-16
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -95,22 +95,6 @@ test<FixtureTestContext>('Should revalidate path with On-demand Revalidation', a
95
expect(dateCacheInitial).not.toBe(dateCacheRevalidated)
95
expect(dateCacheInitial).not.toBe(dateCacheRevalidated)
96
})
96
})
97

97

98-
test<FixtureTestContext>('Should return JSON for data req to page route', async (ctx) => {
99-
await createFixture('page-router', ctx)
100-
await runPlugin(ctx)
101-
102-
const response = await invokeFunction(ctx, {
103-
url: '/static/revalidate-manual?__nextDataReq=1',
104-
headers: { 'x-nextjs-data': '1' },
105-
})
106-
107-
expect(response.body).toMatch(/^{"pageProps":/)
108-
109-
const data = JSON.parse(response.body)
110-
111-
expect(data.pageProps.show).toBeDefined()
112-
})
113-
114
test.skipIf(platform === 'win32')<FixtureTestContext>(
98
test.skipIf(platform === 'win32')<FixtureTestContext>(
115
'Should set permanent "netlify-cdn-cache-control" header on fully static pages"',
99
'Should set permanent "netlify-cdn-cache-control" header on fully static pages"',
116
async (ctx) => {
100
async (ctx) => {

tests/utils/create-e2e-fixture.ts

+1
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -325,6 +325,7 @@ export const fixtureFactories = {
325
bun: () => createE2EFixture('simple', { packageManger: 'bun' }),
325
bun: () => createE2EFixture('simple', { packageManger: 'bun' }),
326
middleware: () => createE2EFixture('middleware'),
326
middleware: () => createE2EFixture('middleware'),
327
middlewareOg: () => createE2EFixture('middleware-og'),
327
middlewareOg: () => createE2EFixture('middleware-og'),
328+
middlewarePages: () => createE2EFixture('middleware-pages'),
328
pageRouter: () => createE2EFixture('page-router'),
329
pageRouter: () => createE2EFixture('page-router'),
329
pageRouterBasePathI18n: () => createE2EFixture('page-router-base-path-i18n'),
330
pageRouterBasePathI18n: () => createE2EFixture('page-router-base-path-i18n'),
330
turborepo: () =>
331
turborepo: () =>

0 commit comments

Comments
 (0)