test: add react-router navigation speed benchmark#6795
Conversation
📝 WalkthroughWalkthroughIntroduces a new performance benchmark test file for React Router that measures navigation speed using a synthetic router with components exercising the useParams and useSearch hooks through repeated parameter and search string updates. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
|
View your CI Pipeline Execution ↗ for commit 7cc446b
☁️ Nx Cloud last updated this comment at |
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: 7cc446b128
ℹ️ 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".
| from: rootRouteId, | ||
| select: (params) => runPerfSelectorComputation(Number(params.id ?? 0)), |
There was a problem hiding this comment.
useParams here is subscribed with from: rootRouteId, but the root route in this test has no strict path params while the id param is defined on the child /$id route. In this setup params.id is always undefined, so runPerfSelectorComputation(Number(params.id ?? 0)) stays constant and these 20 Params selectors do not measure path-param recomputation during navigation, which skews the reported navigation-speed benchmark results.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react-router/tests/speed.bench.tsx (1)
104-109: Route paramidis passed as number instead of string.
nextIdis a number, but route params extracted from URLs are typed as strings. While the router may coerce the value, this could cause a type mismatch in TypeScript strict mode.Consider explicit string conversion for type safety
next = () => { const nextId = id++ return navigate({ to: '/$id', - params: { id: nextId }, - search: { id: nextId }, + params: { id: String(nextId) }, + search: { id: nextId }, // search params can be any serializable value replace: true, }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/tests/speed.bench.tsx` around lines 104 - 109, The test passes a numeric nextId into route params which are expected to be strings; to fix, convert nextId to a string when calling navigate (e.g., use String(nextId) or nextId.toString()) so the params: { id: ... } value matches the router's string typing; update the navigate call that sets params: { id: nextId } (and optionally search: { id: nextId }) to pass the stringified id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-router/tests/speed.bench.tsx`:
- Around line 30-46: The Params and Search components currently call useParams
and useSearch with from: rootRouteId which doesn't target the route that defines
the id param; change the from option to the concrete route id '/$id' in both
Params and Search so the select functions receive the correct param types
(update the useParams call in function Params and the useSearch call in function
Search to use from: '/$id').
---
Nitpick comments:
In `@packages/react-router/tests/speed.bench.tsx`:
- Around line 104-109: The test passes a numeric nextId into route params which
are expected to be strings; to fix, convert nextId to a string when calling
navigate (e.g., use String(nextId) or nextId.toString()) so the params: { id:
... } value matches the router's string typing; update the navigate call that
sets params: { id: nextId } (and optionally search: { id: nextId }) to pass the
stringified id.
| function Params() { | ||
| const params = useParams({ | ||
| from: rootRouteId, | ||
| select: (params) => runPerfSelectorComputation(Number(params.id ?? 0)), | ||
| }) | ||
| void params | ||
| return null | ||
| } | ||
|
|
||
| function Search() { | ||
| const search = useSearch({ | ||
| from: rootRouteId, | ||
| select: (search) => runPerfSelectorComputation(Number(search.id ?? 0)), | ||
| }) | ||
| void search | ||
| return null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Explore the speed.bench.tsx file structure and context
# First, read the full benchmark file to understand context
echo "=== Full speed.bench.tsx file ==="
cat packages/react-router/tests/speed.bench.tsxRepository: TanStack/router
Length of output: 2737
🏁 Script executed:
#!/bin/bash
# Find the route definitions to understand rootRouteId and child route structure
echo "=== Looking for route definitions in tests directory ==="
rg -n "rootRouteId|createRootRoute|\$id" packages/react-router/tests/speed.bench.tsx -B 2 -A 2
echo "=== Checking if there's a routes file in the test directory ==="
fd -t f "route" packages/react-router/tests/ --max-depth 3Repository: TanStack/router
Length of output: 1527
🏁 Script executed:
#!/bin/bash
# Check TypeScript configuration and compilation
echo "=== TypeScript config for react-router tests ==="
find packages/react-router -name "tsconfig*.json" -type f | head -5 | xargs -I {} sh -c 'echo "File: {}"; cat {}'
echo "=== Check if this file type is included in type checking ==="
rg -n "bench.tsx|speed.bench" packages/react-router/tsconfig* 2>/dev/null || echo "Pattern not found in tsconfig files"Repository: TanStack/router
Length of output: 543
🏁 Script executed:
#!/bin/bash
# Look for similar useParams/useSearch patterns with from parameter in test files
echo "=== Similar useParams/useSearch patterns in tests ==="
rg -n "useParams|useSearch" packages/react-router/tests/ -A 2 | head -50Repository: TanStack/router
Length of output: 3662
🏁 Script executed:
#!/bin/bash
# Check if there are any type checking configurations that might exclude bench files
echo "=== Checking package.json for type checking scripts ==="
cat packages/react-router/package.json | grep -A 5 '"type\|"test\|"check' | head -30
echo "=== Looking for any tsconfig exclusions ==="
cat packages/react-router/tsconfig.json | grep -i excludeRepository: TanStack/router
Length of output: 1475
🏁 Script executed:
#!/bin/bash
# Check the useParams implementation to understand how select function is typed
echo "=== Finding useParams hook implementation ==="
fd -t f "useParams" packages/react-router/src --max-depth 2
echo "=== Examining useParams implementation ==="
rg -n "export.*useParams|function useParams" packages/react-router/src -A 20 | head -80Repository: TanStack/router
Length of output: 3131
🏁 Script executed:
#!/bin/bash
# Look for type definitions to understand select function parameter typing
echo "=== Looking for useParams type definitions ==="
fd -t f "\.d\.ts" packages/react-router/src --max-depth 3 | xargs grep -l "useParams" 2>/dev/null | head -3
echo "=== Checking if select function has implicit any ==="
ast-grep --pattern 'select: ($_) => $$$'Repository: TanStack/router
Length of output: 158
🏁 Script executed:
#!/bin/bash
# Check the UseParamsOptions type definition to understand select parameter typing
echo "=== Looking for UseParamsOptions type definition ==="
rg -n "UseParamsOptions|type UseParamsOptions" packages/react-router/src -A 10 | head -60
echo "=== Checking typePrimitives file for ValidateUseParamsOptions ==="
cat packages/react-router/src/typePrimitives.ts | grep -A 30 "ValidateUseParamsOptions" | head -50Repository: TanStack/router
Length of output: 4651
🏁 Script executed:
#!/bin/bash
# Verify which tsconfig is actually used for type checking
echo "=== Content of tsconfig.legacy.json ==="
cat packages/react-router/tsconfig.legacy.json
echo ""
echo "=== Checking root tsconfig for defaults ==="
cat tsconfig.json | head -40Repository: TanStack/router
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Verify actual TypeScript compilation would catch this
echo "=== Trying to compile the bench file with TypeScript ==="
# Check if the file would have type errors by looking at its structure
echo "Looking at line 33 specifically where params.id is accessed:"
sed -n '30,35p' packages/react-router/tests/speed.bench.tsx
echo ""
echo "Root route definition (no params):"
sed -n '62,64p' packages/react-router/tests/speed.bench.tsx
echo ""
echo "Child route with \$id param:"
sed -n '66,70p' packages/react-router/tests/speed.bench.tsxRepository: TanStack/router
Length of output: 594
🏁 Script executed:
#!/bin/bash
# Check the UseParamsBaseOptions to understand the select function parameter type
echo "=== Looking for UseParamsBaseOptions definition ==="
rg -n "UseParamsBaseOptions|type UseParamsBaseOptions" packages/react-router/src -A 15 | head -80
echo ""
echo "=== Check if select parameter has explicit type ==="
rg -n "select.*\?:" packages/react-router/src/useParams.tsx -B 2 -A 2Repository: TanStack/router
Length of output: 3532
🏁 Script executed:
#!/bin/bash
# Confirm that test files are NOT actually type-checked by the test:types script
echo "=== Verifying type checking excludes test files ==="
echo "The test:types script uses tsconfig.legacy.json:"
cat packages/react-router/package.json | grep -A 10 '"test:types"'
echo ""
echo "And tsconfig.legacy.json only includes src:"
cat packages/react-router/tsconfig.legacy.json
echo ""
echo "But packages/react-router/tsconfig.json includes tests:"
cat packages/react-router/tsconfig.json | grep -A 5 '"include"'Repository: TanStack/router
Length of output: 1192
🏁 Script executed:
#!/bin/bash
# Check how ResolveUseParams resolves the type when from: rootRouteId is used
echo "=== Looking for ResolveUseParams type definition ==="
rg -n "ResolveUseParams|type ResolveUseParams" packages/react-router/src -A 15 | head -100
echo ""
echo "=== Check if there's special handling for from parameter ==="
rg -n "from.*rootRouteId|resolveFrom" packages/router-core/src -A 5 | head -40Repository: TanStack/router
Length of output: 2275
🏁 Script executed:
#!/bin/bash
# Double-check: is this file actually compiled in vitest or just interpreted?
echo "=== Checking vitest/tsconfig behavior for .tsx files ==="
cat packages/react-router/vite.config.ts 2>/dev/null | head -50 || echo "Checking for vitest config..."
echo ""
echo "=== Confirm benchmark file is intended for performance testing only ==="
head -30 packages/react-router/tests/speed.bench.tsxRepository: TanStack/router
Length of output: 1858
🏁 Script executed:
#!/bin/bash
# The key finding: vite.config.ts shows typecheck is enabled!
# This means the bench file IS type-checked. Let's check ResolveUseParams in router-core
echo "=== Checking ResolveUseParams definition in router-core ==="
fd -t f "ResolveUseParams" packages/router-core/src --max-depth 3
echo ""
echo "=== Finding the actual implementation ==="
rg -n "type ResolveUseParams|ResolveUseParams =" packages/router-core/src -A 10 | head -80Repository: TanStack/router
Length of output: 905
🏁 Script executed:
#!/bin/bash
# Check if rootRouteId has special typing behavior
echo "=== Checking rootRouteId definition ==="
rg -n "rootRouteId|export.*rootRouteId" packages/router-core/src -B 2 -A 5 | head -40
echo ""
echo "=== Checking how from parameter affects type resolution ==="
rg -n "from.*string.*undefined|TFrom extends" packages/router-core/src -B 2 -A 2 | head -50Repository: TanStack/router
Length of output: 5750
🏁 Script executed:
#!/bin/bash
# Check the actual rootRouteId constant definition
echo "=== Finding rootRouteId constant ==="
rg -n "export.*rootRouteId|const rootRouteId" packages/router-core/src -B 2 -A 5
echo ""
echo "=== Check RouteById type to understand how it resolves ==="
rg -n "type RouteById|RouteById<" packages/router-core/src -A 8 | head -60Repository: TanStack/router
Length of output: 4993
🏁 Script executed:
#!/bin/bash
# Most important: verify if type checking actually catches this as an error
echo "=== Try running the actual typecheck on this file ==="
cd packages/react-router
echo "Running vitest typecheck on speed.bench.tsx:"
npx vitest typecheck packages/react-router/tests/speed.bench.tsx 2>&1 || echo "Exit code: $?"Repository: TanStack/router
Length of output: 1148
🏁 Script executed:
#!/bin/bash
# Verify final key details about type checking
echo "=== Verify: Does tsconfig.legacy.json (used by test:types) include bench files? ==="
cat packages/react-router/tsconfig.legacy.json
echo ""
echo "=== Verify: Does vite.config.ts enable typecheck for bench files? ==="
cat packages/react-router/vite.config.ts | grep -A 5 "typecheck"
echo ""
echo "=== Summary: Check what RoutesById contains for root route ==="
rg -n "type RoutesById|RoutesById<" packages/router-core/src -A 5 | head -40Repository: TanStack/router
Length of output: 2781
🏁 Script executed:
#!/bin/bash
# Final verification: Can we understand what allParams resolves to for root route?
echo "=== Looking at ParseRoute and how allParams is built ==="
rg -n "allParams|'types'" packages/router-core/src -B 2 -A 2 | head -80Repository: TanStack/router
Length of output: 5641
🏁 Script executed:
#!/bin/bash
# Final verification: Check if there are comments or patterns suggesting this is intentional
echo "=== Check context around the Params/Search components ==="
sed -n '28,50p' packages/react-router/tests/speed.bench.tsx
echo ""
echo "=== Check if void suppression is common pattern in benchmarks ==="
rg -n "void params|void search" packages/react-router/tests/ -B 2 -A 2
echo ""
echo "=== Understand the benchmark purpose ==="
sed -n '88,110p' packages/react-router/tests/speed.bench.tsxRepository: TanStack/router
Length of output: 2038
Use from: '/$id' for accessing the id parameter in select functions.
Using from: rootRouteId in useParams and useSearch to access id (defined on the /$id route) is imprecise. The root route defines no params, so the select function's parameter type infers to {}. While the code compiles due to flexible select function typing, using from: '/$id' would correctly reference the route where id is actually defined and improve type safety.
function Params() {
const params = useParams({
from: '/$id',
select: (params) => runPerfSelectorComputation(Number(params.id ?? 0)),
})
void params
return null
}
function Search() {
const search = useSearch({
from: '/$id',
select: (search) => runPerfSelectorComputation(Number(search.id ?? 0)),
})
void search
return null
}This aligns with TypeScript strict mode requirements and makes the code's intent clearer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-router/tests/speed.bench.tsx` around lines 30 - 46, The Params
and Search components currently call useParams and useSearch with from:
rootRouteId which doesn't target the route that defines the id param; change the
from option to the concrete route id '/$id' in both Params and Search so the
select functions receive the correct param types (update the useParams call in
function Params and the useSearch call in function Search to use from: '/$id').
1 participant
Summary
tests/speed.bench.tsxbenchmark in@tanstack/react-routerfocused on navigation throughput.useParamsanduseSearchselectors under navigation to stress route/search recomputation cost.replace: true, synchronized param/search ids, and resolving each iteration afteronRendered.Testing
CI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:perf --outputStyle=stream --skipRemoteCache -- tests/speed.bench.tsxCI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:types --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:unit --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:eslint --outputStyle=stream --skipRemoteCache -- tests/speed.bench.tsxNotes
packages/react-router/llms/rules/*.tsfiles that are unrelated to this benchmark.Summary by CodeRabbit