fix(headers): align draft mode cookie attributes with Next.js#992
Conversation
commit: |
Draft mode cookies currently use SameSite=Lax in production and clear with Max-Age=0. That diverges from Next.js and can prevent production preview flows from receiving the bypass cookie in cross-site contexts. Centralize draft-mode cookie attributes in the headers shim, use SameSite=None and Secure outside development, and clear the bypass cookie with an expired date. Add focused compatibility coverage for production enable and disable attributes.
NathanDrake2406
force-pushed
the
nathan/fix-draft-mode-cookie-attrs
branch
from
945e73a to
4b1da5d
Compare
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-scoped fix. The cookie attribute changes match the linked Next.js source exactly, the helper consolidates the duplicated typeof process guard, and the tests cover both enable() and disable() in production mode.
One minor nit below — not blocking.
|
|
||
| // Draft mode cookie name (matches Next.js convention) | ||
| const DRAFT_MODE_COOKIE = "__prerender_bypass"; | ||
| const DRAFT_MODE_EXPIRED_DATE = new Date(0).toUTCString(); |
There was a problem hiding this comment.
Nit: EXPIRED_COOKIE_DATE at line 64 and DRAFT_MODE_EXPIRED_DATE here are the same value (new Date(0).toUTCString()). Consider reusing the existing constant instead of introducing a second one.
| const DRAFT_MODE_EXPIRED_DATE = new Date(0).toUTCString(); | |
| const DRAFT_MODE_EXPIRED_DATE = EXPIRED_COOKIE_DATE; |
Or just use EXPIRED_COOKIE_DATE directly at the call site on line 771. Not blocking — the duplication is harmless and both are module-scoped constants.
| } | ||
| }); | ||
|
|
||
| it("draftMode().enable() uses cross-site cookie attributes in production", async () => { |
There was a problem hiding this comment.
Nit: This test is almost identical to the existing "includes Secure flag in production" test at line 79 — both set NODE_ENV=production, call enable(), and assert Secure. The only new assertion here is SameSite=None. Consider folding the SameSite=None assertion into the existing test rather than duplicating the entire setup. Keeps the test file tighter and avoids two NODE_ENV swaps for the same code path.
|
Review submitted. Here's a summary: Approved with two minor nits:
The core change is correct and well-motivated — |
What this changes
Vinext draft-mode cookies now match Next.js production cookie semantics.
draftMode().enable()emitsSameSite=None; Secureoutside development, anddraftMode().disable()clears the bypass cookie with an expired date while preserving the same production attributes.Why
The existing shim always emitted
SameSite=Laxand cleared withMax-Age=0. That diverges from Next.js and can prevent production preview flows from receiving the__prerender_bypasscookie in cross-site contexts, leavingdraftMode().isEnabledfalse even after enabling draft mode.Next.js references:
DraftModeProvider.enable()usessameSite: process.env.NODE_ENV !== 'development' ? 'none' : 'lax'andsecure: process.env.NODE_ENV !== 'development': draft-mode-provider.ts#L68-L75DraftModeProvider.disable()clears viaexpires: new Date(0)with the same cookie attributes: draft-mode-provider.ts#L80-L92sameSiteandsecurepolicy: api-resolver.ts#L131-L160Approach
The draft-mode cookie behavior stays in the normal
next/headersshim module. Generated entries still only ask the shim for the pending draft-mode cookie header, so codegen remains app-shape wiring rather than owning cookie policy.The implementation adds a small helper for draft-mode cookie attributes:
Path=/; HttpOnly; SameSite=LaxPath=/; HttpOnly; SameSite=None; SecureDisable now emits
Expires=Thu, 01 Jan 1970 00:00:00 GMTinstead ofMax-Age=0, matching Next.js's documented source comment and avoiding a cookie-clear form Next.js explicitly does not use.Validation
vp test run tests/nextjs-compat/draft-mode.test.ts -t "cross-site|Next.js production cookie attributes"failed before the fix withSameSite=LaxandMax-Age=0.vp test run tests/nextjs-compat/draft-mode.test.tsvp test run tests/shims.test.ts -t "draftMode"vp lintvp fmtvp checkRisks / follow-ups
This intentionally changes only draft-mode cookie serialization. Request parsing, secret validation, generated entry wiring, and route-handler response plumbing are unchanged.