fix: set x-deployment-id to every middleware prefetch request #71193

abvthecity · 2024-10-12T14:24:49Z

What?

x-deployment-id is missing on prefetch requests

Why?

To handle version skew on the pages router. When version skew is turned on, x-deployment-id should be used on prefetch otherwise the server will return 404. But with middleware enabled, the 404 does not appear to hard-refresh the page.

How?

next.js seems to be relying on the 404 to perform a hard navigation and auto-refresh the client. But that doesn't appear to happen when middleware is in use (separate issue, not addressed in this PR), which is causing the client to navigate to 404 whenever we deploy a new version of the site to vercel.

instead of sending the client to 404, passing the x-deployment-id helps guarantee that next-data requests respect version skew protection.

ijjk · 2024-10-12T14:25:12Z

Allow CI Workflow Run

approve CI run for commit: fce1c6d

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

ztanner · 2024-10-17T19:17:32Z

packages/next/src/shared/lib/router/router.ts

+        isPrefetch && hasMiddleware
+          ? {
+              'x-middleware-prefetch': '1',
+              'x-deployment-id': process.env.NEXT_DEPLOYMENT_ID,


I think we should just ensure this header is always passed for requests, even if they're not prefetch / middleware is present.

Just bumping this, if you're able to make this change we could re-review!

Thanks for bumping! Updated the PR 👍

packages/next/src/shared/lib/router/router.ts

Co-authored-by: JJ Kasper <jj@jjsweb.site>

ijjk

Looks like lint fixes just need applied can run pnpm lint-fix for this. Seems we can't as it's a PR from an organization which doesn't allow us to push.

ijjk · 2024-12-18T08:17:14Z

Failing test suites

Commit: c2a77f1

pnpm test-start test/production/deployment-id-handling/deployment-id-handling.test.ts

deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID > should contain deployment id in prefetch request
deployment-id-handling enabled with NEXT_DEPLOYMENT_ID > should contain deployment id in prefetch request

Expand output

● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should contain deployment id in prefetch request

expect(received).toBeGreaterThan(expected)

Expected: > 0
Received:   0

  100 |       await browser.elementById('edge-link').click()
  101 |
> 102 |       expect(dataHeaders.length).toBeGreaterThan(0)
      |                                  ^
  103 |
  104 |       expect(
  105 |         dataHeaders.every(

  at Object.toBeGreaterThan (production/deployment-id-handling/deployment-id-handling.test.ts:102:34)

● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should contain deployment id in prefetch request

expect(received).toBeGreaterThan(expected)

Expected: > 0
Received:   0

  100 |       await browser.elementById('edge-link').click()
  101 |
> 102 |       expect(dataHeaders.length).toBeGreaterThan(0)
      |                                  ^
  103 |
  104 |       expect(
  105 |         dataHeaders.every(

  at Object.toBeGreaterThan (production/deployment-id-handling/deployment-id-handling.test.ts:102:34)

Read more about building and testing Next.js in contributing.md.

pnpm test test/integration/amp-export-validation/test/index.test.js (turbopack)

AMP Validation on Export > production mode > should have shown errors during build

Expand output

● AMP Validation on Export › production mode › should have shown errors during build

expect(received).toMatch(expected)

Expected pattern: /error.*The mandatory attribute 'height' is missing in tag 'amp-video'\./
Received string:  "   Loading config from /root/actions-runner/_work/next.js/next.js/test/integration/amp-export-validation/next.config.js
   Loading config from /root/actions-runner/_work/next.js/next.js/test/integration/amp-export-validation/next.config.js
   ▲ Next.js 15.1.1-canary.12 (Turbopack)·
   Checking validity of types ...
   Creating an optimized production build ...
   Loading config from /root/actions-runner/_work/next.js/next.js/test/integration/amp-export-validation/next.config.js
   Building (0/7) ...
   Building (1/7)··
   Building (3/7)··
   Building (5/7)··
 ✓ Building (7/7)
 ✓ Compiled successfully in 6s
   Collecting page data ...
   Generating static pages (0/8) ...
   Generating static pages (2/8)··
 ⚠ Linting is disabled.
Error occurred prerendering page \"/third\". Read more: https://nextjs.org/docs/messages/prerender-error
AssertionError: Assertion failed: WebAssembly is uninitialized

  at new module$contents$goog$asserts_AssertionError (../evalmachine.<anonymous>:102:1441)
  at module$contents$goog$asserts_doAssertFailure (../evalmachine.<anonymous>:103:354)
  at goog.asserts.assertExists (../evalmachine.<anonymous>:104:374)
  at Object.module$contents$amp$validator_validateString [as validateString] (../evalmachine.<anonymous>:2238:108)
  at Validator.validateString (../packages/next/dist/compiled/amphtml-validator/index.js:1:20650)
  at validateAmp (../packages/next/dist/export/routes/pages.js:106:34)
  at async exportPagesPage (../packages/next/dist/export/routes/pages.js:140:17)
  at async Span.traceAsyncFn (../packages/next/dist/trace/trace.js:156:20)
  at async exportPage (../packages/next/dist/export/worker.js:337:18)
  Export encountered an error on /third, exiting the build.
   ⨯ Static worker exited with code: 1 and signal: null
  "
  at Object.toMatch (integration/amp-export-validation/test/index.test.js:28:29)

Read more about building and testing Next.js in contributing.md.

ijjk · 2024-12-18T08:21:18Z

Stats from current PR

Default Build

General

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
buildDuration	18s	18.5s	⚠️ +479ms
buildDurationCached	15.1s	12.1s	N/A
nodeModulesSize	416 MB	410 MB	N/A
nextStartRea..uration (ms)	448ms	453ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
1187-HASH.js gzip	52.4 kB	51.1 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.36 kB	5.36 kB	N/A
bccd1874-HASH.js gzip	53 kB	53 kB	N/A
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	232 B	235 B	N/A
main-HASH.js gzip	34.1 kB	34 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	0 B	0 B	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.49 kB	4.49 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.34 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
_buildManifest.js gzip	749 B	746 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
index.html gzip	522 B	522 B	✓
link.html gzip	538 B	536 B	N/A
withRouter.html gzip	519 B	519 B	✓
Overall change	1.04 kB	1.04 kB	✓

Edge SSR bundle Size

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
edge-ssr.js gzip	129 kB	128 kB	N/A
page.js gzip	205 kB	204 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
middleware-b..fest.js gzip	669 B	667 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.3 kB	31.2 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
274-experime...dev.js gzip	322 B	322 B	✓
274.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	356 kB	323 kB	N/A
app-page-exp..prod.js gzip	129 kB	127 kB	N/A
app-page-tur..prod.js gzip	142 kB	140 kB	N/A
app-page-tur..prod.js gzip	137 kB	135 kB	N/A
app-page.run...dev.js gzip	346 kB	314 kB	N/A
app-page.run..prod.js gzip	125 kB	123 kB	N/A
app-route-ex...dev.js gzip	37.5 kB	37.4 kB	N/A
app-route-ex..prod.js gzip	25.5 kB	25.5 kB	N/A
app-route-tu..prod.js gzip	25.5 kB	25.5 kB	N/A
app-route-tu..prod.js gzip	25.3 kB	25.3 kB	N/A
app-route.ru...dev.js gzip	39.1 kB	39 kB	N/A
app-route.ru..prod.js gzip	25.3 kB	25.3 kB	N/A
pages-api-tu..prod.js gzip	9.69 kB	9.69 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.68 kB	9.68 kB	✓
pages-turbo...prod.js gzip	21.7 kB	21.7 kB	N/A
pages.runtim...dev.js gzip	27.5 kB	27.4 kB	N/A
pages.runtim..prod.js gzip	21.7 kB	21.7 kB	N/A
server.runti..prod.js gzip	916 kB	916 kB	N/A
Overall change	31.6 kB	31.6 kB	✓

build cache

	vercel/next.js canary	fern-api/next.js ajiang/fix-x-deployment-id-prefetch	Change
0.pack gzip	2.08 MB	2.06 MB	N/A
index.pack gzip	74 kB	72 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 1187-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

failed to diff

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

failed to diff

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

failed to diff

Commit: fce1c6d

abvthecity · 2024-12-18T12:18:24Z

No problem, ran prettier-lint again, and also fixed the test so that it actually forces the router to make a nextjs-data request.

fix: set x-deployment-id to every prefetch request

41f5d9c

ijjk added tests type: next labels Oct 12, 2024

abvthecity added 2 commits October 12, 2024 10:58

isolate setting x-deployment-id to middleware-only

0f86036

add middleware to test

f10807a

abvthecity changed the title ~~fix: set x-deployment-id to every prefetch request~~ fix: set x-deployment-id to every middleware prefetch request Oct 12, 2024

abvthecity mentioned this pull request Oct 12, 2024

fix: enable hard navigate on 404 #71198

Open

ztanner reviewed Oct 17, 2024

View reviewed changes

abvthecity and others added 3 commits December 17, 2024 12:14

fix: assign x-deployment-id to all next data fetch requests

aaa77da

Merge branch 'canary' into ajiang/fix-x-deployment-id-prefetch

a7f05e1

lint-fix

c496c85

ijjk reviewed Dec 17, 2024

View reviewed changes

packages/next/src/shared/lib/router/router.ts Outdated Show resolved Hide resolved

Update router.ts

c2a77f1

Co-authored-by: JJ Kasper <jj@jjsweb.site>

ijjk reviewed Dec 18, 2024

View reviewed changes

abvthecity added 2 commits December 18, 2024 06:47

lint-fix

bab0894

fix: update test with getServerSideProps

fce1c6d

abvthecity requested a review from ijjk December 18, 2024 14:32

ijjk approved these changes Dec 18, 2024

View reviewed changes

ijjk enabled auto-merge (squash) December 18, 2024 16:22

ijjk merged commit d31b8b8 into vercel:canary Dec 18, 2024
125 checks passed

abvthecity deleted the ajiang/fix-x-deployment-id-prefetch branch December 19, 2024 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: set x-deployment-id to every middleware prefetch request #71193

fix: set x-deployment-id to every middleware prefetch request #71193

abvthecity commented Oct 12, 2024 •

edited

Loading

ijjk commented Oct 12, 2024 •

edited

Loading

ztanner Oct 17, 2024

ijjk Dec 17, 2024

abvthecity Dec 17, 2024

ijjk left a comment

ijjk commented Dec 18, 2024 •

edited

Loading

ijjk commented Dec 18, 2024 •

edited

Loading

abvthecity commented Dec 18, 2024

fix: set x-deployment-id to every middleware prefetch request #71193

fix: set x-deployment-id to every middleware prefetch request #71193

Conversation

abvthecity commented Oct 12, 2024 • edited Loading

What?

Why?

How?

ijjk commented Oct 12, 2024 • edited Loading

ztanner Oct 17, 2024

Choose a reason for hiding this comment

ijjk Dec 17, 2024

Choose a reason for hiding this comment

abvthecity Dec 17, 2024

Choose a reason for hiding this comment

ijjk left a comment

Choose a reason for hiding this comment

ijjk commented Dec 18, 2024 • edited Loading

Failing test suites

ijjk commented Dec 18, 2024 • edited Loading

Stats from current PR

abvthecity commented Dec 18, 2024

abvthecity commented Oct 12, 2024 •

edited

Loading

ijjk commented Oct 12, 2024 •

edited

Loading

ijjk commented Dec 18, 2024 •

edited

Loading

ijjk commented Dec 18, 2024 •

edited

Loading