fix(cookies): deduplicate server cookie writes

[pjpjq]

What changed

• Wrap server-side cookies.setAll with a duplicate-payload guard.
• Skip repeated identical cookies+headers batches for the same setAll handler while still allowing different payloads through.
• Add regression coverage for duplicate and changed payload writes.

Why

When multiple server clients are created during the same SSR request, identical refresh writes can call the same response cookie writer repeatedly and emit duplicate Set-Cookie headers.

Fixes #144.

Validation

• pnpm test src/cookies.spec.ts --run
• pnpm build
• git diff --check

[mandarini]

Hi @pjpjq, thank you so much for contributing to Supabase 💚

Really appreciate you digging into #144, this is a nasty one in production (CloudFront 502s from blown header limits are no fun to debug), and the wrapper itself is written carefully: the WeakMap, the sync check-then-set, the error rollback are all thoughtful.

That said, I want to flag something before we go further, because I think the current approach won't actually fix the bug for most users hitting it.

The dedupe is keyed on the setAll function reference:

const lastServerSetAllSignatures = new WeakMap<SetAllCookies, string>();

But in the canonical Next.js pattern (which is what almost every user reporting this is on), each call to the createClient() factory produces a brand new setAll closure:

export async function createClient() {
  const cookieStore = await cookies()
  return createServerClient(url, key, {
    cookies: {
      getAll() { return cookieStore.getAll() },
      setAll(cookiesToSet) { /* ... */ }
    }
  })
}

So when issue #144 says "createServerClient might be called in the auth loader, and again later in some server function", those are two separate factory calls, each with its own setAll reference. The WeakMap stores them as separate keys, the dedup never fires, and duplicate Set-Cookie headers still go out.

The two new tests pass because they share sharedSetAll between both storages, which is exactly the shape the implementation supports but not the shape the issue describes. If you write a test that mirrors the real scenario (two separate setAll closures targeting the same underlying response), you'll see it still emits duplicates.

I think there's a cleaner fix here that sidesteps the keying problem entirely.

The cookies you're about to write are already known by applyServerStorage via its getAll() call. If you diff what you're about to set against what's already there and skip the no-op writes, you get dedup for free, without any module-scoped state:

// inside applyServerStorage, after computing setCookies/removeCookies
const currentByName = new Map(allCookies?.map(c => [c.name, c.value]));

const setsNeeded = setCookies.filter(c => currentByName.get(c.name) !== c.value);
const removesNeeded = removeCookies.filter(name => currentByName.has(name));

if (setsNeeded.length === 0 && removesNeeded.length === 0) {
  return;
}

Why this works for the #144 scenario: the first client refreshes and writes the new tokens, the shared cookieStore now reflects them, the second client's getAll() returns those new tokens, its diff comes back empty, and setAll is not called. No duplicate header. Works regardless of how the user structured their setAll closures, no cross-request state, no WeakMap, and it's a few lines.

Would you be up for restructuring the PR around that approach? Happy to chat through edge cases if anything's unclear (one to think about: what if the user's getAll doesn't reflect writes from setAll, which violates the documented invariant but does happen in the wild). And it would be great to add a regression test that mirrors the real #144 setup, with two distinct setAll closures, so future refactors can't silently regress this.

Thank you again for taking the time, contributions like this are how this library gets better.

[pjpjq]

Thanks for the detailed review — I reworked the PR around the reflected-cookie-state approach.

What changed:

• applyServerStorage now filters both set and remove operations down to the cookies that actually need to be written based on the latest getAll() result before calling setAll.
• The regression test now explicitly verifies the real Refreshing a session when you have created multiple serverClients for a request results in duplicate Set-Cookie headers, can blow out header limits on CF #144 shape: two separate storage/client instances with distinct setAll closures sharing the same underlying reflected cookie store, where the second no-op write does not emit another Set-Cookie batch.

Local validation:

• pnpm exec vitest run src/cookies.spec.ts — 36 passed
• pnpm run build — passed
• pnpm exec prettier --check src/cookies.ts src/cookies.spec.ts — passed
• pnpm exec vitest run — 99 passed