fix(router): final-review regressions, changeset, and byte recovery - #7883
Conversation
Fixes the regressions vs main found in the final review of #7805: - restore history-subscription cleanup on router provider unmount in all three adapters (react returns the unsubscribe unconditionally, solid uses onCleanup, vue uses onUnmounted), pinned by new transitioner-remount tests — a discarded router could previously keep loading on back/forward as a zombie - same-destination navigations now join the in-flight transaction instead of aborting and restarting it (double-click no longer reruns loaders); reloads and same-location redirects keep restart semantics - commitMatches cache retention can no longer resurrect a one-generation-old loaderData when a newer committed success exists for the same match id - same-location dedup keeps __tempLocation/__tempKey so masked vs real same-href navigations commit their state again Adds the missing release changeset enumerating the removed/changed public API with migration hints. Byte recovery: merges rendered-matches.ts, route-chunks.ts and hydrate.ts into load-client.ts so the client bundle shares gzip dictionary context across what were separate module boundaries (-104..-186 gzip B per start scenario; plain router scenarios remain net-negative vs main and hydration still tree-shakes out of them). The solid/vue store-update-count pins were re-adjusted after verifying the published update sequences are byte-identical: the counts only shifted because a one-microtask timing change moves the tests' waitFor-sampled window boundary (5 of 7 counts decreased). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 33588d1
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview4 package(s) bumped directly, 19 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
The following scenarios have bundle-size changes compared with the baseline:
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud has identified a possible root cause for your failed CI:
We classified this failure as an environment issue rather than a code regression. The failing project (@tanstack/router-devtools-core) is not touched by this PR, and the error is a 5-second import hang — not a module-not-found error that would be expected if a deleted router-core file were a missing dependency. This points to a transient infrastructure stall (e.g. Vite transform timeout) unrelated to our changes.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
Merging this PR will regress 6 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server error-paths unmatched (react) |
270.5 KB | 557.7 KB | -51.49% |
| ❌ | Memory | mem server error-paths not-found (vue) |
330.5 KB | 488 KB | -32.27% |
| ❌ | Simulation | ssr server-fn during document ssr (react) |
64 ms | 66.8 ms | -4.14% |
| ❌ | Memory | mem server error-paths not-found (react) |
244.1 KB | 254.2 KB | -4% |
| ❌ | Simulation | client-nested-params navigation loop (vue) |
150.3 ms | 156.2 ms | -3.73% |
| ❌ | Simulation | ssr global-mw server-fn (solid) |
57.6 ms | 59.6 ms | -3.32% |
| ⚡ | Memory | mem server request-churn (react) |
810.6 KB | 482.4 KB | +68.03% |
| ⚡ | Memory | mem client unique-location-churn (solid) |
527 KB | 341.8 KB | +54.22% |
| ⚡ | Memory | mem server error-paths redirect (solid) |
362.4 KB | 282.2 KB | +28.41% |
| ⚡ | Simulation | ssr server-fn not-found (solid) |
70.1 ms | 61.9 ms | +13.21% |
| ⚡ | Simulation | ssr server-fn POST (solid) |
71.4 ms | 64.8 ms | +10.26% |
| ⚡ | Memory | mem server serialization-payload (react) |
3.5 MB | 3.2 MB | +8.92% |
| ⚡ | Simulation | ssr server-fn send-context (solid) |
71.3 ms | 66 ms | +8.05% |
| ⚡ | Simulation | ssr server-fn GET (solid) |
75.2 ms | 70.6 ms | +6.46% |
| ⚡ | Simulation | ssr server-fn redirect (solid) |
55.5 ms | 52.5 ms | +5.85% |
| ⚡ | Memory | mem client navigation-churn (solid) |
622 KB | 590.1 KB | +5.41% |
| ⚡ | Memory | mem client preload-churn (vue) |
779.6 KB | 739.6 KB | +5.41% |
| ⚡ | Memory | mem server server-fn-churn (vue) |
273.8 KB | 263.3 KB | +3.98% |
| ⚡ | Memory | mem server server-fn-churn (react) |
283.6 KB | 273.2 KB | +3.81% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix-lane-loader-review-regressions (33588d1) with fix-router-core-lane-match-loader (f84d087)1
Footnotes
…budget The cache-replacement test warmed Vite's lazy transform of BaseTanStackRouterDevtoolsPanel inside the test body, so on slow CI runners the transform alone could blow the 5s test timeout (observed at 5.89s). Move the warm-up to a beforeAll with its own generous timeout and give the test itself explicit headroom. Pre-existing flake on the lane branch tip, surfaced by this PR's CI being the first to run on it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The lane pipeline already carried phase brands at lane granularity (Lane<'matched'|'contextualized'|'reduced'|'projected'>); this finishes the pattern at the two granularities it was missing, at zero runtime cost (all emitted dist JS verified byte-identical): - settleInto becomes the sole granter of a SettledMatch brand via an assertion signature, and cacheLoaderMatch requires it — the loader -> settle -> cache ordering is now compiler-enforced. The one legitimate phase jump (hydration cache-seeding of already-settled dehydrated data) is a single named, commented boundary cast. - commitMatches/commitRefreshMatches only accept a projected lane's matches (LaneMatches<'projected'>), so committing an earlier-phase lane no longer compiles. - LoadTransaction's dev-only refresh triple collapses into one optional slot [presentation, handoff?], making a half-armed refresh state unrepresentable and shrinking the clear/read sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
baa18cd
into
fix-router-core-lane-match-loader
Stacked on #7805 — implements the outcomes of the final review pass before partner testing.
Regressions vs
main, fixed (each pinned test-first, red without the fix)mainunconditionally unsubscribed; the lane branch dropped that in fix(router-core): complete lane match loader rewrite #7858 — a discarded router kept loading on back/forward as a zombie, with acks that never resolve. React now returns the unsubscribe unconditionally (fewer bytes than the previous dev-only gate), Solid usesonCleanup, Vue usesonUnmounted. Pinned by newtransitioner-remounttests in each adapter (unmount → history change → no load; remount with the same router → exactly one load per push).mainjoined the in-flight load; the branch aborted the flights before the new lane's donor lookup ran, so every beforeLoad/loader re-ran. Same-location navigations now join the in-flight transaction (_dedupeflag fromcommitLocation's same-location branch). Reloads,invalidate(), and same-location redirects keep restart semantics. The headless no-subscriber fallback load moved inside the history-commit branch so headless same-location commits don't double-load. One existing pin re-adjusted: the reentrant onLeave navigation now firesonEnterexactly once instead of a suppressed enter plus a spuriousonStay.commitMatchesretention was keyed on the rendered prefix only, so_cachecould retain an older generation of a beyond-boundary match while_committedheld a newer one — and the cache-first planning lookup would silently present the stale data. Retention now also excludes same-id settled successes anywhere in the new lane; the "hidden terminal suffix must not evict a newer preload" contract stays intact._getUserHistoryStatestripped__tempLocation/__tempKey, so masked↔real same-href navigations skipped the history commit and never wrote/cleared mask state. The mask payloads now participate in the comparison, matchingmain'sisSameState.Deliberately not fixed here (follow-up, per review discussion): concurrent loader-redirect ordering (temporally-first vs shallowest-first) — that logic is being redone separately.
Changeset
Adds the missing release changeset for the four router packages (patch, following the repo's precedent), enumerating every removed/changed public API surface with migration hints:
RouterState.{loadedAt,isTransitioning,statusCode,redirect},RouteMatch.fetchCount/'redirected'status/globalNotFound, the removedRouterCoremembers, server-onlyheaders(),StartTransitionFnsignature, and the 30min→5min gcTime defaults.Byte recovery
The rewrite's start-scenario gzip regression is compression shape, not code mass (bundles are 3–4 KB smaller minified-raw than
main). Mergingrendered-matches.ts,route-chunks.ts, andhydrate.tsintoload-client.tslets gzip share dictionary context across what were separate module boundaries:All plain router and vue scenarios remain net-negative vs
main; hydration still tree-shakes fully out of non-Start bundles (verified raw-byte-identical guards). Code moved verbatim with comments; hydration lives under a banner at the end ofload-client.ts.Test-count re-pins
store-updates-during-navigation(solid + vue): 7 exact-count pins re-adjusted. The published store-update sequences were captured and verified byte-identical before/after these changes — the counts only moved because a one-microtask timing shift relocates the tests'waitFor-sampled window boundary. 5 of 7 counts decreased.Validation
test:typesmatrix green (TS 5.5–5.9) for router-core + all three adapters🤖 Generated with Claude Code