Skip to content

Commit 7af10ec

Browse files
piehmrstork
pieh
and
mrstork
authored
fix: don't 404 on prefetches to fully static pages router pages when middleware is present (#2957)
* test: expand middleware + json data e2e tests * fix: disable instead of removing middleware from lambda * test: make rewrite test case conditional on next.js version being used --------- Co-authored-by: Mateusz Bocian <[email protected]>
1 parent 39b0fd4 commit 7af10ec

19 files changed

+486
-14
lines changed

‎src/build/content/server.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { join as posixJoin, sep as posixSep } from 'node:path/posix'
1616
import { trace } from '@opentelemetry/api'
1717
import { wrapTracer } from '@opentelemetry/api/experimental'
1818
import glob from 'fast-glob'
19+
import type { MiddlewareManifest } from 'next/dist/build/webpack/plugins/middleware-plugin.js'
1920
import { prerelease, satisfies, lt as semverLowerThan, lte as semverLowerThanOrEqual } from 'semver'
2021

2122
import type { RunConfig } from '../../run/config.js'
@@ -324,23 +325,44 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> =>
324325
}
325326

326327
/**
327-
* Generates a copy of the middleware manifest without any middleware in it. We
328+
* Generates a copy of the middleware manifest that make all matchers never match on anything. We
328329
* do this because we'll run middleware in an edge function, and we don't want
329-
* to run it again in the server handler.
330+
* to run it again in the server handler. Additionally Next.js conditionally enable some handling
331+
* depending if there is a middleware present, so we need to keep reference to middleware in server
332+
* even if we don't actually want to ever run it there.
330333
*/
331334
const replaceMiddlewareManifest = async (sourcePath: string, destPath: string) => {
332335
await mkdir(dirname(destPath), { recursive: true })
333336

334337
const data = await readFile(sourcePath, 'utf8')
335-
const manifest = JSON.parse(data)
338+
const manifest = JSON.parse(data) as MiddlewareManifest
336339

337340
// TODO: Check for `manifest.version` and write an error to the system log
338341
// when we find a value that is not equal to 2. This will alert us in case
339342
// Next.js starts using a new format for the manifest and we're writing
340343
// one with the old version.
341344
const newManifest = {
342345
...manifest,
343-
middleware: {},
346+
middleware: Object.fromEntries(
347+
Object.entries(manifest.middleware).map(([key, edgeFunctionDefinition]) => {
348+
return [
349+
key,
350+
{
351+
...edgeFunctionDefinition,
352+
matchers: edgeFunctionDefinition.matchers.map((matcher) => {
353+
return {
354+
...matcher,
355+
// matcher that won't match on anything
356+
// this is meant to disable actually running middleware in the server handler,
357+
// while still allowing next server to enable some middleware specific handling
358+
// such as _next/data normalization ( https://github.com/vercel/next.js/blob/7bb72e508572237fe0d4aac5418546d4b4b3a363/packages/next/src/server/lib/router-utils/resolve-routes.ts#L395 )
359+
regexp: '(?!.*)',
360+
}
361+
}),
362+
},
363+
]
364+
}),
365+
),
344366
}
345367
const newData = JSON.stringify(newManifest)
346368

‎tests/e2e/edge-middleware.test.ts

Lines changed: 137 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'
33
import { test } from '../utils/playwright-helpers.js'
44
import { getImageSize } from 'next/dist/server/image-optimizer.js'
55

6+
type ExtendedWindow = Window & {
7+
didReload?: boolean
8+
}
9+
610
test('Runs edge middleware', async ({ page, middleware }) => {
711
await page.goto(`${middleware.url}/test/redirect`)
812

@@ -53,21 +57,144 @@ test('it should render OpenGraph image meta tag correctly', async ({ page, middl
5357
expect([size.width, size.height]).toEqual([1200, 630])
5458
})
5559

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+
test.describe('json data', () => {
61+
const testConfigs = [
62+
{
63+
describeLabel: 'NextResponse.next() -> getServerSideProps page',
64+
selector: 'NextResponse.next()#getServerSideProps',
65+
jsonPathMatcher: '/link/next-getserversideprops.json',
6066
},
61-
})
67+
{
68+
describeLabel: 'NextResponse.next() -> getStaticProps page',
69+
selector: 'NextResponse.next()#getStaticProps',
70+
jsonPathMatcher: '/link/next-getstaticprops.json',
71+
},
72+
{
73+
describeLabel: 'NextResponse.next() -> fully static page',
74+
selector: 'NextResponse.next()#fullyStatic',
75+
jsonPathMatcher: '/link/next-fullystatic.json',
76+
},
77+
{
78+
describeLabel: 'NextResponse.rewrite() -> getServerSideProps page',
79+
selector: 'NextResponse.rewrite()#getServerSideProps',
80+
jsonPathMatcher: '/link/rewrite-me-getserversideprops.json',
81+
},
82+
{
83+
describeLabel: 'NextResponse.rewrite() -> getStaticProps page',
84+
selector: 'NextResponse.rewrite()#getStaticProps',
85+
jsonPathMatcher: '/link/rewrite-me-getstaticprops.json',
86+
},
87+
]
88+
89+
// Linking to static pages reloads on rewrite for versions below 14
90+
if (nextVersionSatisfies('>=14.0.0')) {
91+
testConfigs.push({
92+
describeLabel: 'NextResponse.rewrite() -> fully static page',
93+
selector: 'NextResponse.rewrite()#fullyStatic',
94+
jsonPathMatcher: '/link/rewrite-me-fullystatic.json',
95+
})
96+
}
97+
98+
test.describe('no 18n', () => {
99+
for (const testConfig of testConfigs) {
100+
test.describe(testConfig.describeLabel, () => {
101+
test('json data fetch', async ({ middlewarePages, page }) => {
102+
const dataFetchPromise = new Promise<Response>((resolve) => {
103+
page.on('response', (response) => {
104+
if (response.url().includes(testConfig.jsonPathMatcher)) {
105+
resolve(response)
106+
}
107+
})
108+
})
109+
110+
await page.goto(`${middlewarePages.url}/link`)
111+
112+
await page.hover(`[data-link="${testConfig.selector}"]`)
62113

63-
expect(response.ok).toBe(true)
64-
const body = await response.text()
114+
const dataResponse = await dataFetchPromise
65115

66-
expect(body).toMatch(/^{"pageProps":/)
116+
expect(dataResponse.ok()).toBe(true)
117+
})
67118

68-
const data = JSON.parse(body)
119+
test('navigation', async ({ middlewarePages, page }) => {
120+
await page.goto(`${middlewarePages.url}/link`)
69121

70-
expect(data.pageProps.message).toBeDefined()
122+
await page.evaluate(() => {
123+
// set some value to window to check later if browser did reload and lost this state
124+
;(window as ExtendedWindow).didReload = false
125+
})
126+
127+
await page.click(`[data-link="${testConfig.selector}"]`)
128+
129+
// wait for page to be rendered
130+
await page.waitForSelector(`[data-page="${testConfig.selector}"]`)
131+
132+
// check if browser navigation worked by checking if state was preserved
133+
const browserNavigationWorked =
134+
(await page.evaluate(() => {
135+
return (window as ExtendedWindow).didReload
136+
})) === false
137+
138+
// we expect client navigation to work without browser reload
139+
expect(browserNavigationWorked).toBe(true)
140+
})
141+
})
142+
}
143+
})
144+
test.describe('with 18n', () => {
145+
for (const testConfig of testConfigs) {
146+
test.describe(testConfig.describeLabel, () => {
147+
for (const { localeLabel, pageWithLinksPathname } of [
148+
{ localeLabel: 'implicit default locale', pageWithLinksPathname: '/link' },
149+
{ localeLabel: 'explicit default locale', pageWithLinksPathname: '/en/link' },
150+
{ localeLabel: 'explicit non-default locale', pageWithLinksPathname: '/fr/link' },
151+
]) {
152+
test.describe(localeLabel, () => {
153+
test('json data fetch', async ({ middlewareI18n, page }) => {
154+
const dataFetchPromise = new Promise<Response>((resolve) => {
155+
page.on('response', (response) => {
156+
if (response.url().includes(testConfig.jsonPathMatcher)) {
157+
resolve(response)
158+
}
159+
})
160+
})
161+
162+
await page.goto(`${middlewareI18n.url}${pageWithLinksPathname}`)
163+
164+
await page.hover(`[data-link="${testConfig.selector}"]`)
165+
166+
const dataResponse = await dataFetchPromise
167+
168+
expect(dataResponse.ok()).toBe(true)
169+
})
170+
171+
test('navigation', async ({ middlewareI18n, page }) => {
172+
await page.goto(`${middlewareI18n.url}${pageWithLinksPathname}`)
173+
174+
await page.evaluate(() => {
175+
// set some value to window to check later if browser did reload and lost this state
176+
;(window as ExtendedWindow).didReload = false
177+
})
178+
179+
await page.click(`[data-link="${testConfig.selector}"]`)
180+
181+
// wait for page to be rendered
182+
await page.waitForSelector(`[data-page="${testConfig.selector}"]`)
183+
184+
// check if browser navigation worked by checking if state was preserved
185+
const browserNavigationWorked =
186+
(await page.evaluate(() => {
187+
return (window as ExtendedWindow).didReload
188+
})) === false
189+
190+
// we expect client navigation to work without browser reload
191+
expect(browserNavigationWorked).toBe(true)
192+
})
193+
})
194+
}
195+
})
196+
}
197+
})
71198
})
72199

73200
// those tests use `fetch` instead of `page.goto` intentionally to avoid potential client rendering

‎tests/fixtures/middleware-i18n/middleware.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,26 @@ export async function middleware(request) {
88
return NextResponse.next()
99
}
1010

11+
if (url.pathname.startsWith('/link/next')) {
12+
return NextResponse.next({
13+
headers: {
14+
'x-middleware-test': 'link-next',
15+
},
16+
})
17+
}
18+
19+
if (url.pathname.startsWith('/link/rewrite-me')) {
20+
const rewriteUrl = new URL(
21+
url.pathname.replace('/link/rewrite-me', '/link/rewrite-target'),
22+
url,
23+
)
24+
return NextResponse.rewrite(rewriteUrl, {
25+
headers: {
26+
'x-middleware-test': 'link-rewrite',
27+
},
28+
})
29+
}
30+
1131
if (url.pathname === '/old-home') {
1232
if (url.searchParams.get('override') === 'external') {
1333
return Response.redirect('https://example.vercel.sh')

‎tests/fixtures/middleware-i18n/pages/link/index.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import Link from 'next/link'
2+
3+
export default function Page() {
4+
return (
5+
<div>
6+
<h1>Page with Links</h1>
7+
<ul>
8+
<li>
9+
NextResponse.next()
10+
<ul>
11+
<li>
12+
<Link
13+
href="/link/next-getserversideprops"
14+
data-link="NextResponse.next()#getServerSideProps"
15+
>
16+
getServerSideProps
17+
</Link>
18+
</li>
19+
20+
<li>
21+
<Link href="/link/next-getstaticprops" data-link="NextResponse.next()#getStaticProps">
22+
getStaticProps
23+
</Link>
24+
</li>
25+
26+
<li>
27+
<Link href="/link/next-fullystatic" data-link="NextResponse.next()#fullyStatic">
28+
fullyStatic
29+
</Link>
30+
</li>
31+
</ul>
32+
</li>
33+
<li>
34+
NextResponse.rewrite()
35+
<ul>
36+
<li>
37+
<Link
38+
href="/link/rewrite-me-getserversideprops"
39+
data-link="NextResponse.rewrite()#getServerSideProps"
40+
>
41+
getServerSideProps
42+
</Link>
43+
</li>
44+
45+
<li>
46+
<Link
47+
href="/link/rewrite-me-getstaticprops"
48+
data-link="NextResponse.rewrite()#getStaticProps"
49+
>
50+
getStaticProps
51+
</Link>
52+
</li>
53+
54+
<li>
55+
<Link
56+
href="/link/rewrite-me-fullystatic"
57+
data-link="NextResponse.rewrite()#fullyStatic"
58+
>
59+
fullyStatic
60+
</Link>
61+
</li>
62+
</ul>
63+
</li>
64+
</ul>
65+
</div>
66+
)
67+
}

‎tests/fixtures/middleware-i18n/pages/link/next-fullystatic.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Page() {
2+
return (
3+
<div data-page="NextResponse.next()#fullyStatic">
4+
<h1>fully static page</h1>
5+
</div>
6+
)
7+
}

‎tests/fixtures/middleware-i18n/pages/link/next-getserversideprops.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export default function Page() {
2+
return (
3+
<div data-page="NextResponse.next()#getServerSideProps">
4+
<h1>
5+
<code>getServerSideProps</code> page
6+
</h1>
7+
</div>
8+
)
9+
}
10+
11+
export function getServerSideProps() {
12+
return {
13+
props: {},
14+
}
15+
}

‎tests/fixtures/middleware-i18n/pages/link/next-getstaticprops.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export default function Page() {
2+
return (
3+
<div data-page="NextResponse.next()#getStaticProps">
4+
<h1>
5+
<code>getStaticProps</code> page
6+
</h1>
7+
</div>
8+
)
9+
}
10+
11+
export function getStaticProps() {
12+
return {
13+
props: {},
14+
}
15+
}

‎tests/fixtures/middleware-i18n/pages/link/rewrite-target-fullystatic.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Page() {
2+
return (
3+
<div data-page="NextResponse.rewrite()#fullyStatic">
4+
<h1>fully static page</h1>
5+
</div>
6+
)
7+
}

‎tests/fixtures/middleware-i18n/pages/link/rewrite-target-getserversideprops.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export default function Page() {
2+
return (
3+
<div data-page="NextResponse.rewrite()#getServerSideProps">
4+
<h1>
5+
<code>getServerSideProps</code> page
6+
</h1>
7+
</div>
8+
)
9+
}
10+
11+
export function getServerSideProps() {
12+
return {
13+
props: {},
14+
}
15+
}

‎tests/fixtures/middleware-i18n/pages/link/rewrite-target-getstaticprops.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export default function Page() {
2+
return (
3+
<div data-page="NextResponse.rewrite()#getStaticProps">
4+
<h1>
5+
<code>getStaticProps</code> page
6+
</h1>
7+
</div>
8+
)
9+
}
10+
11+
export function getStaticProps() {
12+
return {
13+
props: {},
14+
}
15+
}

0 commit comments

Comments
 (0)