Repository navigation
refactor(browser): use segment-based cross-tab cache invalidation - #13
Merged
Merged
Conversation
- Replace path prefix matching with segment ID comparison for more accurate cache invalidation between tabs - Use direct callback instead of CustomEvent for cross-tab refresh - Defer refresh when navigation is in progress, trigger when idle - Prevent duplicate refresh requests with pending flag 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
ivogt
added a commit
that referenced
this pull request
Jun 18, 2026
…he write The response-cache-serve extraction (review dedup #10/#13) deferred the cache clone into the waitUntil callback (putFresh(response) cloned inside the async body). On a cache MISS the same response is also returned to the middleware chain, where mergeResponse rebuilds it as `new Response(response.body, ...)`. The background clone() and that foreground body read could interleave and throw "Response body object should not be disturbed or locked" — a flaky 500 on cached response routes (cache-isolation e2e). In dev this surfaced as a Vite dev-server "Internal server error" that crashed/restarted the server under Playwright retries, cascading into the dev e2e shards being killed. Fix: clone synchronously at the miss site (before returning), so the returned body is teed independently of the cache write. The SWR path is unaffected (its fresh response is created inside the background callback and never returned). Verified: cache-isolation onResponse-freshness 3x reliably green in dev (was flaky 500); 67 cache-isolation + cache dev e2e green at retries=0; cache/dispatch unit suites 706 green; typecheck 13/13.
ivogt
added a commit
that referenced
this pull request
Jun 18, 2026
…ser, theme, server, and testing primitives (#611) * fix(router): batch A — matching/core correctness + perf (6 fixes) A1 [bug] trie-matching: reject empty required-param segment (/a//b no longer matches /a/:s/b binding s=""); matches the regex ([^/]+) contract + parity row. A2 [perf] pattern-matching: extractStaticPrefix computes the cut per-segment so a literal colon in a static segment (/a:b/c/:id) no longer drops the fast-skip. A3 [perf] middleware: matchMiddleware runs the scope regex once (single match()) instead of test() + extractParams' second match() per matching entry. A4 [api] dsl-helpers: route(name,...) now calls validateUserRouteName like path()/include(), so a reserved name ($prefix_/$path_) errors instead of silently vanishing from reverse()/types. A5 [bug] router: prefetchCacheTTL NaN/Infinity guarded with Number.isFinite (-> 300s default) instead of emitting Cache-Control: max-age=NaN. Extracted to prefetch-cache-ttl.ts for unit coverage. A6 [bug] document the regex-fallback wildcard trailing-slash splat divergence vs the canonical trie with a parity row so it cannot silently widen (trie is canonical; matcher behavior unchanged). * fix(router): batch B — segment-resolution + request-context (6 fixes) B1 [error-handling] DSL loader errors now reach the consumer onError callback and loader.error telemetry: buildLoaderErrorContext threads errorContext into all wrapLoaderPromise call sites (fresh/revalidation/intercept), matching fetchable loaders. B2 [perf] resolveLoaderData short-circuits the per-loaderId dedup BEFORE running the user tags()/TTL/SWR/recordRequestTags work, so a deduped second resolution no longer re-runs tags(). B3 [memory] segment-loader-promise caps the per-key LoaderCacheEntry array (evict-oldest) so it stops growing unbounded across navigations. B4 [bug] intercept segments are cached once on a fresh intercept navigation (dedup state.interceptSegments against allSegments ids), not twice. B5 [bug] createUseFunction loaders (action/dispatch path) now receive the parsed typed ctx.search instead of always {}. B6 [bug] Max-Age cookie parsing treats only a numeric <= 0 as deletion, so Max-Age=05 (zero-prefixed non-zero) is no longer misread as a deletion. * fix(cache): batch C — runtime/scope/document/profile/tag/base64 (6 fixes) C1 [bug] "use cache" key for a FormData arg is now derived from sorted entries (boundary-independent), not the random multipart boundary, so identical args produce the same key and actually hit the cache (was: always missed). C2 [error-handling] a consumer cache({ key }) fn that throws now degrades to a cache miss (render continues) instead of crashing the page render. C3 [bug] the document cache no longer stores/serves an unqualified Cache-Control: no-cache response as a fresh HIT (RFC 7234); field-qualified no-cache="set-cookie" is still storable. C4 [error-handling] resolveCacheProfiles now rejects non-finite/negative ttl/swr at createRouter time instead of creating a permanently-fresh, never-evicting entry. C5 [bug] normalizeTag trims surrounding whitespace so " products " and "products" are one cache tag, not two. C6 [perf] bufferToBase64 builds the latin1 string in 8KB chunks instead of one byte at a time (byte-identical output), cheaper for large document payloads. * fix(rsc): batch D — action/render/loader-fetch/handler (7 fixes) D1 [error-handling] a malformed JS-path action body now yields a controlled 400 (error boundary or plain 400) instead of an opaque host 500, converging with the no-JS PE path; the decode error is sanitized (no leak). D2 [bug] the initial full-render payload carries warmupEnabled, so warmup:false is respected from first load (was ignored until a navigation). D3 [bug] a loader error boundary that renders to a falsy node (0/""/false) is used instead of being discarded and the original error rethrown (== null sentinel, not falsy). D4 [bug] a thrown Response (redirect/notFound) from a no-middleware fetchable loader now produces the real 302/404 instead of a generic 500. D5 [security] the production fetchable-loader error payload no longer leaks the consumer error class name (dev unchanged). D6 [bug] negotiated response routes and RSC renders emit Vary with Accept once (appendVaryAccept dedup helper) instead of a duplicated token. D7 [perf] early SSR setup is only kicked off when the request will render HTML, not for no-Accept requests that return RSC (avoids wasted work + an orphan rejection window); mayNeedSSR / document-cache keying untouched. * fix(build,vite): batch E — route-types + expose-id transforms + presets (8 fixes) E1 [bug] source-scan classifier is now regex-literal-aware (a /.../ literal containing a quote or // no longer desyncs the code/string/comment state and drops a real router file from discovery). E2 [bug] include-resolution resolves an include() target imported via a combined default+named import (import Foo, { bar }) instead of false-hard-erroring. E3 [bug] createRouter $$id idempotency guard checks for the actual injected $$routeNames marker, not a substring, so args containing the literal "$$id" no longer drop named-route wiring. E4 [bug] aliased loader exports get the same single id (exportNames[0]) on the client stub as the server registers, fixing client/server $$id divergence. E5 [bug] transformHandles branches on binding.argCount (AST-derived) instead of a string-trim heuristic, so a comment-prefixed createHandle arg transforms correctly. E6 [bug] the Node preset's rsc environment gets the same optimizeDeps.exclude the cloudflare preset and ssr env set. E7 [perf] prerender runWithConcurrency stops scheduling new work on the first error instead of letting remaining workers finish before the build aborts. E8 [bug] the client-component-hmr 'use client' sniff is AST-based and tolerates a leading comment before the directive (hasUseClientDirective). * fix(browser): batch F — link-status/partial-update/scroll/prefetch/Link (7 fixes) F1 [bug] useLinkStatus releases the pinned useOptimistic value when navigation returns to idle, so a Link spinner no longer stays stuck after a view-transition navigation. F2 [concurrency] the partial-update full-update fallback re-checks the history key after the stream completes (like the partial branch), so a stale stale-revalidation full update can't clobber a page the user navigated to. F3 [test-gap] adds unit coverage for ScrollRestoration (LRU eviction, MRU re-order, sessionStorage quota evict-and-retry ladder, custom getKey). F4 [memory] a direct (hover) prefetch with no caller AbortSignal now installs an internal abort+timeout so a never-settling fetch can't strand the inflight key and permanently block re-prefetch of that URL. F5 [bug] Link evaluates touch capability per render (resolveAdaptiveStrategy) instead of once at module load, so prefetch="adaptive" reflects current input capability (hybrid device, SSR->hydrate). F6 [type-safety] LocationState.delete() guards that history.state is an object before `key in current`, so a non-null primitive state is a no-op instead of throwing. F7 [test-gap] adds the missing useSearchParams SSR-empty-seed + mount-catch-up unit test (mirrors usePathname); no source divergence found. * fix(theme,handles): batch G — theme config/provider + meta/breadcrumbs (5 fixes) G1 [bug] resolveThemeConfig coerces a "system" defaultTheme to a concrete theme when enableSystem is false, so <html> never gets a bogus class="system". G2 [bug] client setTheme validates against the configured theme set (warn+ignore an unknown theme) like server ctx.setTheme, preventing SSR drift. G3 [bug] collectMeta handles Promise meta descriptors explicitly and dev-warns when one is pushed under an active title template (async meta is append-only and documented; collect is synchronous so it cannot participate in dedup). G4 [bug] breadcrumbs href-dedup keeps each href's first position but last value, so re-pushing a parent crumb refreshes its label in place instead of reordering it to the end (parent->child order preserved). G5 [api] the FOUC theme script is idempotent (a per-storageKey window guard), so rendering both MetaTags (auto-inject) and ThemeScript no longer double-runs the script or leaks a matchMedia listener; JSDoc steers consumers to pick one. * fix(server,core): batch H — loader-registry/waitUntil/search/client-rsc (5 fixes) H1 [error-handling] a loader module-load (dynamic import) failure now surfaces as a 500 + onError instead of being swallowed into a misleading 404; an unregistered loader still 404s. H2 [error-handling] waitUntil defers the callback (Promise.resolve().then) so a synchronously-throwing callback is fire-and-forget (logged) instead of propagating into the request flow, on both the EC and no-EC seats. H3 [bug] parseSearchParams number coercion is strict-decimal: ""/whitespace/ Infinity/0x10 are treated as missing (omitted, so schema defaults apply) instead of becoming 0/Infinity/16; valid decimals and 1e3 still parse. H4 [api] the RSC client entry (client.rsc.tsx) re-exports the full "use client" hook set the default entry exports (useParams/useMount/useSegments/ useSearchParams/...), matching the surface; navigation/action stay omitted by design. H5 [dead-code] removed the deprecated dead extractUrlsVariableFromRouter (function + re-export), per the pre-stable no-deprecated-API rule; its self-test now exercises the canonical extractUrlsFromRouter. * fix(testing): batch I — primitive/production parity (4 fixes) I1 [api] a seeded ctx.use(Loader) now returns a Promise (Promise.resolve) like production and the real-fn delegate path, instead of the raw value. I2 [test-gap] dispatch wires the production response-route cache path (getResponse / SWR / putResponse / resolveCacheTags), so cached path.json/path.text routes are reachable + cache via the primitive; cache-scope is lazily imported so the testing barrel doesn't eagerly pull plugin-rsc. I3 [test-gap] runInRequestContext (and runLoader) drain ctx.onResponse() callbacks on both the normal and throw-redirect branches, matching production. I4 [test-gap] renderToFlightString accepts theme/cacheStore/cacheProfiles and threads them into the RequestContext, so a server component reading ctx.theme or invoking use-cache works under the primitive instead of crashing. * test(e2e): batch J — dev+prod e2e for round-4 consumer-observable fixes Adds dev+prod e2e (guard-safe (production) siblings, each run locally) for the HTTP/browser-observable round-4 fixes: - A1 empty path segment /item//detail does not bind :itemId="" (cloudflare-basic trie-routing). - C3 an unqualified Cache-Control: no-cache response is never served as a fresh document-cache HIT (cloudflare-basic blog-cache + a fixture page). - D4 a fetchable loader throwing a redirect Response yields the real 302, not 500 (router e2e, test-app ThrownRedirectLoader). - D5 in production the fetchable-loader error body does not leak err.name; dev does (router e2e, test-app NamedErrorLoader). - D6 a negotiated response/RSC route emits Vary: Accept exactly once (router e2e content-negotiation). D1 (malformed-action 400) and G1 (theme system+enableSystem:false) are covered by unit tests but deferred at the e2e layer: D1's 400 path isn't reliably reachable over HTTP (React decodeReply is lazy, so a malformed body surfaces on the action -runtime 500 path); G1's theme config is router-global and an e2e would break the existing theme suite or require a second app. cloudflare-basic router.named-routes.gen.ts regenerated for the one new route. * fix(round4): address /simplify + /code-review findings (24 items) A multi-agent review (62 raw -> 33 confirmed) of the 54-fix branch surfaced real issues in the new code; all verified-to-fix items applied: Bugs (red-before-green): - cache-runtime: the C1 Blob/File cache-key token percent-encodes type/name so a user-controlled colon in a MIME type or filename can't collide two files onto one key. - expose-id-utils countArgs: a comment-only createHandle/createLoader arg list is counted as 0 args at the producer (fixes E5 on the regex-fallback path, not just the AST path). - location-state write(): coerces a primitive/null history.state to a fresh dict like delete() (the F6 guard was missing from the sibling writer). - setTheme rejects "system" when enableSystem is false on BOTH client and server (shared isValidTheme/warnInvalidTheme helper), closing the gap the G1 coercion left in the setter. - intercept cache-hit loader revalidation reports onError isPartial:true (matches the revalidation path). - loader-fetch: notFound() (a DataNotFoundError) maps to 404, not a generic 500. - meta async-descriptor dev-warning no longer false-positives on a normal async descriptor (only the title-template collision case). - prefetch stalled-fetch timeout is layered even when the caller passes a signal (queue-driven prefetches), so a stalled server can't strand the inflight key. Simplify / reuse (no behavior change): - shared leading-directive sniff (rango.ts + version-plugin), shared isThenable predicate (meta/MetaTags/breadcrumbs), shared loader-error response builder, shared response-cache-serve leaf used by both production and the dispatch primitive (plugin-rsc-free via injected deps), drainOnResponseCallbacks reused from rsc/helpers instead of inlined, de-duplicated TTL-guard comments. Coverage / docs: - binary-arg use-cache HIT test, warmupEnabled-via-router.fetch primitive test, a real Max-Age=00 deletion assertion, corrected scroll-restoration JSDoc. * fix(round4): address PR-review findings (P1 security + P2/P3 hardening) P1 [security] response-cache-serve: a configured cache({ key }) or store keyGenerator that THROWS now degrades to a cache miss (runs uncached, no read/write) instead of falling back to the broad response:type:host+path key and caching under it — which could serve one user's personalized output to another when the key encodes tenant/user/auth state. The no-key default path is unchanged. P2a cache-runtime: replyToCacheKey type-tags string FormData entries ("s:") like it already tags Blob/File entries ("b:..."), so a user string equal to a blob token can no longer collide two distinct arg lists onto one "use cache" key. P2b cache-scope + handle-snapshot: segment-codec (which pulls @vitejs/plugin-rsc) is now imported lazily at its async use sites, so cache-scope is plugin-rsc-free at module load. The dispatch() testing primitive's cached-response-route path is now usable in a plain Node/Vitest consumer test without mocking plugin-rsc (covered by a new no-mock dispatch test); segment-caching behavior is unchanged. P3 theme: the inline FOUC boot script and the cross-tab storage event handler now apply the same validity rule as setTheme — a stored/received "system" with enableSystem:false falls back to defaultTheme instead of leaving a bogus class="system"/colorScheme="system" on <html>. * fix(cache): clone the response synchronously before deferring the cache write The response-cache-serve extraction (review dedup #10/#13) deferred the cache clone into the waitUntil callback (putFresh(response) cloned inside the async body). On a cache MISS the same response is also returned to the middleware chain, where mergeResponse rebuilds it as `new Response(response.body, ...)`. The background clone() and that foreground body read could interleave and throw "Response body object should not be disturbed or locked" — a flaky 500 on cached response routes (cache-isolation e2e). In dev this surfaced as a Vite dev-server "Internal server error" that crashed/restarted the server under Playwright retries, cascading into the dev e2e shards being killed. Fix: clone synchronously at the miss site (before returning), so the returned body is teed independently of the cache write. The SWR path is unaffected (its fresh response is created inside the background callback and never returned). Verified: cache-isolation onResponse-freshness 3x reliably green in dev (was flaky 500); 67 cache-isolation + cache dev e2e green at retries=0; cache/dispatch unit suites 706 green; typecheck 13/13. * ci: shard the dev e2e across 5 jobs instead of 3 The rangojs-router dev e2e runs in dev mode (a live Vite dev server + esbuild + Chromium held resident across the shard) over ~300 tests. At 3-way sharding the two heaviest shards exhausted the GitHub-hosted runner's memory and the runner was terminated mid-run ("received a shutdown signal" at 5-8 min, well under the 20-minute job cap) — surfacing as flaky dev-shard failures while the same shard passes locally (more RAM). Splitting dev across 5 smaller shards keeps each within the runner's memory budget and shortens each shard. Production sharding (2) and the HMR jobs are unchanged. * ci: raise rangojs-router-e2e job timeout 20 -> 30 min Complements the 3->5 dev sharding. The smaller shards removed the runner-OOM, but the shard that draws the timing-sensitive dev tests (handle-breadcrumbs late-push + cache param-change soft-navs) spends real wall-clock on per-test timeouts and their Playwright retries under CI load, which brushed the old 20-minute cap. 30 minutes gives that headroom; production/HMR jobs are unchanged. * ci: cap each playwright install-deps step at 60s and retry once `playwright install-deps chromium` shells out to apt-get to install browser system libraries, which can hang on a slow/transient package mirror and stall an e2e job. Wrap all 8 install-deps steps so each attempt is bounded to 60s (coreutils `timeout`, available on ubuntu-latest) and retried once on failure or timeout (`timeout 60 ... || timeout 60 ...`), so a flaky mirror costs at most ~2 minutes and self-heals instead of hanging to the job cap. * fix(build): regex-position scan uses keyword context, not just the previous char The E1 regex-literal skip decided "regex vs division" from the previous significant CHAR only. After a keyword like `return` (which ends in a word char) it read `/` as division, so a regex body's inner quotes opened a phantom string and swallowed a later real createRouter() -> the router file was dropped from discovery / HMR / typegen. Example: `return /it's a "x"/g; createRouter()`. Now, when the previous char ends an identifier, the scanner walks back over any whitespace + the identifier and treats `/` as a regex iff that identifier is a regex-preceding keyword (return/typeof/instanceof/in/of/new/delete/void/do/else/ yield/await/case/throw). A value-named identifier (`returnValue / b`) stays division. Red-before-green: the `return`/`typeof`/`case`/`throw` cases fail without the keyword lookback and pass with it; division-after-a-value is unaffected. * ci: raise the playwright install-deps per-attempt timeout 60s -> 180s The 60s cap was too aggressive: `playwright install-deps chromium` runs apt-get, which legitimately takes ~1-2 min on some runners, so a 60s timeout killed it mid-install (both the attempt and the retry) and failed 3 e2e shards on a clean run. 180s per attempt still catches a genuine hang (a stuck mirror would far exceed it) while allowing a normal/slow-but-real install; the single retry is kept. * fix(browser): bound the prefetch body stream and evict a stalled entry by identity Two reviewer findings on the hover/queued prefetch path: P2 (body stall) — the stall timeout only covered "no response ever arrives". Once headers arrived the timer was cleared in `.finally()`, so a body that stalled after headers left a published entry whose payload/streamComplete never settle: future prefetches dedupe against it and navigation awaits it forever. The timer now stays armed past headers; it is cleared when the stream completes (or in `.finally()` only for paths that publish no streaming entry), and on fire it aborts the fetch/stream and evicts the published entry. P3 (identity-guarded eviction) — evicting by generation alone could delete a fresh entry republished under the same key in the same generation (stalled entry consumed, then a new prefetch publishes, then the stale timer fires). removePrefetch now takes the published entry and deletes only when cache.get(key)?.entry === entry, so it can only ever drop its own stalled entry. Regression tests (red-before-green): a response whose body never closes is evicted after the timeout and re-prefetchable; and a stalled entry's timer firing does NOT drop a fresh entry republished under the same key after the stalled one was consumed. * ci: revert the playwright install-deps timeout+retry (it broke the step) The `timeout 180 ... || timeout 180 ...` wrapper was net-harmful and failed ~4 e2e jobs per run: - `playwright install-deps` (apt-get downloading + installing ~21MB of browser system libs) legitimately exceeds 180s on some runners, so `timeout` killed it mid-install. - `timeout` only kills the pnpm wrapper; the sudo'd apt-get grandchild keeps running and holds /var/lib/dpkg/lock-frontend, so the retry fails with "E: Could not get lock ... held by process N (apt-get)". The job-level 20-minute timeout already bounds a genuinely hung install, so the per-step timeout/retry added failure modes (killed-but-working apt, lock contention on retry) without real benefit. Restore the plain install-deps step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
setCrossTabRefreshCallbackto NavigationStore interfaceTest plan
🤖 Generated with Claude Code