Conversation
|
View your CI Pipeline Execution ↗ for commit 540230a
☁️ Nx Cloud last updated this comment at |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (27)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe PR adds React, Solid, and Vue client-navigation hydration benchmarks. Each scenario renders an SSR fixture, hydrates it in isolated jsdom samples, validates restored router state, and integrates with Nx, Vitest, and performance benchmarks. ChangesClient-navigation hydration benchmarks
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~60 minutes Change: Other Merge Risk: ⚪ Minimal · up to The development-only benchmark additions appear mergeable after normal checks, with no identified production or benchmark workflow risk requiring correction. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 57 functions across 31 files. (7 skipped: 7 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Changeset Version Preview4 package(s) bumped directly, 19 bumped as dependents. 🟩 Patch bumps
|
Bundle Size BenchmarksThis pull request does not affect bundle size in any measured scenario. |
Merging this PR will not alter performance
Performance Changes
Comparing 
Footnotes
|
Merging this PR will not alter performance
Performance Changes
Comparing
|
Sheraff
changed the title
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We fix the Vue hydration class mismatch by mirroring the existing React pattern: the includeHash active-state comparison in getIsActive is now gated behind an isHydrated flag, which is false during the initial hydration render and becomes true after onMounted fires. This ensures the Vue Link's first client-side render matches the server-rendered DOM (where the hash is unknown), eliminating the class="null" vs class="active" mismatch that caused the benchmark harness to throw on every iteration.
Tip
✅ We verified this fix by re-running @benchmarks/client-nav-hydration-vue:test:unit.
Suggested Fix changes
diff --git a/packages/vue-router/src/link.tsx b/packages/vue-router/src/link.tsx
index 6d2fea69..21a8c69e 100644
--- a/packages/vue-router/src/link.tsx
+++ b/packages/vue-router/src/link.tsx
@@ -10,6 +10,7 @@ import {
import { isServer } from '@tanstack/router-core/isServer'
import { useSelector } from '@tanstack/vue-store'
+import { useHydrated } from './ClientOnly'
import { useRouter } from './useRouter'
import { useIntersectionObserver } from './utils'
@@ -136,6 +137,7 @@ function useLinkPropsImpl(
next,
options.activeOptions,
router,
+ false,
)
const { resolvedProps, resolvedClass, resolvedStyle } = resolveStyleProps(
@@ -159,6 +161,8 @@ function useLinkPropsImpl(
// Determine if the link is external or internal. This is client-only so
// server renders do not allocate a computed wrapper for every link.
+ const isHydrated = useHydrated()
+
const isExternal = Vue.computed(() => !!getUrlScheme(`${getOptions().to}`))
const currentLocation: Vue.Ref<
@@ -234,6 +238,7 @@ function useLinkPropsImpl(
next.value,
options.activeOptions,
router,
+ isHydrated.value,
)
})
@@ -663,6 +668,7 @@ function getIsActive(
},
activeOptions: LinkOptions['activeOptions'],
router: AnyRouter,
+ isHydrated = true,
) {
const currentPath = removeTrailingSlash(loc.pathname, router.basepath)
const nextPath = removeTrailingSlash(nextLoc.pathname, router.basepath)
@@ -693,7 +699,7 @@ function getIsActive(
}
if (activeOptions?.includeHash) {
- return loc.hash === nextLoc.hash
+ return isHydrated && loc.hash === nextLoc.hash
}
return true
}
Or Apply changes locally with:
npx nx-cloud apply-locally wtYK-yid1
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
1 participant
🎯 Changes
Add React, Solid, and Vue hydration scenarios to the existing
benchmarks/client-navCodSpeed suite. These cover the gap between server rendering and client mounting/navigation, including post-hydration Link updates.Workload and measurement
Each adapter restores three matched routes, three
beforeLoadcontexts, two loader results, 192 ordinary Links, and eight hash-sensitive Links. The server URL has no fragment; the client URL has#details, matching half of the hash-sensitive Links.Static HTML and real Router SSR bootstrap scripts are generated during the build. Every invocation gets a fresh jsdom window and production client bundle instance. The measured region executes the payload, creates/restores the router through its public hydration API, and hydrates the existing DOM through the follow-up updates. Server rendering, HTML parsing, initial bundle evaluation, validation, and teardown are outside measurement.
createSSRAppcontainer and flush post-mount updates withnextTick. Keep production hydration-mismatch diagnostics enabled so incorrect DOM fails rather than producing a misleading benchmark result.Solid and Vue cap settlement at 100 turns. All scenarios support both CodSpeed suite hooks and Tinybench per-iteration hooks, with diagnostic render/reactivity counting confined to harness tests. Assertions cover restored data/context, zero client loader or
beforeLoadcalls, DOM reuse, hrefs/active state, event handlers, and fresh-state cleanup.Wire all three scenarios into their framework aggregate builds and type-check graph; existing Vitest/CodSpeed discovery picks them up. Update the benchmark documentation and expected CPU result count. Generated HTML and bundles remain build artifacts.
Related fix and validation
The Vue workload exercises the hash-hydration mismatch fixed by #8440. That fix is required for the strict Vue hydration assertions to pass against main; the router fixes are kept in that separate PR.
Hydration workloads were exercised during development. No checks were rerun for this update; CI will validate the aggregate integration and instrumented runs. Local wall-time results were noisy and are not used to claim a performance gain.
✅ Checklist
🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation