Skip to content

refactor(browser): use segment-based cross-tab cache invalidation - #13

Merged
ivogt merged 1 commit into
mainfrom
feature/segment-based-cross-tab-sync
Dec 4, 2025
Merged

ivogt merged 1 commit into
mainfrom
feature/segment-based-cross-tab-sync

Conversation

@ivogt

@ivogt ivogt commented Dec 4, 2025

Copy link
Copy Markdown
Collaborator

Summary

  • 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

Changes

  • navigation-store.ts: Broadcast segment IDs with invalidation events, check for shared segments instead of path prefixes, defer refresh when loading
  • navigation-bridge.ts: Register callback with store instead of event listener
  • types.ts: Add setCrossTabRefreshCallback to NavigationStore interface

Test plan

  • Open same route in two tabs
  • Trigger server action in one tab
  • Verify other tab refreshes
  • Test with unrelated routes (should NOT refresh)
  • Test with parent/child routes sharing segments (SHOULD refresh)

🤖 Generated with Claude Code

- 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
ivogt merged commit c6086d2 into main Dec 4, 2025
@ivogt
ivogt deleted the feature/segment-based-cross-tab-sync branch December 4, 2025 08:39
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant