benchmark: memory (codspeed)#7623
Open
Sheraff wants to merge 26 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a memory benchmarking workspace (client + server) with scenario benches, harnesses, route trees, fixtures, and per-scenario Vite/Nx/Vitest/tsconfig configs, and updates SSR stream rendering to suppress logging for expected abort/disconnect errors. ChangesMemory benchmark suite
Server benchmark suite
SSR abort logging behavior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
|
View your CI Pipeline Execution ↗ for commit e4fbeb1
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview3 package(s) bumped directly, 10 bumped as dependents. 🟩 Patch bumps
|
Contributor
Bundle Size Benchmarks
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 improve performance by 3.88%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | client-side navigation loop (vue) |
85.1 ms | 81.4 ms | +4.56% |
| ⚡ | Simulation | client-side navigation loop (solid) |
116.6 ms | 112.3 ms | +3.81% |
| ⚡ | Simulation | ssr streaming deferred (react) |
73.2 ms | 70.8 ms | +3.27% |
| 🆕 | Memory | mem aborted-requests (vue) |
N/A | 973.6 KB | N/A |
| 🆕 | Memory | mem error-paths error (vue) |
N/A | 561.9 KB | N/A |
| 🆕 | Memory | mem error-paths not-found (vue) |
N/A | 758.4 KB | N/A |
| 🆕 | Memory | mem error-paths redirect (vue) |
N/A | 636.8 KB | N/A |
| 🆕 | Memory | mem error-paths unmatched (vue) |
N/A | 562.2 KB | N/A |
| 🆕 | Memory | mem peak-large-page (vue) |
N/A | 2.2 MB | N/A |
| 🆕 | Memory | mem request-churn (vue) |
N/A | 1.3 MB | N/A |
| 🆕 | Memory | mem serialization-payload (vue) |
N/A | 6.9 MB | N/A |
| 🆕 | Memory | mem server-fn-churn (vue) |
N/A | 394.9 KB | N/A |
| 🆕 | Memory | mem streaming-peak chunked (vue) |
N/A | 11.9 MB | N/A |
| 🆕 | Memory | mem aborted-requests (react) |
N/A | 1.1 MB | N/A |
| 🆕 | Memory | mem error-paths error (react) |
N/A | 675.7 KB | N/A |
| 🆕 | Memory | mem error-paths not-found (react) |
N/A | 941.1 KB | N/A |
| 🆕 | Memory | mem error-paths redirect (react) |
N/A | 470.2 KB | N/A |
| 🆕 | Memory | mem error-paths unmatched (react) |
N/A | 1.1 MB | N/A |
| 🆕 | Memory | mem peak-large-page (react) |
N/A | 2 MB | N/A |
| 🆕 | Memory | mem request-churn (react) |
N/A | 1.6 MB | N/A |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing bench-codspeed-memory (e4fbeb1) with main (689c5ab)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
.github/workflows/memory-benchmarks.yml (1)
8-14: 📐 Maintainability & Code Quality | ⚡ Quick winBroaden the trigger set for workspace changes.
This workflow will not rerun when root workspace files like
pnpm-workspace.yamlchange, so benchmark registration/dependency drift can slip past CI. Consider including the workspace manifest and lockfile in thepathsfilter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/memory-benchmarks.yml around lines 8 - 14, The workflow's path filters only include 'packages/**' and 'benchmarks/**', so changes to workspace-level files (e.g., pnpm-workspace.yaml or the lockfile) won't trigger the job; update both the push and pull_request paths arrays to also include the workspace manifest and lockfile (for example add 'pnpm-workspace.yaml' and 'pnpm-lock.yaml' or your workspace's lockfile name) so modifications to workspace configuration and dependency lockfiles rerun the memory-benchmarks workflow.benchmarks/memory/client/scenarios/interrupted-navigations/react/vite.config.ts (1)
13-17: 📐 Maintainability & Code Quality | ⚡ Quick winSimplify plugin array pattern.
The
!!(condition) && plugin()pattern inbenchmarks/memory/client/scenarios/interrupted-navigations/react/vite.config.ts,benchmarks/memory/client/scenarios/mount-unmount/react/vite.config.ts,benchmarks/memory/client/scenarios/navigation-churn/react/vite.config.ts, andbenchmarks/memory/client/scenarios/preload-churn/react/vite.config.tsproduces an array containingfalsewhen the condition is falsy. While Vite filters falsy values, conditional spread is cleaner and more explicit.♻️ Recommended pattern
plugins: [ - !!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) && - codspeedPlugin(), + ...(process.env.VITEST && process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()] : []), react(), ],Alternatively, use
.filter(Boolean)at the end of the array.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/memory/client/scenarios/interrupted-navigations/react/vite.config.ts` around lines 13 - 17, The plugins array uses the pattern "!!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) && codspeedPlugin()" which yields falsy entries; update the plugins array to conditionally include codspeedPlugin() more cleanly (e.g., use a conditional spread like ...(process.env.VITEST && process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()] : []) or build the array and call .filter(Boolean) at the end) so only actual plugin functions (react(), codspeedPlugin()) remain in the final plugins list; apply this change for the plugins array that contains codspeedPlugin() and react().benchmarks/memory/client/scenarios/unique-location-churn/react/vite.config.ts (1)
14-15: 🎯 Functional CorrectnessGuard CodSpeed activation against
"false"/"0"env strings.Current CI/README wiring always sets
WITH_INSTRUMENTATION=1, so the existingprocess.env.*truthiness check is unlikely to misfire in benchmarks today. Still, ifWITH_INSTRUMENTATION(orVITEST) is ever set to"false"/"0"in other scripts, those values are truthy and would enablecodspeedPlugin()unintentionally—parse the env vars explicitly (e.g.,=== 'true' || === '1') like in the proposed fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/memory/client/scenarios/unique-location-churn/react/vite.config.ts` around lines 14 - 15, The current truthiness check for enabling codspeedPlugin uses !!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) which treats strings like "false" or "0" as truthy; update the condition in vite.config.ts to explicitly compare process.env.VITEST and process.env.WITH_INSTRUMENTATION against accepted true values (e.g., === 'true' || === '1') before invoking codspeedPlugin(), so codspeedPlugin() is only activated when both env vars are explicitly set to a truthy string.benchmarks/memory/server/scenarios/peak-large-page/react/src/large-page-data.ts (1)
98-111: 📐 Maintainability & Code Quality | 💤 Low valueDuplicated LCG logic requires manual synchronization.
The comment notes this is a local copy of
benchmarks/memory/server/bench-utils.tsthat must be kept in sync manually. Consider whether the benchmark harness could provide this utility at build time or through a shared constant to eliminate the sync risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/memory/server/scenarios/peak-large-page/react/src/large-page-data.ts` around lines 98 - 111, Duplicate LCG logic exists in createDeterministicRandom and randomSegment and is manually kept in sync with benchmarks/memory/server/bench-utils.ts; replace the local copy by importing the shared generator (or move the functions into a common module/package used by both the app and benchmark harness) or wire the harness to inject the utility at build time so createDeterministicRandom and randomSegment are removed from this file and the single shared implementation is consumed instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/memory/client/scenarios/mount-unmount/react/project.json`:
- Line 15: The inline env assignment in the "command" string
("NODE_ENV=production vite build --config {projectRoot}/vite.config.ts") is not
cross‑platform; update the project.json to either prefix the command with
cross-env (e.g., use "cross-env NODE_ENV=production vite build --config
{projectRoot}/vite.config.ts") after adding cross-env as a dependency, or remove
the inline NODE_ENV and set production mode via the Nx executor configuration or
Vite's default production mode/define block in vite.config.ts so builds work on
Windows and Unix alike.
In `@benchmarks/memory/server/package.json`:
- Around line 6-7: The package.json in benchmarks/memory/server has internal
dependencies "`@tanstack/react-router`" and "`@tanstack/react-start`" pinned with
"workspace:^" which violates the repo policy; update their versions to use the
workspace protocol "workspace:*" by replacing the version strings for
"`@tanstack/react-router`" and "`@tanstack/react-start`" in package.json so they
read "workspace:*".
In `@benchmarks/memory/server/scenarios/server-fn-churn/react/memory.bench.ts`:
- Around line 240-252: The expectedIdsByRequest map is only populated for index
=== 0 and index === 1, so most requests bypass validation; remove the
conditional index checks and always set expectedIdsByRequest for each built
request (e.g., for the GET path where expectedIdsByRequest.set(request,
fixture.id) is currently under index === 0, and for the POST flow where it's
under index === 1), ensuring every call to buildPostRequest/buildGetRequest
stores fixture.id (from postFixtures/getFixtures and fixtureIndex) into
expectedIdsByRequest so validateServerFnBody can validate every iteration.
In `@packages/react-router/src/ssr/renderRouterToStream.tsx`:
- Around line 60-63: The onError handler in renderRouterToStream.tsx uses a
single-line if without braces (if (isAbortError(error)) return) which violates
the TypeScript style rule; update the onError callback (the onError function
handling errors in renderRouterToStream) to wrap the early return in curly
braces (e.g., if (isAbortError(error)) { return } ) so that isAbortError(error)
check uses a block-scoped body while preserving the existing console.error
behavior.
---
Nitpick comments:
In @.github/workflows/memory-benchmarks.yml:
- Around line 8-14: The workflow's path filters only include 'packages/**' and
'benchmarks/**', so changes to workspace-level files (e.g., pnpm-workspace.yaml
or the lockfile) won't trigger the job; update both the push and pull_request
paths arrays to also include the workspace manifest and lockfile (for example
add 'pnpm-workspace.yaml' and 'pnpm-lock.yaml' or your workspace's lockfile
name) so modifications to workspace configuration and dependency lockfiles rerun
the memory-benchmarks workflow.
In
`@benchmarks/memory/client/scenarios/interrupted-navigations/react/vite.config.ts`:
- Around line 13-17: The plugins array uses the pattern "!!(process.env.VITEST
&& process.env.WITH_INSTRUMENTATION) && codspeedPlugin()" which yields falsy
entries; update the plugins array to conditionally include codspeedPlugin() more
cleanly (e.g., use a conditional spread like ...(process.env.VITEST &&
process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()] : []) or build the array
and call .filter(Boolean) at the end) so only actual plugin functions (react(),
codspeedPlugin()) remain in the final plugins list; apply this change for the
plugins array that contains codspeedPlugin() and react().
In
`@benchmarks/memory/client/scenarios/unique-location-churn/react/vite.config.ts`:
- Around line 14-15: The current truthiness check for enabling codspeedPlugin
uses !!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) which treats
strings like "false" or "0" as truthy; update the condition in vite.config.ts to
explicitly compare process.env.VITEST and process.env.WITH_INSTRUMENTATION
against accepted true values (e.g., === 'true' || === '1') before invoking
codspeedPlugin(), so codspeedPlugin() is only activated when both env vars are
explicitly set to a truthy string.
In
`@benchmarks/memory/server/scenarios/peak-large-page/react/src/large-page-data.ts`:
- Around line 98-111: Duplicate LCG logic exists in createDeterministicRandom
and randomSegment and is manually kept in sync with
benchmarks/memory/server/bench-utils.ts; replace the local copy by importing the
shared generator (or move the functions into a common module/package used by
both the app and benchmark harness) or wire the harness to inject the utility at
build time so createDeterministicRandom and randomSegment are removed from this
file and the single shared implementation is consumed instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c7015ad-66f5-470f-b9d5-dcd60d724b08
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (156)
.github/workflows/memory-benchmarks.ymlbenchmarks/memory/README.mdbenchmarks/memory/client/bench-utils.tsbenchmarks/memory/client/package.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/react/memory.bench.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/project.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/react/setup.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/app.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/router.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/routes/__root.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/routes/fast.$id.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/routes/index.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/routes/slow.$id.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/react/src/slow-loaders.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/tsconfig.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/react/vite.config.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/memory.bench.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/project.jsonbenchmarks/memory/client/scenarios/loader-data-retention/react/setup.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/src/app.tsxbenchmarks/memory/client/scenarios/loader-data-retention/react/src/loader-data.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/src/router.tsxbenchmarks/memory/client/scenarios/loader-data-retention/react/src/routes/__root.tsxbenchmarks/memory/client/scenarios/loader-data-retention/react/src/routes/page.$id.tsxbenchmarks/memory/client/scenarios/loader-data-retention/react/src/routes/shell.index.tsxbenchmarks/memory/client/scenarios/loader-data-retention/react/src/routes/shell.tsxbenchmarks/memory/client/scenarios/loader-data-retention/react/tsconfig.jsonbenchmarks/memory/client/scenarios/loader-data-retention/react/vite.config.tsbenchmarks/memory/client/scenarios/mount-unmount/react/memory.bench.tsbenchmarks/memory/client/scenarios/mount-unmount/react/project.jsonbenchmarks/memory/client/scenarios/mount-unmount/react/setup.tsbenchmarks/memory/client/scenarios/mount-unmount/react/src/app.tsxbenchmarks/memory/client/scenarios/mount-unmount/react/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/mount-unmount/react/src/router.tsxbenchmarks/memory/client/scenarios/mount-unmount/react/src/routes/__root.tsxbenchmarks/memory/client/scenarios/mount-unmount/react/src/routes/a.tsxbenchmarks/memory/client/scenarios/mount-unmount/react/tsconfig.jsonbenchmarks/memory/client/scenarios/mount-unmount/react/vite.config.tsbenchmarks/memory/client/scenarios/navigation-churn/react/memory.bench.tsbenchmarks/memory/client/scenarios/navigation-churn/react/project.jsonbenchmarks/memory/client/scenarios/navigation-churn/react/setup.tsbenchmarks/memory/client/scenarios/navigation-churn/react/src/app.tsxbenchmarks/memory/client/scenarios/navigation-churn/react/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/navigation-churn/react/src/router.tsxbenchmarks/memory/client/scenarios/navigation-churn/react/src/routes/__root.tsxbenchmarks/memory/client/scenarios/navigation-churn/react/src/routes/a.tsxbenchmarks/memory/client/scenarios/navigation-churn/react/src/routes/b.tsxbenchmarks/memory/client/scenarios/navigation-churn/react/tsconfig.jsonbenchmarks/memory/client/scenarios/navigation-churn/react/vite.config.tsbenchmarks/memory/client/scenarios/preload-churn/react/memory.bench.tsbenchmarks/memory/client/scenarios/preload-churn/react/project.jsonbenchmarks/memory/client/scenarios/preload-churn/react/setup.tsbenchmarks/memory/client/scenarios/preload-churn/react/src/app.tsxbenchmarks/memory/client/scenarios/preload-churn/react/src/item-payload.tsbenchmarks/memory/client/scenarios/preload-churn/react/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/preload-churn/react/src/router.tsxbenchmarks/memory/client/scenarios/preload-churn/react/src/routes/__root.tsxbenchmarks/memory/client/scenarios/preload-churn/react/src/routes/index.tsxbenchmarks/memory/client/scenarios/preload-churn/react/src/routes/items.$id.tsxbenchmarks/memory/client/scenarios/preload-churn/react/tsconfig.jsonbenchmarks/memory/client/scenarios/preload-churn/react/vite.config.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/memory.bench.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/project.jsonbenchmarks/memory/client/scenarios/unique-location-churn/react/setup.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/src/app.tsxbenchmarks/memory/client/scenarios/unique-location-churn/react/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/src/router.tsxbenchmarks/memory/client/scenarios/unique-location-churn/react/src/routes/__root.tsxbenchmarks/memory/client/scenarios/unique-location-churn/react/src/routes/items.$id.tsxbenchmarks/memory/client/scenarios/unique-location-churn/react/tsconfig.jsonbenchmarks/memory/client/scenarios/unique-location-churn/react/vite.config.tsbenchmarks/memory/client/tsconfig.jsonbenchmarks/memory/client/vitest.react.config.tsbenchmarks/memory/client/vitest.setup.tsbenchmarks/memory/server/bench-utils.tsbenchmarks/memory/server/package.jsonbenchmarks/memory/server/scenarios/aborted-requests/react/memory.bench.tsbenchmarks/memory/server/scenarios/aborted-requests/react/project.jsonbenchmarks/memory/server/scenarios/aborted-requests/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/aborted-requests/react/src/router.tsxbenchmarks/memory/server/scenarios/aborted-requests/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/aborted-requests/react/src/routes/index.tsxbenchmarks/memory/server/scenarios/aborted-requests/react/src/routes/stream.$id.tsxbenchmarks/memory/server/scenarios/aborted-requests/react/tsconfig.jsonbenchmarks/memory/server/scenarios/aborted-requests/react/vite.config.tsbenchmarks/memory/server/scenarios/error-paths/react/memory.bench.tsbenchmarks/memory/server/scenarios/error-paths/react/project.jsonbenchmarks/memory/server/scenarios/error-paths/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/error-paths/react/src/router.tsxbenchmarks/memory/server/scenarios/error-paths/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/error-paths/react/src/routes/boom.$id.tsxbenchmarks/memory/server/scenarios/error-paths/react/src/routes/from.$id.tsxbenchmarks/memory/server/scenarios/error-paths/react/src/routes/index.tsxbenchmarks/memory/server/scenarios/error-paths/react/src/routes/missing.$id.tsxbenchmarks/memory/server/scenarios/error-paths/react/src/routes/target.$id.tsxbenchmarks/memory/server/scenarios/error-paths/react/tsconfig.jsonbenchmarks/memory/server/scenarios/error-paths/react/vite.config.tsbenchmarks/memory/server/scenarios/peak-large-page/react/memory.bench.tsbenchmarks/memory/server/scenarios/peak-large-page/react/project.jsonbenchmarks/memory/server/scenarios/peak-large-page/react/src/large-page-data.tsbenchmarks/memory/server/scenarios/peak-large-page/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/peak-large-page/react/src/router.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/index.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.l3.l4.l5.l6.l7.l8.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.l3.l4.l5.l6.l7.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.l3.l4.l5.l6.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.l3.l4.l5.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.l3.l4.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.l3.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.l2.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/src/routes/l1.tsxbenchmarks/memory/server/scenarios/peak-large-page/react/tsconfig.jsonbenchmarks/memory/server/scenarios/peak-large-page/react/vite.config.tsbenchmarks/memory/server/scenarios/request-churn/react/memory.bench.tsbenchmarks/memory/server/scenarios/request-churn/react/project.jsonbenchmarks/memory/server/scenarios/request-churn/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/request-churn/react/src/router.tsxbenchmarks/memory/server/scenarios/request-churn/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/request-churn/react/src/routes/index.tsxbenchmarks/memory/server/scenarios/request-churn/react/src/routes/items.$id.tsxbenchmarks/memory/server/scenarios/request-churn/react/tsconfig.jsonbenchmarks/memory/server/scenarios/request-churn/react/vite.config.tsbenchmarks/memory/server/scenarios/serialization-payload/react/memory.bench.tsbenchmarks/memory/server/scenarios/serialization-payload/react/project.jsonbenchmarks/memory/server/scenarios/serialization-payload/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/serialization-payload/react/src/router.tsxbenchmarks/memory/server/scenarios/serialization-payload/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/serialization-payload/react/src/routes/data.$id.tsxbenchmarks/memory/server/scenarios/serialization-payload/react/tsconfig.jsonbenchmarks/memory/server/scenarios/serialization-payload/react/vite.config.tsbenchmarks/memory/server/scenarios/server-fn-churn/react/memory.bench.tsbenchmarks/memory/server/scenarios/server-fn-churn/react/project.jsonbenchmarks/memory/server/scenarios/server-fn-churn/react/src/fns.tsbenchmarks/memory/server/scenarios/server-fn-churn/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/server-fn-churn/react/src/router.tsxbenchmarks/memory/server/scenarios/server-fn-churn/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/server-fn-churn/react/src/routes/api.fn-urls.tsbenchmarks/memory/server/scenarios/server-fn-churn/react/src/routes/index.tsxbenchmarks/memory/server/scenarios/server-fn-churn/react/tsconfig.jsonbenchmarks/memory/server/scenarios/server-fn-churn/react/vite.config.tsbenchmarks/memory/server/scenarios/streaming-peak/react/memory.bench.tsbenchmarks/memory/server/scenarios/streaming-peak/react/project.jsonbenchmarks/memory/server/scenarios/streaming-peak/react/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/streaming-peak/react/src/router.tsxbenchmarks/memory/server/scenarios/streaming-peak/react/src/routes/__root.tsxbenchmarks/memory/server/scenarios/streaming-peak/react/src/routes/index.tsxbenchmarks/memory/server/scenarios/streaming-peak/react/src/routes/stream.$id.tsxbenchmarks/memory/server/scenarios/streaming-peak/react/tsconfig.jsonbenchmarks/memory/server/scenarios/streaming-peak/react/vite.config.tsbenchmarks/memory/server/tsconfig.jsonbenchmarks/memory/server/vitest.react.config.tspackages/react-router/src/ssr/renderRouterToStream.tsxpnpm-workspace.yaml
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
Contributor
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 change because the SyntaxError: missing ) after argument list occurs in the external dummy server process used by the e2e test infrastructure, and cannot be traced to any specific line in the PR's diff. Our changes to renderRouterToStream.tsx are syntactically valid TypeScript and would not produce this kind of runtime syntax error when compiled.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/client-nav-benchmarks.yml (1)
55-57: 📐 Maintainability & Code QualityClear documentation of the React-only limitation.
The TODO comment appropriately explains why memory benchmarks are currently React-only and notes the path to future unification.
Would you like me to open a tracking issue for the planned unification once Solid and Vue memory benchmarks are added?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/client-nav-benchmarks.yml around lines 55 - 57, Clarify the existing TODO by explicitly stating that memory benchmarks currently run only for React (add a short line like "Currently React-only") and add a follow-up action to create a tracking issue for unifying memory benchmarks across frameworks; reference the benchmark matrix and the identifiers `memory-server` and `memory-client` and the `mode: simulation|memory` constraint so the commenter knows exactly what to track and later link the issue to.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/client-nav-benchmarks.yml:
- Around line 55-57: Clarify the existing TODO by explicitly stating that memory
benchmarks currently run only for React (add a short line like "Currently
React-only") and add a follow-up action to create a tracking issue for unifying
memory benchmarks across frameworks; reference the benchmark matrix and the
identifiers `memory-server` and `memory-client` and the `mode:
simulation|memory` constraint so the commenter knows exactly what to track and
later link the issue to.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c996e485-c1b0-434e-903c-c0b78c498af2
📒 Files selected for processing (1)
.github/workflows/client-nav-benchmarks.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/client-nav-benchmarks.yml:
- Line 71: The workflow is passing an unsupported CodSpeed `mode` via
matrix.mode (used by CodSpeedHQ/action@v4.17.0); update the matrix so the `mode`
values are only documented options (e.g., replace `"memory"` with `"walltime"`
or a comma-separated list like `"simulation,walltime"`), or change the step
input `mode: ${{ matrix.mode }}` to a fixed supported value and/or add a
separate matrix axis mapping memory benchmarks to the correct CodSpeed
configuration; ensure the `mode` input to CodSpeedHQ/action is one of the
supported strings (simulation or walltime) rather than `"memory"`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ea06633-d580-4838-a05d-b41422f4a838
📒 Files selected for processing (1)
.github/workflows/client-nav-benchmarks.yml
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
benchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.ts (1)
12-13: 💤 Low valueConsider using array filtering for conditional plugins across Vite configs. The
!!(condition) && plugin()pattern works because Vite filters falsy values, but explicit spread/filtering is clearer and more maintainable:
benchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.ts#L12-L13: replace!!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) && codspeedPlugin()with...(process.env.VITEST && process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()] : [])benchmarks/memory/client/scenarios/unique-location-churn/solid/vite.config.ts#L14-L15: same replacement patternbenchmarks/memory/client/scenarios/preload-churn/vue/vite.config.ts#L15-L16: same replacement pattern🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.ts` around lines 12 - 13, Replace the conditional plugin expression that relies on `!!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) && codspeedPlugin()` with an explicit spread of a conditional array so falsy values are not injected implicitly: in benchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.ts (lines 12-13) use the pattern that spreads either [codspeedPlugin()] or [] based on (process.env.VITEST && process.env.WITH_INSTRUMENTATION); do the same replacement in benchmarks/memory/client/scenarios/unique-location-churn/solid/vite.config.ts (lines 14-15) and benchmarks/memory/client/scenarios/preload-churn/vue/vite.config.ts (lines 15-16), keeping the plugin factory `codspeedPlugin()` and the environment checks unchanged.benchmarks/memory/client/scenarios/loader-data-retention/solid/vite.config.ts (1)
14-15: 💤 Low valueConsider using array filter for cleaner plugin configuration.
The double-negation short-circuit pattern
!!(condition) && plugin()works because Vite filters out falsy values, but it can be clearer to use array methods:...(process.env.VITEST && process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()] : [])This makes the intent more explicit and avoids
falseentries in the array.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/memory/client/scenarios/loader-data-retention/solid/vite.config.ts` around lines 14 - 15, Replace the double-negation short-circuit expression with an explicit spread/ternary so the plugin array contains either codspeedPlugin() or nothing; specifically, change the entry "!!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) && codspeedPlugin()" to use the pattern "...(process.env.VITEST && process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()] : [])" so the intent is clearer and no falsy value can appear in the plugins array.benchmarks/memory/server/scenarios/aborted-requests/vue/project.json (1)
15-15: ⚡ Quick winShell-specific environment variable syntax breaks Windows compatibility.
The
NODE_ENV=productionprefix is a Unix shell pattern that won't work in Windows Command Prompt or PowerShell. Consider usingcross-envor NX's built-in environment variable support:"command": "cross-env NODE_ENV=production vite build --config {projectRoot}/vite.config.ts"Or use NX environment options if available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/memory/server/scenarios/aborted-requests/vue/project.json` at line 15, The "command" property currently uses Unix-only syntax ("NODE_ENV=production vite build --config {projectRoot}/vite.config.ts"); replace this with a cross-platform invocation by prefixing with cross-env (e.g., "cross-env NODE_ENV=production vite build --config {projectRoot}/vite.config.ts") or, if using Nx, move NODE_ENV into the executor/target's environment configuration (use the target's "options" or "env" support) so the build runs on Windows as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/memory/client/scenarios/interrupted-navigations/shared.ts`:
- Around line 18-24: MountedApp currently types router as unknown which forces
an unchecked cast later; change the type to the concrete router interface used
by before()/interrupt() (e.g., replace "router: unknown" with "router:
InterruptedNavigationRouter") so the compiler enforces the router contract,
ensure InterruptedNavigationRouter is imported or declared in this module, and
keep MountTestApp and ResolveAllSlowLoaders signatures as they are; after this
change you can remove the force-cast at the usage site (around line 224) because
the returned MountedApp.router will already be correctly typed.
In `@benchmarks/memory/client/scenarios/loader-data-retention/shared.ts`:
- Around line 118-124: The waitForNextRender function can hang indefinitely;
update it to include a configurable timeout (e.g., 10–30s) so the returned
Promise rejects when the timeout elapses, and ensure the timeout is cleared when
resolveRendered is called; specifically, in waitForNextRender assign a timer via
setTimeout that rejects the promise with a clear error message, store/clear that
timer inside the resolveRendered handler, and propagate the rejection to callers
(e.g., navigateTo) so the benchmark fails fast instead of hanging.
- Around line 8-14: Change the MountedApp type so router is strongly typed as
LoaderDataRouter instead of unknown, update MountTestApp accordingly, and remove
the force-cast of mounted.router (i.e., stop using mounted.router as
LoaderDataRouter) so the returned mount implementations must match the
LoaderDataRouter contract at compile time; also add the necessary import for the
LoaderDataRouter type where MountedApp is declared.
In `@benchmarks/memory/client/scenarios/mount-unmount/react/project.json`:
- Line 15: Replace the platform-dependent inline env assignments with cross-env
in the listed project.json targets: in
benchmarks/memory/client/scenarios/mount-unmount/react/project.json (15-15)
update the build:client command to use cross-env NODE_ENV=production vite build;
in the same file (28-28) update build:client:flame to use cross-env
NODE_ENV=production vite build; in the same file (37-37) update test:flame to
use cross-env NODE_ENV=production node; in
benchmarks/memory/client/scenarios/mount-unmount/vue/project.json (15-15) update
build:client to cross-env NODE_ENV=production vite build; (28-28) update
build:client:flame to cross-env NODE_ENV=production vite build; (37-37) update
test:flame to cross-env NODE_ENV=production node; in
benchmarks/memory/client/scenarios/loader-data-retention/react/project.json
(15-15) update build:client to cross-env NODE_ENV=production vite build; (28-28)
update build:client:flame to cross-env NODE_ENV=production vite build; (37-37)
update test:flame to cross-env NODE_ENV=production node so the commands work
cross-platform.
In `@benchmarks/memory/client/scenarios/mount-unmount/shared.ts`:
- Around line 3-13: The MountedApp type currently declares router as unknown
which forces a runtime assertion elsewhere; change MountedApp.router from
unknown to RenderRouter (type MountedApp = { router: RenderRouter; unmount: ()
=> void }) and remove the explicit cast of mounted.router to RenderRouter where
used so the compiler enforces the contract; ensure MountTestApp still returns
MountedApp and update any affected usages to satisfy the stricter type if
needed.
In `@benchmarks/memory/client/scenarios/unique-location-churn/shared.ts`:
- Around line 13-16: MountedApp.router is typed as unknown which forces a
runtime cast to NavigationRouter; change the MountedApp type so router:
NavigationRouter (and add the necessary import for NavigationRouter), then
remove the cast at the site that currently casts to NavigationRouter (around the
code that references router at ~line 103) so callers use the typed property
directly.
---
Nitpick comments:
In
`@benchmarks/memory/client/scenarios/loader-data-retention/solid/vite.config.ts`:
- Around line 14-15: Replace the double-negation short-circuit expression with
an explicit spread/ternary so the plugin array contains either codspeedPlugin()
or nothing; specifically, change the entry "!!(process.env.VITEST &&
process.env.WITH_INSTRUMENTATION) && codspeedPlugin()" to use the pattern
"...(process.env.VITEST && process.env.WITH_INSTRUMENTATION ? [codspeedPlugin()]
: [])" so the intent is clearer and no falsy value can appear in the plugins
array.
In `@benchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.ts`:
- Around line 12-13: Replace the conditional plugin expression that relies on
`!!(process.env.VITEST && process.env.WITH_INSTRUMENTATION) && codspeedPlugin()`
with an explicit spread of a conditional array so falsy values are not injected
implicitly: in
benchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.ts (lines
12-13) use the pattern that spreads either [codspeedPlugin()] or [] based on
(process.env.VITEST && process.env.WITH_INSTRUMENTATION); do the same
replacement in
benchmarks/memory/client/scenarios/unique-location-churn/solid/vite.config.ts
(lines 14-15) and
benchmarks/memory/client/scenarios/preload-churn/vue/vite.config.ts (lines
15-16), keeping the plugin factory `codspeedPlugin()` and the environment checks
unchanged.
In `@benchmarks/memory/server/scenarios/aborted-requests/vue/project.json`:
- Line 15: The "command" property currently uses Unix-only syntax
("NODE_ENV=production vite build --config {projectRoot}/vite.config.ts");
replace this with a cross-platform invocation by prefixing with cross-env (e.g.,
"cross-env NODE_ENV=production vite build --config
{projectRoot}/vite.config.ts") or, if using Nx, move NODE_ENV into the
executor/target's environment configuration (use the target's "options" or "env"
support) so the build runs on Windows as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13bdfa67-9a85-48f2-8aa0-778341660347
📒 Files selected for processing (240)
.github/workflows/client-nav-benchmarks.yml.gitignorebenchmarks/memory/README.mdbenchmarks/memory/client/bench-utils.tsbenchmarks/memory/client/benchmark.tsbenchmarks/memory/client/flame-runner.tsbenchmarks/memory/client/jsdom.tsbenchmarks/memory/client/package.jsonbenchmarks/memory/client/runner.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/memory.bench.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/memory.flame.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/project.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/react/setup.tsbenchmarks/memory/client/scenarios/interrupted-navigations/react/tsconfig.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/shared.tsbenchmarks/memory/client/scenarios/interrupted-navigations/solid/memory.bench.tsbenchmarks/memory/client/scenarios/interrupted-navigations/solid/memory.flame.tsbenchmarks/memory/client/scenarios/interrupted-navigations/solid/project.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/solid/setup.tsbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/app.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/router.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routes/__root.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routes/fast.$id.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routes/index.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routes/slow.$id.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/solid/src/slow-loaders.tsbenchmarks/memory/client/scenarios/interrupted-navigations/solid/tsconfig.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/solid/vite.config.tsbenchmarks/memory/client/scenarios/interrupted-navigations/vue/memory.bench.tsbenchmarks/memory/client/scenarios/interrupted-navigations/vue/memory.flame.tsbenchmarks/memory/client/scenarios/interrupted-navigations/vue/project.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/vue/setup.tsbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/app.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/router.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/__root.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/fast.$id.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/index.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/slow.$id.tsxbenchmarks/memory/client/scenarios/interrupted-navigations/vue/src/slow-loaders.tsbenchmarks/memory/client/scenarios/interrupted-navigations/vue/tsconfig.jsonbenchmarks/memory/client/scenarios/interrupted-navigations/vue/vite.config.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/memory.bench.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/memory.flame.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/project.jsonbenchmarks/memory/client/scenarios/loader-data-retention/react/setup.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/src/loader-data.tsbenchmarks/memory/client/scenarios/loader-data-retention/react/tsconfig.jsonbenchmarks/memory/client/scenarios/loader-data-retention/shared.tsbenchmarks/memory/client/scenarios/loader-data-retention/solid/memory.bench.tsbenchmarks/memory/client/scenarios/loader-data-retention/solid/memory.flame.tsbenchmarks/memory/client/scenarios/loader-data-retention/solid/project.jsonbenchmarks/memory/client/scenarios/loader-data-retention/solid/setup.tsbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/app.tsxbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/loader-data.tsbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/router.tsxbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/routes/__root.tsxbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/routes/page.$id.tsxbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/routes/shell.index.tsxbenchmarks/memory/client/scenarios/loader-data-retention/solid/src/routes/shell.tsxbenchmarks/memory/client/scenarios/loader-data-retention/solid/tsconfig.jsonbenchmarks/memory/client/scenarios/loader-data-retention/solid/vite.config.tsbenchmarks/memory/client/scenarios/loader-data-retention/vue/memory.bench.tsbenchmarks/memory/client/scenarios/loader-data-retention/vue/memory.flame.tsbenchmarks/memory/client/scenarios/loader-data-retention/vue/project.jsonbenchmarks/memory/client/scenarios/loader-data-retention/vue/setup.tsbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/app.tsxbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/loader-data.tsbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/router.tsxbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/__root.tsxbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/page.$id.tsxbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/shell.index.tsxbenchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/shell.tsxbenchmarks/memory/client/scenarios/loader-data-retention/vue/tsconfig.jsonbenchmarks/memory/client/scenarios/loader-data-retention/vue/vite.config.tsbenchmarks/memory/client/scenarios/mount-unmount/react/memory.bench.tsbenchmarks/memory/client/scenarios/mount-unmount/react/memory.flame.tsbenchmarks/memory/client/scenarios/mount-unmount/react/project.jsonbenchmarks/memory/client/scenarios/mount-unmount/react/setup.tsbenchmarks/memory/client/scenarios/mount-unmount/react/tsconfig.jsonbenchmarks/memory/client/scenarios/mount-unmount/shared.tsbenchmarks/memory/client/scenarios/mount-unmount/solid/memory.bench.tsbenchmarks/memory/client/scenarios/mount-unmount/solid/memory.flame.tsbenchmarks/memory/client/scenarios/mount-unmount/solid/project.jsonbenchmarks/memory/client/scenarios/mount-unmount/solid/setup.tsbenchmarks/memory/client/scenarios/mount-unmount/solid/src/app.tsxbenchmarks/memory/client/scenarios/mount-unmount/solid/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/mount-unmount/solid/src/router.tsxbenchmarks/memory/client/scenarios/mount-unmount/solid/src/routes/__root.tsxbenchmarks/memory/client/scenarios/mount-unmount/solid/src/routes/a.tsxbenchmarks/memory/client/scenarios/mount-unmount/solid/tsconfig.jsonbenchmarks/memory/client/scenarios/mount-unmount/solid/vite.config.tsbenchmarks/memory/client/scenarios/mount-unmount/vue/memory.bench.tsbenchmarks/memory/client/scenarios/mount-unmount/vue/memory.flame.tsbenchmarks/memory/client/scenarios/mount-unmount/vue/project.jsonbenchmarks/memory/client/scenarios/mount-unmount/vue/setup.tsbenchmarks/memory/client/scenarios/mount-unmount/vue/src/app.tsxbenchmarks/memory/client/scenarios/mount-unmount/vue/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/mount-unmount/vue/src/router.tsxbenchmarks/memory/client/scenarios/mount-unmount/vue/src/routes/__root.tsxbenchmarks/memory/client/scenarios/mount-unmount/vue/src/routes/a.tsxbenchmarks/memory/client/scenarios/mount-unmount/vue/tsconfig.jsonbenchmarks/memory/client/scenarios/mount-unmount/vue/vite.config.tsbenchmarks/memory/client/scenarios/navigation-churn/react/memory.bench.tsbenchmarks/memory/client/scenarios/navigation-churn/react/memory.flame.tsbenchmarks/memory/client/scenarios/navigation-churn/react/project.jsonbenchmarks/memory/client/scenarios/navigation-churn/react/setup.tsbenchmarks/memory/client/scenarios/navigation-churn/react/tsconfig.jsonbenchmarks/memory/client/scenarios/navigation-churn/shared.tsbenchmarks/memory/client/scenarios/navigation-churn/solid/memory.bench.tsbenchmarks/memory/client/scenarios/navigation-churn/solid/memory.flame.tsbenchmarks/memory/client/scenarios/navigation-churn/solid/project.jsonbenchmarks/memory/client/scenarios/navigation-churn/solid/setup.tsbenchmarks/memory/client/scenarios/navigation-churn/solid/src/app.tsxbenchmarks/memory/client/scenarios/navigation-churn/solid/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/navigation-churn/solid/src/router.tsxbenchmarks/memory/client/scenarios/navigation-churn/solid/src/routes/__root.tsxbenchmarks/memory/client/scenarios/navigation-churn/solid/src/routes/a.tsxbenchmarks/memory/client/scenarios/navigation-churn/solid/src/routes/b.tsxbenchmarks/memory/client/scenarios/navigation-churn/solid/tsconfig.jsonbenchmarks/memory/client/scenarios/navigation-churn/solid/vite.config.tsbenchmarks/memory/client/scenarios/navigation-churn/vue/memory.bench.tsbenchmarks/memory/client/scenarios/navigation-churn/vue/memory.flame.tsbenchmarks/memory/client/scenarios/navigation-churn/vue/project.jsonbenchmarks/memory/client/scenarios/navigation-churn/vue/setup.tsbenchmarks/memory/client/scenarios/navigation-churn/vue/src/app.tsxbenchmarks/memory/client/scenarios/navigation-churn/vue/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/navigation-churn/vue/src/router.tsxbenchmarks/memory/client/scenarios/navigation-churn/vue/src/routes/__root.tsxbenchmarks/memory/client/scenarios/navigation-churn/vue/src/routes/a.tsxbenchmarks/memory/client/scenarios/navigation-churn/vue/src/routes/b.tsxbenchmarks/memory/client/scenarios/navigation-churn/vue/tsconfig.jsonbenchmarks/memory/client/scenarios/navigation-churn/vue/vite.config.tsbenchmarks/memory/client/scenarios/preload-churn/react/memory.bench.tsbenchmarks/memory/client/scenarios/preload-churn/react/memory.flame.tsbenchmarks/memory/client/scenarios/preload-churn/react/project.jsonbenchmarks/memory/client/scenarios/preload-churn/react/setup.tsbenchmarks/memory/client/scenarios/preload-churn/react/tsconfig.jsonbenchmarks/memory/client/scenarios/preload-churn/shared.tsbenchmarks/memory/client/scenarios/preload-churn/solid/memory.bench.tsbenchmarks/memory/client/scenarios/preload-churn/solid/memory.flame.tsbenchmarks/memory/client/scenarios/preload-churn/solid/project.jsonbenchmarks/memory/client/scenarios/preload-churn/solid/setup.tsbenchmarks/memory/client/scenarios/preload-churn/solid/src/app.tsxbenchmarks/memory/client/scenarios/preload-churn/solid/src/item-payload.tsbenchmarks/memory/client/scenarios/preload-churn/solid/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/preload-churn/solid/src/router.tsxbenchmarks/memory/client/scenarios/preload-churn/solid/src/routes/__root.tsxbenchmarks/memory/client/scenarios/preload-churn/solid/src/routes/index.tsxbenchmarks/memory/client/scenarios/preload-churn/solid/src/routes/items.$id.tsxbenchmarks/memory/client/scenarios/preload-churn/solid/tsconfig.jsonbenchmarks/memory/client/scenarios/preload-churn/solid/vite.config.tsbenchmarks/memory/client/scenarios/preload-churn/vue/memory.bench.tsbenchmarks/memory/client/scenarios/preload-churn/vue/memory.flame.tsbenchmarks/memory/client/scenarios/preload-churn/vue/project.jsonbenchmarks/memory/client/scenarios/preload-churn/vue/setup.tsbenchmarks/memory/client/scenarios/preload-churn/vue/src/app.tsxbenchmarks/memory/client/scenarios/preload-churn/vue/src/item-payload.tsbenchmarks/memory/client/scenarios/preload-churn/vue/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/preload-churn/vue/src/router.tsxbenchmarks/memory/client/scenarios/preload-churn/vue/src/routes/__root.tsxbenchmarks/memory/client/scenarios/preload-churn/vue/src/routes/index.tsxbenchmarks/memory/client/scenarios/preload-churn/vue/src/routes/items.$id.tsxbenchmarks/memory/client/scenarios/preload-churn/vue/tsconfig.jsonbenchmarks/memory/client/scenarios/preload-churn/vue/vite.config.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/memory.bench.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/memory.flame.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/project.jsonbenchmarks/memory/client/scenarios/unique-location-churn/react/setup.tsbenchmarks/memory/client/scenarios/unique-location-churn/react/tsconfig.jsonbenchmarks/memory/client/scenarios/unique-location-churn/shared.tsbenchmarks/memory/client/scenarios/unique-location-churn/solid/memory.bench.tsbenchmarks/memory/client/scenarios/unique-location-churn/solid/memory.flame.tsbenchmarks/memory/client/scenarios/unique-location-churn/solid/project.jsonbenchmarks/memory/client/scenarios/unique-location-churn/solid/setup.tsbenchmarks/memory/client/scenarios/unique-location-churn/solid/src/app.tsxbenchmarks/memory/client/scenarios/unique-location-churn/solid/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/unique-location-churn/solid/src/router.tsxbenchmarks/memory/client/scenarios/unique-location-churn/solid/src/routes/__root.tsxbenchmarks/memory/client/scenarios/unique-location-churn/solid/src/routes/items.$id.tsxbenchmarks/memory/client/scenarios/unique-location-churn/solid/tsconfig.jsonbenchmarks/memory/client/scenarios/unique-location-churn/solid/vite.config.tsbenchmarks/memory/client/scenarios/unique-location-churn/vue/memory.bench.tsbenchmarks/memory/client/scenarios/unique-location-churn/vue/memory.flame.tsbenchmarks/memory/client/scenarios/unique-location-churn/vue/project.jsonbenchmarks/memory/client/scenarios/unique-location-churn/vue/setup.tsbenchmarks/memory/client/scenarios/unique-location-churn/vue/src/app.tsxbenchmarks/memory/client/scenarios/unique-location-churn/vue/src/routeTree.gen.tsbenchmarks/memory/client/scenarios/unique-location-churn/vue/src/router.tsxbenchmarks/memory/client/scenarios/unique-location-churn/vue/src/routes/__root.tsxbenchmarks/memory/client/scenarios/unique-location-churn/vue/src/routes/items.$id.tsxbenchmarks/memory/client/scenarios/unique-location-churn/vue/tsconfig.jsonbenchmarks/memory/client/scenarios/unique-location-churn/vue/vite.config.tsbenchmarks/memory/client/vitest.react.config.tsbenchmarks/memory/client/vitest.setup.tsbenchmarks/memory/client/vitest.solid.config.tsbenchmarks/memory/client/vitest.vue.config.tsbenchmarks/memory/flame-control.tsbenchmarks/memory/run-flame.mjsbenchmarks/memory/server/bench-utils.tsbenchmarks/memory/server/benchmark.tsbenchmarks/memory/server/flame-runner.tsbenchmarks/memory/server/package.jsonbenchmarks/memory/server/runner.tsbenchmarks/memory/server/scenarios/aborted-requests/react/memory.bench.tsbenchmarks/memory/server/scenarios/aborted-requests/react/memory.flame.tsbenchmarks/memory/server/scenarios/aborted-requests/react/project.jsonbenchmarks/memory/server/scenarios/aborted-requests/react/setup.tsbenchmarks/memory/server/scenarios/aborted-requests/react/tsconfig.jsonbenchmarks/memory/server/scenarios/aborted-requests/shared.tsbenchmarks/memory/server/scenarios/aborted-requests/solid/memory.bench.tsbenchmarks/memory/server/scenarios/aborted-requests/solid/memory.flame.tsbenchmarks/memory/server/scenarios/aborted-requests/solid/project.jsonbenchmarks/memory/server/scenarios/aborted-requests/solid/setup.tsbenchmarks/memory/server/scenarios/aborted-requests/solid/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/aborted-requests/solid/src/router.tsxbenchmarks/memory/server/scenarios/aborted-requests/solid/src/routes/__root.tsxbenchmarks/memory/server/scenarios/aborted-requests/solid/src/routes/index.tsxbenchmarks/memory/server/scenarios/aborted-requests/solid/src/routes/stream.$id.tsxbenchmarks/memory/server/scenarios/aborted-requests/solid/tsconfig.jsonbenchmarks/memory/server/scenarios/aborted-requests/solid/vite.config.tsbenchmarks/memory/server/scenarios/aborted-requests/vue/memory.bench.tsbenchmarks/memory/server/scenarios/aborted-requests/vue/memory.flame.tsbenchmarks/memory/server/scenarios/aborted-requests/vue/project.jsonbenchmarks/memory/server/scenarios/aborted-requests/vue/setup.tsbenchmarks/memory/server/scenarios/aborted-requests/vue/src/routeTree.gen.tsbenchmarks/memory/server/scenarios/aborted-requests/vue/src/router.tsxbenchmarks/memory/server/scenarios/aborted-requests/vue/src/routes/__root.tsxbenchmarks/memory/server/scenarios/aborted-requests/vue/src/routes/index.tsxbenchmarks/memory/server/scenarios/aborted-requests/vue/src/routes/stream.$id.tsxbenchmarks/memory/server/scenarios/aborted-requests/vue/tsconfig.jsonbenchmarks/memory/server/scenarios/aborted-requests/vue/vite.config.tsbenchmarks/memory/server/scenarios/error-paths/react/memory.bench.tsbenchmarks/memory/server/scenarios/error-paths/react/memory.flame.tsbenchmarks/memory/server/scenarios/error-paths/react/project.jsonbenchmarks/memory/server/scenarios/error-paths/react/setup.tsbenchmarks/memory/server/scenarios/error-paths/react/tsconfig.json
✅ Files skipped from review due to trivial changes (83)
- benchmarks/memory/client/scenarios/loader-data-retention/solid/memory.flame.ts
- benchmarks/memory/client/scenarios/loader-data-retention/solid/src/routes/shell.index.tsx
- benchmarks/memory/client/scenarios/loader-data-retention/solid/memory.bench.ts
- benchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/shell.tsx
- benchmarks/memory/server/scenarios/aborted-requests/vue/setup.ts
- benchmarks/memory/server/scenarios/aborted-requests/vue/memory.flame.ts
- benchmarks/memory/client/vitest.vue.config.ts
- benchmarks/memory/client/scenarios/loader-data-retention/solid/src/routes/page.$id.tsx
- benchmarks/memory/client/scenarios/loader-data-retention/vue/memory.flame.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/solid/memory.flame.ts
- benchmarks/memory/client/scenarios/navigation-churn/solid/memory.flame.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/vue/memory.bench.ts
- benchmarks/memory/client/scenarios/unique-location-churn/solid/memory.bench.ts
- benchmarks/memory/client/scenarios/unique-location-churn/vue/setup.ts
- benchmarks/memory/server/runner.ts
- benchmarks/memory/client/scenarios/unique-location-churn/solid/src/router.tsx
- benchmarks/memory/client/scenarios/mount-unmount/react/memory.flame.ts
- benchmarks/memory/client/scenarios/loader-data-retention/solid/src/router.tsx
- benchmarks/memory/client/scenarios/navigation-churn/solid/memory.bench.ts
- benchmarks/memory/client/scenarios/preload-churn/solid/memory.bench.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/slow.$id.tsx
- benchmarks/memory/client/benchmark.ts
- benchmarks/memory/client/scenarios/navigation-churn/react/tsconfig.json
- benchmarks/memory/server/scenarios/aborted-requests/solid/src/routes/index.tsx
- benchmarks/memory/client/scenarios/navigation-churn/vue/tsconfig.json
- benchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/index.tsx
- benchmarks/memory/server/scenarios/aborted-requests/solid/tsconfig.json
- benchmarks/memory/client/scenarios/preload-churn/solid/memory.flame.ts
- benchmarks/memory/client/scenarios/mount-unmount/solid/src/router.tsx
- benchmarks/memory/client/scenarios/mount-unmount/vue/src/routes/a.tsx
- benchmarks/memory/client/scenarios/interrupted-navigations/solid/src/router.tsx
- benchmarks/memory/client/scenarios/navigation-churn/vue/vite.config.ts
- benchmarks/memory/client/scenarios/preload-churn/solid/src/router.tsx
- benchmarks/memory/client/scenarios/interrupted-navigations/solid/tsconfig.json
- benchmarks/memory/client/scenarios/mount-unmount/vue/src/router.tsx
- benchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/shell.index.tsx
- benchmarks/memory/server/scenarios/aborted-requests/solid/memory.bench.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/vue/vite.config.ts
- benchmarks/memory/server/benchmark.ts
- benchmarks/memory/client/scenarios/navigation-churn/vue/memory.flame.ts
- benchmarks/memory/client/runner.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routes/__root.tsx
- benchmarks/memory/client/scenarios/loader-data-retention/vue/vite.config.ts
- benchmarks/memory/client/scenarios/loader-data-retention/vue/src/router.tsx
- benchmarks/memory/client/scenarios/navigation-churn/solid/src/routes/a.tsx
- benchmarks/memory/server/scenarios/aborted-requests/vue/src/routes/__root.tsx
- benchmarks/memory/client/scenarios/navigation-churn/solid/src/routes/__root.tsx
- benchmarks/memory/client/scenarios/unique-location-churn/vue/tsconfig.json
- benchmarks/memory/client/scenarios/interrupted-navigations/solid/memory.bench.ts
- benchmarks/memory/client/scenarios/mount-unmount/vue/tsconfig.json
- benchmarks/memory/client/scenarios/loader-data-retention/solid/tsconfig.json
- benchmarks/memory/client/scenarios/unique-location-churn/vue/src/routes/__root.tsx
- benchmarks/memory/client/scenarios/navigation-churn/solid/vite.config.ts
- benchmarks/memory/client/scenarios/preload-churn/vue/memory.flame.ts
- benchmarks/memory/client/scenarios/loader-data-retention/vue/src/routes/__root.tsx
- benchmarks/memory/client/scenarios/unique-location-churn/solid/setup.ts
- benchmarks/memory/client/scenarios/unique-location-churn/vue/vite.config.ts
- benchmarks/memory/client/scenarios/navigation-churn/vue/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/navigation-churn/vue/src/router.tsx
- benchmarks/memory/client/scenarios/unique-location-churn/vue/src/router.tsx
- benchmarks/memory/client/scenarios/mount-unmount/vue/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/mount-unmount/solid/tsconfig.json
- benchmarks/memory/client/scenarios/loader-data-retention/solid/project.json
- benchmarks/memory/client/scenarios/mount-unmount/solid/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/loader-data-retention/react/tsconfig.json
- benchmarks/memory/client/scenarios/preload-churn/solid/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/vue/src/routes/__root.tsx
- benchmarks/memory/client/scenarios/loader-data-retention/react/setup.ts
- benchmarks/memory/client/scenarios/preload-churn/vue/tsconfig.json
- benchmarks/memory/client/scenarios/unique-location-churn/solid/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/navigation-churn/solid/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/solid/src/routeTree.gen.ts
- benchmarks/memory/server/scenarios/aborted-requests/vue/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/preload-churn/vue/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/preload-churn/react/project.json
- benchmarks/memory/client/scenarios/preload-churn/react/tsconfig.json
- benchmarks/memory/server/scenarios/aborted-requests/solid/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/unique-location-churn/react/tsconfig.json
- benchmarks/memory/client/scenarios/unique-location-churn/vue/src/routeTree.gen.ts
- benchmarks/memory/client/scenarios/loader-data-retention/vue/src/routeTree.gen.ts
- benchmarks/memory/README.md
- benchmarks/memory/client/scenarios/loader-data-retention/solid/src/routeTree.gen.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- benchmarks/memory/client/vitest.react.config.ts
- benchmarks/memory/client/bench-utils.ts
- benchmarks/memory/client/scenarios/navigation-churn/react/setup.ts
- benchmarks/memory/server/scenarios/error-paths/react/tsconfig.json
- benchmarks/memory/client/scenarios/mount-unmount/react/tsconfig.json
- benchmarks/memory/client/scenarios/navigation-churn/react/memory.bench.ts
- benchmarks/memory/client/scenarios/interrupted-navigations/react/tsconfig.json
- benchmarks/memory/server/bench-utils.ts
- benchmarks/memory/client/scenarios/loader-data-retention/react/src/loader-data.ts
- benchmarks/memory/server/scenarios/aborted-requests/react/project.json
Comment on lines
+8
to
+14
| type MountedApp = { | ||
| router: unknown | ||
| unmount: () => void | ||
| } | ||
|
|
||
| type MountTestApp = (container: HTMLDivElement) => MountedApp | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where the unsafe cast/unknown contract exists and inspect setup providers.
rg -nP 'router:\s+unknown|as\s+LoaderDataRouter|createSetup\(' -C2 benchmarks/memory/client/scenarios/loader-data-retention
fd -i 'setup.ts' benchmarks/memory/client/scenarios/loader-data-retention --exec sed -n '1,220p' {}Repository: TanStack/router
Length of output: 3586
Replace MountedApp.router: unknown with LoaderDataRouter and remove the mounted.router as LoaderDataRouter cast.
benchmarks/memory/client/scenarios/loader-data-retention/shared.ts declares MountedApp.router as unknown and then force-casts mounted.router at lines 134-136, so drift between each framework’s mountTestApp return type and the LoaderDataRouter contract won’t be caught at compile time.
Proposed fix
type MountedApp = {
- router: unknown
+ router: LoaderDataRouter
unmount: () => void
}
@@
- const router = mounted.router as LoaderDataRouter
+ const router = mounted.routerAs per coding guidelines, **/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/memory/client/scenarios/loader-data-retention/shared.ts` around
lines 8 - 14, Change the MountedApp type so router is strongly typed as
LoaderDataRouter instead of unknown, update MountTestApp accordingly, and remove
the force-cast of mounted.router (i.e., stop using mounted.router as
LoaderDataRouter) so the returned mount implementations must match the
LoaderDataRouter contract at compile time; also add the necessary import for the
LoaderDataRouter type where MountedApp is declared.
Source: Coding guidelines
Comment on lines
+118
to
+124
| function waitForNextRender(pathname: string) { | ||
| expectedRenderedPath = pathname | ||
|
|
||
| return new Promise<void>((resolve) => { | ||
| resolveRendered = resolve | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Add a timeout to render waiting to prevent benchmark hangs.
navigateTo can wait forever if the expected onRendered event never arrives, which can stall CI runs instead of failing fast.
Proposed fix
-function waitForNextRender(pathname: string) {
+function waitForNextRender(pathname: string, timeoutMs = 5_000) {
expectedRenderedPath = pathname
- return new Promise<void>((resolve) => {
- resolveRendered = resolve
+ return new Promise<void>((resolve, reject) => {
+ const timer = setTimeout(() => {
+ resolveRendered = () => {}
+ expectedRenderedPath = undefined
+ reject(new Error(`Timed out waiting for render: ${pathname}`))
+ }, timeoutMs)
+
+ resolveRendered = () => {
+ clearTimeout(timer)
+ resolve()
+ }
})
}Also applies to: 152-162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/memory/client/scenarios/loader-data-retention/shared.ts` around
lines 118 - 124, The waitForNextRender function can hang indefinitely; update it
to include a configurable timeout (e.g., 10–30s) so the returned Promise rejects
when the timeout elapses, and ensure the timeout is cleared when resolveRendered
is called; specifically, in waitForNextRender assign a timer via setTimeout that
rejects the promise with a clear error message, store/clear that timer inside
the resolveRendered handler, and propagate the rejection to callers (e.g.,
navigateTo) so the benchmark fails fast instead of hanging.
Comment on lines
+3
to
+13
| type MountedApp = { | ||
| router: unknown | ||
| unmount: () => void | ||
| } | ||
|
|
||
| type MountTestApp = (container: HTMLDivElement) => MountedApp | ||
|
|
||
| type RenderRouter = { | ||
| load: () => Promise<void> | ||
| subscribe: (event: 'onRendered', listener: () => void) => () => void | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Tighten router typing instead of casting from unknown.
Line 54 force-casts mounted.router to RenderRouter, which bypasses strict checks and can hide contract mismatches until runtime. Type MountedApp.router as RenderRouter directly and remove the assertion.
♻️ Suggested change
type MountedApp = {
- router: unknown
+ router: RenderRouter
unmount: () => void
}
@@
- const router = mounted.router as RenderRouter
+ const router = mounted.routerAs per coding guidelines, **/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.
Also applies to: 54-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/memory/client/scenarios/mount-unmount/shared.ts` around lines 3 -
13, The MountedApp type currently declares router as unknown which forces a
runtime assertion elsewhere; change MountedApp.router from unknown to
RenderRouter (type MountedApp = { router: RenderRouter; unmount: () => void })
and remove the explicit cast of mounted.router to RenderRouter where used so the
compiler enforces the contract; ensure MountTestApp still returns MountedApp and
update any affected usages to satisfy the stricter type if needed.
Source: Coding guidelines
Comment on lines
+13
to
+16
| type MountedApp = { | ||
| router: unknown | ||
| unmount: () => void | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify there are no remaining weak router contracts/casts in this scenario.
rg -n --type=ts --type=tsx -C2 'router:\s*unknown|as\s+NavigationRouter' benchmarks/memory/client/scenarios/unique-location-churnRepository: TanStack/router
Length of output: 87
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="benchmarks/memory/client/scenarios/unique-location-churn/shared.ts"
echo "== Matching router: unknown in target file =="
rg -n 'router:\s*unknown' "$FILE" || true
echo "== Matching 'as NavigationRouter' in target file =="
rg -n 'as\s+NavigationRouter' "$FILE" || true
echo "== Show relevant sections (lines 1-180) =="
nl -ba "$FILE" | sed -n '1,180p'Repository: TanStack/router
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="benchmarks/memory/client/scenarios/unique-location-churn/shared.ts"
echo "== Matching router: unknown in target file =="
rg -n 'router:\s*unknown' "$FILE" || true
echo "== Matching 'as NavigationRouter' in target file =="
rg -n 'as\s+NavigationRouter' "$FILE" || true
echo "== Show relevant sections (lines 1-180) =="
nl -ba "$FILE" | sed -n '1,180p'Repository: TanStack/router
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="benchmarks/memory/client/scenarios/unique-location-churn/shared.ts"
echo "== Matches =="
rg -n 'router:\s*unknown|as\s+NavigationRouter' "$FILE" || true
echo
echo "== Lines 1-60 =="
cat -n "$FILE" | sed -n '1,60p'
echo
echo "== Lines 90-130 =="
cat -n "$FILE" | sed -n '90,130p'
echo
echo "== Any other NavigationRouter casts in repo path =="
rg -n --type=ts --type=tsx 'as\s+NavigationRouter' benchmarks/memory/client/scenarios/unique-location-churn || trueRepository: TanStack/router
Length of output: 3440
Strengthen MountedApp.router typing to eliminate unknown + cast.
In benchmarks/memory/client/scenarios/unique-location-churn/shared.ts, MountedApp.router is unknown and is cast to NavigationRouter at lines ~103; typing the field as NavigationRouter directly removes the runtime-unsafe assertion.
♻️ Proposed change
type MountedApp = {
- router: unknown
+ router: NavigationRouter
unmount: () => void
}
@@
- const router = mounted.router as NavigationRouter
+ const router = mounted.router🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/memory/client/scenarios/unique-location-churn/shared.ts` around
lines 13 - 16, MountedApp.router is typed as unknown which forces a runtime cast
to NavigationRouter; change the MountedApp type so router: NavigationRouter (and
add the necessary import for NavigationRouter), then remove the cast at the site
that currently casts to NavigationRouter (around the code that references router
at ~line 103) so callers use the typed property directly.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit