[PR #532] [MERGED] fix: concurrent SSR isolation for pages router head/router state #652

New issue

Closed

opened 2026-05-06 13:09:20 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:09:20 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/532
Author: @aidantrabs
Created: 3/13/2026
Status: ✅ Merged
Merged: 3/15/2026
Merged by: @james-elicx

Base: main ← Head: feat/als-phase2-parity-tests

📝 Commits (10+)

0034a09 test: add concurrent ssr isolation fixture pages for pages router
1f8891a test: add pages router dev concurrency isolation tests
3069026 fix: register head/router als accessors in ssr module graph
e3d2a9c test: add prod concurrency isolation tests
2df5230 docs: add als architecture and per call scope docs
49635d0 fix: move als accessor registration to one time setup in createSSRHandler
07e9bd7 test: add assertion messages and explicit guards for ci debugging
4d2e6af test: use lookahead in extractMetaContent for attribute order independence
7f86a91 test: add comment explaining prod head test limitations
22958f0 docs: clarify pr #450 wording in als architecture doc

📊 Changes

8 files changed (+419 additions, -54 deletions)

View changed files

📝 packages/vinext/src/server/dev-server.ts (+16 -1)
➕ packages/vinext/src/shims/ALS-ARCHITECTURE.md (+98 -0)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+56 -50)
➕ tests/fixtures/pages-basic/pages/concurrent-head.tsx (+26 -0)
➕ tests/fixtures/pages-basic/pages/concurrent-router.tsx (+31 -0)
📝 tests/helpers.ts (+6 -2)
➕ tests/pages-router-concurrency.test.ts (+180 -0)
📝 tests/pages-router.test.ts (+6 -1)

📄 Description

Closes #478 (first follow-up PR)

summary

found and fixed a real bug: <Head> elements leaked between concurrent dev requests because head-state.ts als accessors were only registered in vite's node module graph, not the ssr module graph where the Head component runs
added ssrLoadModule calls for vinext/head-state and vinext/router-state in dev-server.ts to register als-backed accessors in the correct environment
added concurrent ssr isolation tests for both dev and prod (15 parallel requests per test, verifying head, router, and getServerSideProps data)
added als architecture documentation covering the unified request context, per-call cache scopes, and the shim registration pattern
documented the decision to keep cacheContextStorage and _unstableCacheAls separate from the unified context - they are per-call, not per-request

what changed

packages/vinext/src/server/dev-server.ts - load head-state and router-state in the ssr module graph before rendering
tests/fixtures/pages-basic/pages/concurrent-head.tsx - fixture page that sets <Head> title and meta from a query param
tests/fixtures/pages-basic/pages/concurrent-router.tsx - fixture page that echoes ssr pathname and query from getServerSideProps and useRouter
tests/pages-router-concurrency.test.ts - dev and prod concurrency tests
packages/vinext/src/shims/ALS-ARCHITECTURE.md - architecture docs

why

vite's dev server has separate module graphs for node and ssr environments.

static imports in dev-server.ts load into the node graph, but react components render in the ssr graph. the head-state.ts registration was only happening in the node graph, so the ssr copy of head.ts kept using a shared module-level array for <Head> children. every concurrent request pushed into the same array.

the prod server doesn't have this problem because bundling collapses everything into one module graph.

test plan

pnpm test tests/pages-router-concurrency.test.ts - 4 tests pass
- dev: head isolation, router isolation
- prod: getServerSideProps data isolation, ssr props isolation
pnpm run fmt:check - clean
pnpm run lint - clean
pnpm run typecheck - clean

next steps (issue #478 part 2)

the issue outlines a second optional/conditional cleanup pr:

audit each standalone fallback als path (headers.ts, navigation-state.ts, cache.ts, cache-runtime.ts, fetch-cache.ts, request-context.ts, router-state.ts, head-state.ts, i18n-state.ts) and remove the ones that are no longer reachable now that the unified context is in place
keep any fallback paths that still serve ssr, test, or entry-wrapper compatibility and document why

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/532 **Author:** [@aidantrabs](https://github.com/aidantrabs) **Created:** 3/13/2026 **Status:** ✅ Merged **Merged:** 3/15/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `feat/als-phase2-parity-tests` --- ### 📝 Commits (10+) - [`0034a09`](https://github.com/cloudflare/vinext/commit/0034a09fdde9ae8cdb608af6d7aa0c7c78fded1e) test: add concurrent ssr isolation fixture pages for pages router - [`1f8891a`](https://github.com/cloudflare/vinext/commit/1f8891aa9002d7430b6adec35d644b5eaa2c988b) test: add pages router dev concurrency isolation tests - [`3069026`](https://github.com/cloudflare/vinext/commit/3069026303d799595a5be481630c047f81e7bfc8) fix: register head/router als accessors in ssr module graph - [`e3d2a9c`](https://github.com/cloudflare/vinext/commit/e3d2a9c4194dd6a856dcb5b463295d7900dfaf1f) test: add prod concurrency isolation tests - [`2df5230`](https://github.com/cloudflare/vinext/commit/2df52308e38ef3ed6e856d55e5a0c670b83a24fc) docs: add als architecture and per call scope docs - [`49635d0`](https://github.com/cloudflare/vinext/commit/49635d00e8bf75daf2bc9a52f48cec56a40ef137) fix: move als accessor registration to one time setup in createSSRHandler - [`07e9bd7`](https://github.com/cloudflare/vinext/commit/07e9bd78e0394c86c5e8ee91dbf25af396988d9e) test: add assertion messages and explicit guards for ci debugging - [`4d2e6af`](https://github.com/cloudflare/vinext/commit/4d2e6af7b50cb9392784c78a0dcf6e1e2ee2f3e6) test: use lookahead in extractMetaContent for attribute order independence - [`7f86a91`](https://github.com/cloudflare/vinext/commit/7f86a9190b05bac6058740b2c9742fd9f25f22b2) test: add comment explaining prod head test limitations - [`22958f0`](https://github.com/cloudflare/vinext/commit/22958f04eda356360857dd208d7be84a91d89f71) docs: clarify pr #450 wording in als architecture doc ### 📊 Changes **8 files changed** (+419 additions, -54 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/dev-server.ts` (+16 -1) ➕ `packages/vinext/src/shims/ALS-ARCHITECTURE.md` (+98 -0) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+56 -50) ➕ `tests/fixtures/pages-basic/pages/concurrent-head.tsx` (+26 -0) ➕ `tests/fixtures/pages-basic/pages/concurrent-router.tsx` (+31 -0) 📝 `tests/helpers.ts` (+6 -2) ➕ `tests/pages-router-concurrency.test.ts` (+180 -0) 📝 `tests/pages-router.test.ts` (+6 -1) </details> ### 📄 Description Closes #478 (first follow-up PR) ## summary - found and fixed a real bug: `<Head>` elements leaked between concurrent dev requests because `head-state.ts` als accessors were only registered in vite's node module graph, not the ssr module graph where the `Head` component runs - added `ssrLoadModule` calls for `vinext/head-state` and `vinext/router-state` in dev-server.ts to register als-backed accessors in the correct environment - added concurrent ssr isolation tests for both dev and prod (15 parallel requests per test, verifying head, router, and getServerSideProps data) - added als architecture documentation covering the unified request context, per-call cache scopes, and the shim registration pattern - documented the decision to keep `cacheContextStorage` and `_unstableCacheAls` separate from the unified context - they are per-call, not per-request ## what changed - `packages/vinext/src/server/dev-server.ts` - load head-state and router-state in the ssr module graph before rendering - `tests/fixtures/pages-basic/pages/concurrent-head.tsx` - fixture page that sets `<Head>` title and meta from a query param - `tests/fixtures/pages-basic/pages/concurrent-router.tsx` - fixture page that echoes ssr pathname and query from getServerSideProps and useRouter - `tests/pages-router-concurrency.test.ts` - dev and prod concurrency tests - `packages/vinext/src/shims/ALS-ARCHITECTURE.md` - architecture docs ## why vite's dev server has separate module graphs for node and ssr environments. static imports in dev-server.ts load into the node graph, but react components render in the ssr graph. the `head-state.ts` registration was only happening in the node graph, so the ssr copy of `head.ts` kept using a shared module-level array for `<Head>` children. every concurrent request pushed into the same array. the prod server doesn't have this problem because bundling collapses everything into one module graph. ## test plan - [x] `pnpm test tests/pages-router-concurrency.test.ts` - 4 tests pass - dev: head isolation, router isolation - prod: getServerSideProps data isolation, ssr props isolation - [x] `pnpm run fmt:check` - clean - [x] `pnpm run lint` - clean - [x] `pnpm run typecheck` - clean ## next steps (issue #478 part 2) the issue outlines a second optional/conditional cleanup pr: - audit each standalone fallback als path (`headers.ts`, `navigation-state.ts`, `cache.ts`, `cache-runtime.ts`, `fetch-cache.ts`, `request-context.ts`, `router-state.ts`, `head-state.ts`, `i18n-state.ts`) and remove the ones that are no longer reachable now that the unified context is in place - keep any fallback paths that still serve ssr, test, or entry-wrapper compatibility and document why --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>