fix(router): address lane loader regressions - #7880
Conversation
|
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 0561968
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
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. |
Merging this PR will regress 7 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server error-paths not-found (solid) |
409.7 KB | 749.8 KB | -45.36% |
| ❌ | Memory | mem server error-paths not-found (vue) |
330.5 KB | 347.3 KB | -4.85% |
| ❌ | Memory | mem server peak-large-page (solid) |
790.6 KB | 829.8 KB | -4.73% |
| ❌ | Simulation | ssr global-mw server-route (vue) |
68.2 ms | 71.4 ms | -4.46% |
| ❌ | Simulation | ssr control-flow route headers (react) |
60.3 ms | 62.7 ms | -3.8% |
| ❌ | Simulation | client-side navigation loop (vue) |
71.2 ms | 73.5 ms | -3.16% |
| ❌ | Simulation | client-nested-params navigation loop (vue) |
150.3 ms | 155.1 ms | -3.04% |
| ⚡ | Memory | mem server request-churn (react) |
810.6 KB | 487.9 KB | +66.16% |
| ⚡ | Memory | mem client navigation-churn (solid) |
622 KB | 516.6 KB | +20.41% |
| ⚡ | Memory | mem server error-paths redirect (solid) |
362.4 KB | 301.7 KB | +20.11% |
| ⚡ | Memory | mem server serialization-payload (react) |
3.5 MB | 3.2 MB | +8.35% |
| ⚡ | Memory | mem client unique-location-churn (solid) |
527 KB | 494.9 KB | +6.51% |
| ⚡ | Memory | mem client preload-churn (vue) |
779.6 KB | 739.5 KB | +5.43% |
| ⚡ | Simulation | ssr server-fn not-found (solid) |
70.1 ms | 66.8 ms | +4.96% |
| ⚡ | Memory | mem server error-paths unmatched (react) |
270.5 KB | 259.7 KB | +4.18% |
| ⚡ | Memory | mem server server-fn-churn (vue) |
273.8 KB | 263.5 KB | +3.93% |
| ⚡ | Memory | mem server server-fn-churn (react) |
283.6 KB | 273.7 KB | +3.61% |
| ⚡ | Simulation | ssr control-flow error 500 (react) |
62.5 ms | 60.7 ms | +3% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix-router-core-lane-match-loader-regressions (0561968) with fix-router-core-lane-match-loader (f84d087)
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 is proposing a fix for your failed CI:
We add a <Suspense> boundary around the lazy-loaded BaseTanStackRouterDevtoolsPanel in TanStackRouterDevtoolsPanelCore, which fixes the failing cache-replacement unit test. Without it, Solid.js's lazy() throws an unhandled suspension in the jsdom environment and the component never renders, causing vi.waitFor to time out.
Tip
✅ We verified this fix by re-running @tanstack/router-devtools-core:test:unit.
Suggested Fix changes
diff --git a/packages/router-devtools-core/src/TanStackRouterDevtoolsPanelCore.tsx b/packages/router-devtools-core/src/TanStackRouterDevtoolsPanelCore.tsx
index 2d38ac4c..df6a6880 100644
--- a/packages/router-devtools-core/src/TanStackRouterDevtoolsPanelCore.tsx
+++ b/packages/router-devtools-core/src/TanStackRouterDevtoolsPanelCore.tsx
@@ -1,5 +1,5 @@
import { render } from 'solid-js/web'
-import { createSignal, lazy } from 'solid-js'
+import { Suspense, createSignal, lazy } from 'solid-js'
import { DevtoolsOnCloseContext, ShadowDomTargetContext } from './context'
import type { JSX } from 'solid-js'
import type { AnyRouter } from '@tanstack/router-core'
@@ -97,14 +97,16 @@ export class TanStackRouterDevtoolsPanelCore {
onCloseClick: () => {},
}}
>
- <BaseTanStackRouterDevtoolsPanel
- router={router}
- routerState={routerState}
- shadowDOMTarget={shadowDOMTarget}
- setIsOpen={setIsOpen}
- style={style}
- className={className}
- />
+ <Suspense>
+ <BaseTanStackRouterDevtoolsPanel
+ router={router}
+ routerState={routerState}
+ shadowDOMTarget={shadowDOMTarget}
+ setIsOpen={setIsOpen}
+ style={style}
+ className={className}
+ />
+ </Suspense>
</DevtoolsOnCloseContext.Provider>
</ShadowDomTargetContext.Provider>
)
Or Apply changes locally with:
npx nx-cloud apply-locally 0J3J-ZBUL
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
- rename the internal `_dataOnlyAssetEnd` match field to `_assetEnd`: property keys survive minification, so the shorter internal name saves bytes at every site in every bundle - reduce the `_getAssetMatches` guard to a presence check: the field is only ever set on hydration presentation clones that already satisfy the pending/data-only/error-free/not-not-found conditions, and commits clear it (invariant documented at the guard) - clone-then-patch the hydration handoff splice instead of spreading a conditional object per element, and drop the unreachable zero guard in `_getAssetMatches` Measured (gzip, vs PR tip): react-router.full -43 B, vue-router.full -35 B, react-router.minimal -8 B. All unit suites green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
undefinedloader hydration: Serialization omits an actual value for a loader that successfully returnsundefined. Hydration could mistake that for a loader that never ran, losing its successful cache state and selecting or rerunning later failures incorrectly.data-onlyorssr:falseboundary could make the client temporarily omit head tags, stylesheets, preloads, or scripts that the server rendered for verified descendant routes. This caused server/client head mismatches and transient tag removal or FOUC until the client load completed.onEnterinstead ofonStay.useMatchvalue becomeundefinedbefore Vue unmounted it. This exposed impossible transient state to component effects and could trigger invariant failures or undefined property reads.Mapidentity and size. Replacing an existing key with a new match object changed neither, so the panel could permanently display stale loader data, status, and timestamps.CancelledErrorwas not reproduced in those adapters. Avoiding that unproven parity contract also avoids shipping a large branch-retention runtime.Test plan
Bundle size
+27 B, Solid+25 B, Vue+67 B+141 B, Solid+135 B, Vue+142 B+265-351 B, Solid+273-292 B, Vue+290-339 B