Skip to content

Commit 82cd994

Browse files
authored
Release the instant navs lock without clearing the whole cookie jar (#94947)
The `instant()` helper in `@next/playwright` released the navigation lock by calling `page.context().clearCookies({ name: INSTANT_COOKIE })`. Playwright implements a filtered `clearCookies` non-atomically: it clears the entire cookie jar and then re-adds the cookies that don't match the filter, briefly removing the application's own cookies too. Because Next.js reacts to the instant cookie's deletion by immediately re-rendering (a soft refresh or a full reload), a render whose request raced that empty window observed none of the test's cookies, so a page reading `cookies()` rendered as if nothing was set, for example `testCookie: not set`. Resource contention widened the clear-then-re-add window, which is why the instant navigation suite failed intermittently in CI. <img width="225" height="225" alt="empty cookie jar" src="https://github.com/user-attachments/assets/770b8817-e4c4-4b4b-8251-30353d892816" /> We now release the lock by reading the instant cookie's stored entries (Next.js may have updated the value, e.g. from `[0]` to `[1, null]`, but preserves the domain and path) and re-adding each with a past expiry, which deletes only those entries and leaves every other cookie untouched. This removes the transient empty-jar window while still firing the CookieStore deletion event that releases the lock. To exercise the fix under the conditions that surfaced it, we want CI's flake-detection job to replay the instant navigation suite, which only happens when the suite's test file changes. We trigger that by un-skipping the `renders runtime-prefetched content instantly during navigation` test. That test had been skipped as flaky because of a real bug in the dev runtime-prefetch decode, which #94866 has since fixed, so it passes reliably now and no longer needs to be skipped.
1 parent 6df3f23 commit 82cd994

2 files changed

Lines changed: 37 additions & 15 deletions

File tree

packages/next-playwright/src/index.ts

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,12 @@ interface PlaywrightBrowserContext {
1414
url?: string
1515
domain?: string
1616
path?: string
17+
expires?: number
1718
}>
1819
): Promise<void>
19-
cookies(): Promise<Array<{ name: string; value: string }>>
20-
clearCookies(options?: {
21-
name?: string
22-
domain?: string
23-
path?: string
24-
}): Promise<void>
20+
cookies(): Promise<
21+
Array<{ name: string; value: string; domain: string; path: string }>
22+
>
2523
}
2624

2725
interface PlaywrightPage {
@@ -86,13 +84,38 @@ export async function instant<T>(
8684
try {
8785
return await fn()
8886
} finally {
89-
// Release the lock by clearing the cookie. Next.js may have updated the
90-
// cookie value (e.g. from [0] to [1,null]) during the lock scope. We
91-
// clear by name to remove the cookie regardless of its current value or
92-
// which domain variant it was stored under.
93-
await step('Release Instant Lock', () =>
94-
page.context().clearCookies({ name: INSTANT_COOKIE })
95-
)
87+
// Release the lock by expiring the instant cookie, leaving every other
88+
// cookie untouched.
89+
//
90+
// We must NOT use `context.clearCookies({ name: INSTANT_COOKIE })` here.
91+
// Playwright implements a filtered `clearCookies` by clearing the ENTIRE
92+
// cookie jar and then re-adding the cookies that don't match the filter.
93+
// That briefly removes the application's own cookies too. Next.js reacts
94+
// to the instant cookie's deletion by immediately re-rendering, and if
95+
// that render's request races the empty window it observes none of the
96+
// app's cookies (e.g. a navigated page renders as if no cookies were set).
97+
//
98+
// Instead we read the instant cookie's stored entries (Next.js may have
99+
// updated the value, e.g. from [0] to [1,null], but preserves the domain
100+
// and path) and re-add each with a past expiry, which deletes only those
101+
// entries without disturbing the rest of the jar.
102+
await step('Release Instant Lock', async () => {
103+
const instantCookies = (await page.context().cookies()).filter(
104+
(cookie) => cookie.name === INSTANT_COOKIE
105+
)
106+
if (instantCookies.length > 0) {
107+
await page.context().addCookies(
108+
instantCookies.map((cookie) => ({
109+
name: cookie.name,
110+
value: cookie.value,
111+
domain: cookie.domain,
112+
path: cookie.path,
113+
// A past expiry (Unix epoch seconds) deletes the cookie.
114+
expires: 1,
115+
}))
116+
)
117+
}
118+
})
96119
}
97120
}
98121

test/e2e/app-dir/instant-navigation-testing-api/instant-navigation-testing-api.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ describe('instant-navigation-testing-api', () => {
9393
)
9494
})
9595

96-
// TODO: This has been flaky in CI for weeks and needs further investigation.
97-
it.skip('renders runtime-prefetched content instantly during navigation', async () => {
96+
it('renders runtime-prefetched content instantly during navigation', async () => {
9897
const page = await openPage(next, '/')
9998

10099
await instant(page, async () => {

0 commit comments

Comments
 (0)