test: client-nav benchmarks use a more representative app#6812
test: client-nav benchmarks use a more representative app#6812
Conversation
📝 WalkthroughWalkthroughThe PR adds a Match component and imports useMatch across React/Solid/Vue benchmarks, reduces selector loop iterations from 100 to 50, changes navigation param computation to update params less frequently, and extends route declarations with validateSearch, beforeLoad, loaderDeps, and loader hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant Bench as Bench Client
participant Router as Router
participant Route as Route (validateSearch / beforeLoad / loaderDeps / loader)
participant Component as Component (Match/Params/Search)
Bench->>Router: navigate(to, params, search)
Router->>Route: validateSearch(search)
Route-->>Router: validatedSearch
Router->>Route: beforeLoad()
Route-->>Router: Promise.resolve()
Router->>Route: loaderDeps({search})
Route-->>Router: deps
Router->>Route: loader()
Route-->>Router: Promise.resolve()
Router->>Component: render with params/search/match
Component-->>Bench: updated UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @benchmarks/client-nav:test:perf:solid |
❌ Failed | 1m 16s | View ↗ |
nx run @benchmarks/ssr:test:perf:vue |
✅ Succeeded | 2m 9s | View ↗ |
nx run @benchmarks/ssr:test:perf:solid |
✅ Succeeded | 1m 55s | View ↗ |
nx run @benchmarks/ssr:test:perf:react |
✅ Succeeded | 2m 4s | View ↗ |
nx run @benchmarks/client-nav:test:perf:vue |
✅ Succeeded | 1m 28s | View ↗ |
nx run @benchmarks/client-nav:test:perf:react |
✅ Succeeded | 1m 30s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-03 18:14:28 UTC
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11d0bd4a61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
benchmarks/client-nav/vue/app.tsx (1)
100-104: Consider adding type annotations forsearchparameter.Per TypeScript strict mode guidelines, the
searchparameter invalidateSearchandloaderDepsis implicitly typed. While this works for a benchmark file, explicit typing would improve safety.♻️ Optional: Add explicit type annotation
const route = createRoute({ getParentRoute: () => root, path: '/$id', - validateSearch: (search) => ({ id: search.id }), + validateSearch: (search: Record<string, unknown>) => ({ id: search.id as string }), component: () => <div />, beforeLoad: () => Promise.resolve(), - loaderDeps: ({ search }) => ({ id: search.id }), + loaderDeps: ({ search }: { search: { id: string } }) => ({ id: search.id }), loader: () => Promise.resolve(), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/client-nav/vue/app.tsx` around lines 100 - 104, The validateSearch and loaderDeps functions use an implicitly typed search parameter; add an explicit type for search (e.g., define or import a Search type/interface) and annotate the parameter in validateSearch (validateSearch: (search: Search) => ...) and in loaderDeps (loaderDeps: ({ search }: { search: Search }) => ... or equivalent destructured typing) so TypeScript strict mode is satisfied; ensure any downstream returns ({ id: search.id }) remain unchanged and update any related types used by loader and beforeLoad if needed to keep signatures consistent with the new Search type.benchmarks/client-nav/solid/speed.bench.tsx (1)
41-42: Use explicit string conversion in navigation payload to avoid implicit coercion.For stricter typing and clearer intent, serialize both values at the callsite instead of relying on downstream coercion.
Proposed diff
return navigate({ to: '/$id', - params: { id: Math.floor((nextId + 1) / 2) }, - search: { id: Math.floor(nextId / 2) }, + params: { id: String(Math.floor((nextId + 1) / 2)) }, + search: { id: String(Math.floor(nextId / 2)) }, replace: true, })As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/client-nav/solid/speed.bench.tsx` around lines 41 - 42, The numeric values assigned to params and search are implicitly coerced from nextId; update the callsite to explicitly serialize both values to strings (e.g., use String(nextIdExpression) or .toString()) for params.id and search.id to satisfy strict TypeScript typing and avoid implicit coercion; locate the object with keys params and search that reference nextId and replace the numeric expressions (Math.floor((nextId + 1) / 2) and Math.floor(nextId / 2)) with their stringified equivalents.benchmarks/client-nav/solid/app.tsx (1)
78-84: Normalizesearch.idinvalidateSearchto keep downstream types stable.
validateSearchcurrently forwardssearch.idas-is, soloaderDepsinherits a weakly-typed value. Normalizing once here keeps search typing deterministic across the route lifecycle.Proposed diff
const route = createRoute({ getParentRoute: () => root, path: '/$id', - validateSearch: (search) => ({ id: search.id }), + validateSearch: (search) => ({ + id: + typeof search.id === 'string' + ? search.id + : String(search.id ?? '0'), + }), component: () => { return <div /> }, beforeLoad: () => Promise.resolve(), loaderDeps: ({ search }) => ({ id: search.id }), loader: () => Promise.resolve(), })As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/client-nav/solid/app.tsx` around lines 78 - 84, validateSearch is forwarding search.id raw which makes downstream types in loaderDeps unstable; update validateSearch to normalize id (e.g., return { id: String(search.id ?? '') } or another explicit normalized type your route expects) so the route lifecycle has a deterministic type for id; change the implementation of validateSearch (and confirm loaderDeps: ({ search }) => ({ id: search.id }) now receives the normalized/typed id) to keep types strict and consistent across validateSearch, loaderDeps, and loader.benchmarks/client-nav/react/app.tsx (1)
85-89: Consider type-safe validation in benchmarks only if consistency is preferred across frameworks.While stricter input validation for
search.idwould align with TypeScript's strict mode expectations, this is a performance benchmark where minimal overhead is intentional. The three framework implementations (React, Solid, Vue) all use identical pass-through validation, and the actual performance-sensitive code in the component already normalizes the value where it matters (line 37:Number(search.id ?? 0)). The loaders and component handlers are no-ops, so the validation doesn't affect benchmark accuracy. Adopt the suggested normalization only if cross-framework consistency or downstream type contracts require stricter schemas.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/client-nav/react/app.tsx` around lines 85 - 89, The current pass-through validation in validateSearch and loaderDeps yields ({ id: search.id }) which is intentional for minimal overhead in benchmarks; if you decide to enforce type-safe normalization for cross-framework consistency, update validateSearch and loaderDeps to normalize id (e.g., convert to a number with Number(search.id ?? 0)) so downstream code (including component and loader) receives a predictable numeric id, and adjust any related loader or loaderDeps expectations accordingly; otherwise leave the existing pass-through implementations (validateSearch, loaderDeps, component, loader) as-is to avoid adding benchmark overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@benchmarks/client-nav/react/app.tsx`:
- Around line 85-89: The current pass-through validation in validateSearch and
loaderDeps yields ({ id: search.id }) which is intentional for minimal overhead
in benchmarks; if you decide to enforce type-safe normalization for
cross-framework consistency, update validateSearch and loaderDeps to normalize
id (e.g., convert to a number with Number(search.id ?? 0)) so downstream code
(including component and loader) receives a predictable numeric id, and adjust
any related loader or loaderDeps expectations accordingly; otherwise leave the
existing pass-through implementations (validateSearch, loaderDeps, component,
loader) as-is to avoid adding benchmark overhead.
In `@benchmarks/client-nav/solid/app.tsx`:
- Around line 78-84: validateSearch is forwarding search.id raw which makes
downstream types in loaderDeps unstable; update validateSearch to normalize id
(e.g., return { id: String(search.id ?? '') } or another explicit normalized
type your route expects) so the route lifecycle has a deterministic type for id;
change the implementation of validateSearch (and confirm loaderDeps: ({ search
}) => ({ id: search.id }) now receives the normalized/typed id) to keep types
strict and consistent across validateSearch, loaderDeps, and loader.
In `@benchmarks/client-nav/solid/speed.bench.tsx`:
- Around line 41-42: The numeric values assigned to params and search are
implicitly coerced from nextId; update the callsite to explicitly serialize both
values to strings (e.g., use String(nextIdExpression) or .toString()) for
params.id and search.id to satisfy strict TypeScript typing and avoid implicit
coercion; locate the object with keys params and search that reference nextId
and replace the numeric expressions (Math.floor((nextId + 1) / 2) and
Math.floor(nextId / 2)) with their stringified equivalents.
In `@benchmarks/client-nav/vue/app.tsx`:
- Around line 100-104: The validateSearch and loaderDeps functions use an
implicitly typed search parameter; add an explicit type for search (e.g., define
or import a Search type/interface) and annotate the parameter in validateSearch
(validateSearch: (search: Search) => ...) and in loaderDeps (loaderDeps: ({
search }: { search: Search }) => ... or equivalent destructured typing) so
TypeScript strict mode is satisfied; ensure any downstream returns ({ id:
search.id }) remain unchanged and update any related types used by loader and
beforeLoad if needed to keep signatures consistent with the new Search type.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
benchmarks/client-nav/react/app.tsxbenchmarks/client-nav/react/speed.bench.tsxbenchmarks/client-nav/solid/app.tsxbenchmarks/client-nav/solid/speed.bench.tsxbenchmarks/client-nav/vue/app.tsxbenchmarks/client-nav/vue/speed.bench.tsx
Merging this PR will degrade performance by 16.32%
Performance Changes
Comparing 
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
benchmarks/client-nav/solid/app.tsx (1)
78-78: Type safety:validateSearchaccessessearch.idwithout type narrowing.The
searchparameter invalidateSearchisRecord<string, unknown>by default. Accessingsearch.iddirectly may cause TypeScript errors in strict mode since the value isunknown. Consider adding type assertion or proper validation.✏️ Proposed fix with type assertion
- validateSearch: (search) => ({ id: search.id }), + validateSearch: (search: Record<string, unknown>) => ({ id: search.id as string }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/client-nav/solid/app.tsx` at line 78, The validateSearch implementation uses search.id on a parameter typed as Record<string, unknown>, causing TypeScript errors; update validateSearch (the function named validateSearch) to narrow or assert the type of search before accessing .id — either add a runtime guard that checks search is an object and that 'id' in search and typeof (search['id']) === 'string' (or the expected type) before returning { id: ... }, or change the function signature to accept a properly typed parameter (e.g., { id: string } or a typed interface) and adjust callers accordingly so access to search.id is type-safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmarks/client-nav/solid/app.tsx`:
- Around line 43-49: The select callback in Match is accessing match.params.id
but with strict: false match.params can be undefined; update the select to use
optional chaining (e.g. select: (match) =>
runPerfSelectorComputation(Number(match.params?.id ?? 0))) so params is safely
accessed, and ensure any other direct uses of match.params in Match use the same
match.params?.id pattern; keep using runPerfSelectorComputation and the number()
call as-is.
---
Nitpick comments:
In `@benchmarks/client-nav/solid/app.tsx`:
- Line 78: The validateSearch implementation uses search.id on a parameter typed
as Record<string, unknown>, causing TypeScript errors; update validateSearch
(the function named validateSearch) to narrow or assert the type of search
before accessing .id — either add a runtime guard that checks search is an
object and that 'id' in search and typeof (search['id']) === 'string' (or the
expected type) before returning { id: ... }, or change the function signature to
accept a properly typed parameter (e.g., { id: string } or a typed interface)
and adjust callers accordingly so access to search.id is type-safe.
| function Match() { | ||
| const number = useMatch({ | ||
| strict: false, | ||
| select: (match) => runPerfSelectorComputation(Number(match.params.id ?? 0)), | ||
| }) | ||
|
|
||
| return null | ||
| return <>{runPerfSelectorComputation(number())}</> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n benchmarks/client-nav/solid/app.tsx | head -60Repository: TanStack/router
Length of output: 1748
🌐 Web query:
TanStack Router useMatch strict false select callback documentation
💡 Result:
TanStack Router documents useMatch’s strict and select options on the useMatch hook API page. (tanstack.com)
strict: false
- Default is
true. - If
strictis false, you must not passfrom(it must be unset), and the returned match type is loosened (aPartial<RouteMatch>-style shape) to reflect that it could be any active match. (tanstack.com) - This is intended for shared/reusable components where you don’t know which route you’re under (also described in the Type Safety guide). (tanstack.com)
Example:
const match = useMatch({ strict: false })select: (match) => TSelected (the “select callback”)
- Optional function called with the
RouteMatch. useMatchreturns whatever yourselectreturns.- The selected value is also used to decide whether to re-render (via shallow equality checks). (tanstack.com)
Example:
const pathname = useMatch({
strict: false,
select: (match) => match.pathname,
})(Example pattern referenced in community discussion; core behavior is in the docs.) (tanstack.com)
Citations:
- 1: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 2: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 3: https://tanstack.com/router/v1/docs/framework/react/guide/type-safety?utm_source=openai
- 4: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 5: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 6: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
🏁 Script executed:
rg "strict.*false" benchmarks/client-nav/solid/app.tsx -A 3Repository: TanStack/router
Length of output: 528
🌐 Web query:
TanStack Solid Router useMatch RouteMatch Partial type strict false
💡 Result:
In TanStack Router for Solid (@tanstack/solid-router), this is expected behavior:
useMatchis strict by default (strict: true). In strict mode you should passfrom(a route id/path), and you get a fully-typedRouteMatch. (tanstack.com)- If you set
strict: false, you must NOT passfrom, and the return type becomesPartial<RouteMatch>(so properties likesearchare optional / possiblyundefined). This is by design for reusable/shared components. (tanstack.com)
What to do
1) Route-specific component (fully typed)
const match = useMatch({ from: Route.fullPath }) // or Route.id
// match.search is fully typed2) Reusable component across many routes (no invariant, looser types)
const match = useMatch({ strict: false })
// match is Partial<RouteMatch>, so match.search may be undefined3) If you don’t want it to throw when a specific match isn’t active
const match = useMatch({ from: '/posts', shouldThrow: false }) // RouteMatch | undefinedCitations:
- 1: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 2: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 3: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 4: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
- 5: https://tanstack.com/router/latest/docs/framework/react/api/router/useMatchHook?utm_source=openai
🏁 Script executed:
# Search for type definitions or documentation in the repo
fd -e ts -e tsx | xargs rg "Partial.*RouteMatch" | head -20Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check for tsconfig and strict mode settings
fd -e json -name "tsconfig*" | xargs cat | grep -A 10 '"strict"' || echo "No strict setting found"Repository: TanStack/router
Length of output: 330
🏁 Script executed:
# Check for any type errors or eslint configs
fd -e json -name ".eslintrc*" -o -name "tsconfig*" benchmarks/ | head -5Repository: TanStack/router
Length of output: 289
🏁 Script executed:
# Look for type definition or implementation of useMatch in solid-router
fd "@tanstack/solid-router" -t d | head -3Repository: TanStack/router
Length of output: 41
🏁 Script executed:
find . -name "tsconfig*.json" -type f 2>/dev/null | head -10Repository: TanStack/router
Length of output: 435
🏁 Script executed:
# Look for TypeScript configuration
cat tsconfig.json 2>/dev/null | head -30 || echo "No root tsconfig found"Repository: TanStack/router
Length of output: 846
🏁 Script executed:
# Check if there's a type definition file in node_modules or packages
find . -path "*/node_modules/@tanstack/solid-router*" -name "*.d.ts" 2>/dev/null | head -5 || echo "Type definitions not found in sandbox"Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Search for type definitions within the repo itself
find . -path "*/packages/solid-router*" -type f \( -name "*.ts" -o -name "*.tsx" \) | grep -i "use-match\|useMatch" | head -10Repository: TanStack/router
Length of output: 197
🏁 Script executed:
cat benchmarks/client-nav/solid/tsconfig.jsonRepository: TanStack/router
Length of output: 315
🏁 Script executed:
cat packages/solid-router/src/useMatch.tsxRepository: TanStack/router
Length of output: 3495
Use optional chaining to safely access match.params with strict: false.
With strict: false, the useMatch hook types the match parameter in the select callback as a partial/union type (MakeRouteMatchUnion), making the params property optional. Accessing match.params.id directly will fail TypeScript strict mode type checking. Use optional chaining to safely handle the potentially undefined params property:
function Match() {
const number = useMatch({
strict: false,
- select: (match) => runPerfSelectorComputation(Number(match.params.id ?? 0)),
+ select: (match) => runPerfSelectorComputation(Number(match?.params?.id ?? 0)),
})
return <>{runPerfSelectorComputation(number())}</>
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Match() { | |
| const number = useMatch({ | |
| strict: false, | |
| select: (match) => runPerfSelectorComputation(Number(match.params.id ?? 0)), | |
| }) | |
| return null | |
| return <>{runPerfSelectorComputation(number())}</> | |
| } | |
| function Match() { | |
| const number = useMatch({ | |
| strict: false, | |
| select: (match) => runPerfSelectorComputation(Number(match?.params?.id ?? 0)), | |
| }) | |
| return <>{runPerfSelectorComputation(number())}</> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@benchmarks/client-nav/solid/app.tsx` around lines 43 - 49, The select
callback in Match is accessing match.params.id but with strict: false
match.params can be undefined; update the select to use optional chaining (e.g.
select: (match) => runPerfSelectorComputation(Number(match.params?.id ?? 0))) so
params is safely accessed, and ensure any other direct uses of match.params in
Match use the same match.params?.id pattern; keep using
runPerfSelectorComputation and the number() call as-is.
1 participant
make the client-nav benchmark more representative of a "full Router app" by
useMatchtooSummary by CodeRabbit