improve bundle size benchmarks and add initial skill#7450
improve bundle size benchmarks and add initial skill#7450schiller-manuel wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughPR expands bundle-size benchmarking with scenario filtering, per-file sizing, sourcemap-based source attribution, optional Nx package builds, a query/diff/history/analyze CLI suite, VM-based data parsing, scenario scaffolding, README/skill docs, and adds a ts-symbol-references CLI. ChangesBundle-Size Benchmarking Tools
TypeScript Symbol References Tool
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit a48f9ae
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@scripts/benchmarks/bundle-size/analyze.mjs`:
- Around line 15-16: Validate the `--top-sources` option (key 'top-sources')
before using it with Array.prototype.slice: parse the value to a number (e.g.,
using parseInt or Number), check Number.isInteger(value) and value > 0, and if
the check fails output a clear error and exit non-zero; update the code paths
where 'top-sources' is read (and where it's applied with slice around the slice
usage at the current slice call) to use the validated integer variable so bad
inputs fail explicitly.
In `@scripts/benchmarks/bundle-size/diff.mjs`:
- Around line 34-37: The code currently builds ids from values.id or from union
of baselineById/currentById, which hides typos when a user passes --id that
doesn't exist; add a guard: if values.id is truthy and neither
baselineById.has(values.id) nor currentById.has(values.id), throw a clear error
(or exit non‑zero) indicating the requested id is unknown; update the logic
around the ids assignment (referencing values.id, baselineById, currentById, and
the ids variable) to perform this existence check before proceeding.
In `@scripts/benchmarks/bundle-size/history.mjs`:
- Around line 15-16: The CLI option 'top-deltas' must be validated before being
used for array slicing: locate the option definition (key 'top-deltas') and the
code that reads argv['top-deltas'] (and any usage like .slice(0, topDeltas)
around the history processing) and add a check that the parsed value is a finite
positive integer (Number.isFinite and Number.isInteger and > 0). If the check
fails, throw or exit with a clear error message stating that --top-deltas must
be a positive integer. Apply the same validation at both usage sites referenced
in the diff (the option definition and the slicing usage).
- Around line 22-25: The code in parseHistory uses vm.runInNewContext to
evaluate user-provided payloads (the block where
trimmed.startsWith('window.BENCHMARK_DATA') creates sandbox and calls
vm.runInNewContext), which allows arbitrary code execution; instead, strip the
optional "window.BENCHMARK_DATA =" prefix and any trailing semicolon from the
trimmed string and parse the remaining content with JSON.parse to return the
benchmark object (i.e., remove the sandbox/vm.runInNewContext logic and replace
it with safe string extraction + JSON.parse).
In `@scripts/benchmarks/bundle-size/measure.mjs`:
- Around line 158-161: The code builds the requested array from the filter
string but doesn't reject cases where the input was only commas/whitespace,
producing an empty requested list; update the logic after the requested
calculation in measure.mjs to detect requested.length === 0 (or equivalently
!requested.length) and fail fast by emitting a clear error mentioning the
--scenario selector was empty and exiting non‑zero (e.g., throw an Error or call
process.exit(1)); reference the requested variable and the code path that
consumes it so the script never proceeds to publish an empty snapshot when no
scenarios were selected.
- Around line 739-743: The code forces at least 1 byte for tail mappings (bytes
= Math.max(1, ...)) which overcounts zero-width mappings; change the calculation
in the block that computes bytes (the variables: previousSource, previousColumn,
generatedLines, lineIndex) to allow zero-width results by using Math.max(0,
(generatedLines[lineIndex]?.length || previousColumn) - previousColumn) (or
equivalent non-negative clamp) so zero-length tail mappings aren't rounded up.
- Around line 954-956: status.command stores a shell command but inserts
args.scenario raw, causing multi-token values like "react-router.minimal,
solid-router.full" to split; update the interpolation in measure.mjs where
status.command is built so args.scenario is shell-quoted or escaped (e.g., wrap
in a quoted string via JSON.stringify or a proper shell-escape helper) before
concatenation, ensuring the persisted rerun command can be pasted verbatim;
apply the same quoting approach to any other arg interpolations in the same
expression if they can contain spaces or punctuation.
In `@scripts/benchmarks/bundle-size/pr-report.mjs`:
- Around line 71-74: The code currently executes untrusted text via
vm.runInNewContext(trimmed, sandbox...) to obtain window.BENCHMARK_DATA; replace
this with a safe extraction: use a regular expression against trimmed to capture
the JSON assigned to window.BENCHMARK_DATA, validate the match, and then parse
the captured JSON with JSON.parse() to return the data instead of executing
code. Locate the block using trimmed and vm.runInNewContext and update it to
extract the RHS of the window.BENCHMARK_DATA assignment via regex, check for a
match, and call JSON.parse on the captured string (handle parse errors) before
returning the resulting object.
🪄 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: 54471d3d-46c0-47c9-bda8-09944273e2f3
📒 Files selected for processing (11)
benchmarks/bundle-size/README.mdbenchmarks/bundle-size/package.jsonpackage.jsonscripts/benchmarks/bundle-size/analyze.mjsscripts/benchmarks/bundle-size/diff.mjsscripts/benchmarks/bundle-size/history.mjsscripts/benchmarks/bundle-size/measure.mjsscripts/benchmarks/bundle-size/pr-report.mjsscripts/benchmarks/bundle-size/query.mjsscripts/ts-symbol-references.mjsskills/bundle-size-optimization/SKILL.md
| 'top-sources': { type: 'string', default: '30' }, | ||
| json: { type: 'boolean' }, |
There was a problem hiding this comment.
Add validation for --top-sources.
Please validate that --top-sources is a positive integer before applying slice, so bad inputs fail explicitly instead of producing confusing output.
Also applies to: 52-52
🤖 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 `@scripts/benchmarks/bundle-size/analyze.mjs` around lines 15 - 16, Validate
the `--top-sources` option (key 'top-sources') before using it with
Array.prototype.slice: parse the value to a number (e.g., using parseInt or
Number), check Number.isInteger(value) and value > 0, and if the check fails
output a clear error and exit non-zero; update the code paths where
'top-sources' is read (and where it's applied with slice around the slice usage
at the current slice call) to use the validated integer variable so bad inputs
fail explicitly.
| const ids = values.id | ||
| ? [values.id] | ||
| : [...new Set([...baselineById.keys(), ...currentById.keys()])].sort() | ||
|
|
There was a problem hiding this comment.
Fail fast on unknown --id.
When --id is provided but absent from both baseline and current, returning an n/a row hides typos. Throwing a clear error matches the behavior of the other bundle-size CLIs.
Suggested guard
const baselineById = byId(readCurrent(values.baseline))
const currentById = byId(readCurrent(values.current))
+
+if (
+ values.id &&
+ !baselineById.has(values.id) &&
+ !currentById.has(values.id)
+) {
+ throw new Error(`Unknown bundle-size metric: ${values.id}`)
+}
+
const ids = values.id
? [values.id]
: [...new Set([...baselineById.keys(), ...currentById.keys()])].sort()📝 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.
| const ids = values.id | |
| ? [values.id] | |
| : [...new Set([...baselineById.keys(), ...currentById.keys()])].sort() | |
| const baselineById = byId(readCurrent(values.baseline)) | |
| const currentById = byId(readCurrent(values.current)) | |
| if ( | |
| values.id && | |
| !baselineById.has(values.id) && | |
| !currentById.has(values.id) | |
| ) { | |
| throw new Error(`Unknown bundle-size metric: ${values.id}`) | |
| } | |
| const ids = values.id | |
| ? [values.id] | |
| : [...new Set([...baselineById.keys(), ...currentById.keys()])].sort() |
🤖 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 `@scripts/benchmarks/bundle-size/diff.mjs` around lines 34 - 37, The code
currently builds ids from values.id or from union of baselineById/currentById,
which hides typos when a user passes --id that doesn't exist; add a guard: if
values.id is truthy and neither baselineById.has(values.id) nor
currentById.has(values.id), throw a clear error (or exit non‑zero) indicating
the requested id is unknown; update the logic around the ids assignment
(referencing values.id, baselineById, currentById, and the ids variable) to
perform this existence check before proceeding.
| 'top-deltas': { type: 'string', default: '20' }, | ||
| json: { type: 'boolean' }, |
There was a problem hiding this comment.
Validate --top-deltas input before slicing.
Invalid or negative values currently degrade into ambiguous output. Add a finite positive-integer check and throw a clear error.
Also applies to: 75-75
🤖 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 `@scripts/benchmarks/bundle-size/history.mjs` around lines 15 - 16, The CLI
option 'top-deltas' must be validated before being used for array slicing:
locate the option definition (key 'top-deltas') and the code that reads
argv['top-deltas'] (and any usage like .slice(0, topDeltas) around the history
processing) and add a check that the parsed value is a finite positive integer
(Number.isFinite and Number.isInteger and > 0). If the check fails, throw or
exit with a clear error message stating that --top-deltas must be a positive
integer. Apply the same validation at both usage sites referenced in the diff
(the option definition and the slicing usage).
| if (trimmed.startsWith('window.BENCHMARK_DATA')) { | ||
| const sandbox = { window: {} } | ||
| vm.runInNewContext(trimmed, sandbox, { timeout: 1000 }) | ||
| return sandbox.window.BENCHMARK_DATA |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What do Node.js docs say about using node:vm with untrusted code, and is it recommended as a security boundary?
💡 Result:
The Node.js documentation explicitly states that the node:vm module is not a security mechanism and should not be used to run untrusted code [1][2]. The official documentation repeatedly emphasizes this warning, clarifying that the term "sandbox" is used in the context of the module only to refer to a separate execution context, not to provide security guarantees [3][4]. It is not recommended as a security boundary because it does not provide true isolation [5]. The node:vm module allows code to be compiled and run within V8 virtual machine contexts, which is primarily a tool for scoping variables and creating separate global environments [1][2]. Malicious code can easily escape these contexts by walking up the prototype chain to access host-realm objects or the host process, effectively bypassing the intended isolation [5]. Consequently, attempting to use node:vm to safely execute untrusted code is considered inherently insecure and is not a valid approach for building secure sandboxes [6][5].
Citations:
- 1: https://nodejs.org/docs/v22.11.0/api/vm.html
- 2: https://nodejs.org/api/vm.html
- 3: https://nodejs.org/dist/v10.0.0/docs/api/vm.html
- 4: https://nodejs.org/dist/v11.0.0/docs/api/vm.html
- 5: https://dev.to/dendrite_soup/nodevm-is-not-a-sandbox-stop-using-it-like-one-2f74
- 6: docs: Clarification around real world risks and use cases of VM module nodejs/node#40718
🏁 Script executed:
# Find the file and examine the parseHistory function and surrounding context
cat -n scripts/benchmarks/bundle-size/history.mjsRepository: TanStack/router
Length of output: 2937
🏁 Script executed:
# Search for CLI argument handling related to history
rg -i "history" scripts/benchmarks/bundle-size/ -A 2 -B 2Repository: TanStack/router
Length of output: 13412
🏁 Script executed:
# Search for how the history file is used in the benchmark script
rg -i "\.mjs|history" --type javascript | grep -i benchmark | head -20Repository: TanStack/router
Length of output: 94
Do not execute history file payloads with vm.runInNewContext.
The --history CLI argument (line 44-45) accepts user-provided files and passes them to parseHistory, which evaluates code via vm.runInNewContext (lines 22-25). Node.js documentation explicitly warns that the vm module is not a security mechanism and should not be used to run untrusted code—it does not provide true isolation.
Since the benchmark data is JSON (optionally wrapped with window.BENCHMARK_DATA = ), replace the code execution with string extraction and JSON.parse:
if (trimmed.startsWith('window.BENCHMARK_DATA')) {
const jsonStr = trimmed.replace(/^window\.BENCHMARK_DATA\s*=\s*/, '').replace(/;$/, '')
return JSON.parse(jsonStr)
}This eliminates arbitrary code execution while parsing the data safely.
🤖 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 `@scripts/benchmarks/bundle-size/history.mjs` around lines 22 - 25, The code in
parseHistory uses vm.runInNewContext to evaluate user-provided payloads (the
block where trimmed.startsWith('window.BENCHMARK_DATA') creates sandbox and
calls vm.runInNewContext), which allows arbitrary code execution; instead, strip
the optional "window.BENCHMARK_DATA =" prefix and any trailing semicolon from
the trimmed string and parse the remaining content with JSON.parse to return the
benchmark object (i.e., remove the sandbox/vm.runInNewContext logic and replace
it with safe string extraction + JSON.parse).
| const requested = filter | ||
| .split(',') | ||
| .map((value) => value.trim()) | ||
| .filter(Boolean) |
There was a problem hiding this comment.
Reject empty --scenario selector lists.
If the user passes only commas or whitespace, this collapses to an empty array and the script reports success with zero measured scenarios. Failing fast here avoids publishing an empty snapshot by accident.
Suggested fix
const requested = filter
.split(',')
.map((value) => value.trim())
.filter(Boolean)
+ if (requested.length === 0) {
+ throw new Error('--scenario must include at least one non-empty selector')
+ }🤖 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 `@scripts/benchmarks/bundle-size/measure.mjs` around lines 158 - 161, The code
builds the requested array from the filter string but doesn't reject cases where
the input was only commas/whitespace, producing an empty requested list; update
the logic after the requested calculation in measure.mjs to detect
requested.length === 0 (or equivalently !requested.length) and fail fast by
emitting a clear error mentioning the --scenario selector was empty and exiting
non‑zero (e.g., throw an Error or call process.exit(1)); reference the requested
variable and the code path that consumes it so the script never proceeds to
publish an empty snapshot when no scenarios were selected.
| status: { | ||
| state: 'success', | ||
| command: `node ${path.relative(repoRoot, fileURLToPath(import.meta.url))}${args.scenario ? ` --scenario ${args.scenario}` : ''}${args.analysis ? ' --analysis' : ''}${args.sourcemap ? ' --sourcemap' : ''}${args.skipPackageBuilds ? ' --skip-package-builds' : ''}`, |
There was a problem hiding this comment.
Quote the persisted rerun command.
status.command stores a shell command, but args.scenario is interpolated raw. A common value like react-router.minimal, solid-router.full gets recorded as two argv tokens and cannot be pasted back verbatim.
Suggested fix
- command: `node ${path.relative(repoRoot, fileURLToPath(import.meta.url))}${args.scenario ? ` --scenario ${args.scenario}` : ''}${args.analysis ? ' --analysis' : ''}${args.sourcemap ? ' --sourcemap' : ''}${args.skipPackageBuilds ? ' --skip-package-builds' : ''}`,
+ command: `node ${path.relative(repoRoot, fileURLToPath(import.meta.url))}${args.scenario ? ` --scenario ${JSON.stringify(args.scenario)}` : ''}${args.analysis ? ' --analysis' : ''}${args.sourcemap ? ' --sourcemap' : ''}${args.skipPackageBuilds ? ' --skip-package-builds' : ''}`,📝 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.
| status: { | |
| state: 'success', | |
| command: `node ${path.relative(repoRoot, fileURLToPath(import.meta.url))}${args.scenario ? ` --scenario ${args.scenario}` : ''}${args.analysis ? ' --analysis' : ''}${args.sourcemap ? ' --sourcemap' : ''}${args.skipPackageBuilds ? ' --skip-package-builds' : ''}`, | |
| status: { | |
| state: 'success', | |
| command: `node ${path.relative(repoRoot, fileURLToPath(import.meta.url))}${args.scenario ? ` --scenario ${JSON.stringify(args.scenario)}` : ''}${args.analysis ? ' --analysis' : ''}${args.sourcemap ? ' --sourcemap' : ''}${args.skipPackageBuilds ? ' --skip-package-builds' : ''}`, |
🤖 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 `@scripts/benchmarks/bundle-size/measure.mjs` around lines 954 - 956,
status.command stores a shell command but inserts args.scenario raw, causing
multi-token values like "react-router.minimal, solid-router.full" to split;
update the interpolation in measure.mjs where status.command is built so
args.scenario is shell-quoted or escaped (e.g., wrap in a quoted string via
JSON.stringify or a proper shell-escape helper) before concatenation, ensuring
the persisted rerun command can be pasted verbatim; apply the same quoting
approach to any other arg interpolations in the same expression if they can
contain spaces or punctuation.
| if (trimmed.startsWith('window.BENCHMARK_DATA')) { | ||
| return JSON.parse( | ||
| trimmed | ||
| .replace(/^window\.BENCHMARK_DATA\s*=\s*/, '') | ||
| .replace(/;\s*$/, ''), | ||
| ) | ||
| const sandbox = { window: {} } | ||
| vm.runInNewContext(trimmed, sandbox, { timeout: 1000 }) | ||
| return sandbox.window.BENCHMARK_DATA |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is node:vm (vm.runInNewContext) considered a security boundary for untrusted JavaScript input in Node.js?
💡 Result:
No, the node:vm module is explicitly not considered a security boundary, and it should never be used to run untrusted JavaScript code [1][2][3]. Official Node.js documentation consistently includes a prominent warning that the module is not a security mechanism and cautions against using it for untrusted input [1][4][3]. While the module uses V8 contexts to provide an isolated global environment for code execution, this is intended for scoping and convenience rather than security [1][5][3]. The primary reasons include: * Lack of true isolation: The module is designed to separate scopes, not to provide a hardened security boundary [5][6]. A malicious script can easily access the host environment by traversing the prototype chain—for example, by accessing constructors—to reach the host's process or global objects [5]. * Misconceptions about sandboxing: The term "sandbox" is sometimes used in informal contexts to describe a separate execution environment, but it does not imply that the code is protected or restricted from interacting with the host [4][5]. * Historical and practical failures: Numerous efforts to build secure "sandboxing" libraries on top of node:vm or similar mechanisms have historically failed, often resulting in critical vulnerabilities and sandbox escape exploits that allow for remote code execution on the host [5][7][8][9]. Because node:vm operates within the same runtime process as the host application, any compromise of the execution context can lead to full host takeover [8][10][6]. For scenarios requiring the execution of untrusted code, experts and documentation recommend using true isolation boundaries, such as separate operating system processes, containers, or dedicated virtual machines/sandboxes that operate outside the host's primary runtime environment [5][8][10].
Citations:
- 1: https://nodejs.org/dist/latest-v19.x/docs/api/vm.html
- 2: https://nodejs.org/download/release/v18.20.4/docs/api/vm.html
- 3: https://nodejs.org/api/vm.html
- 4: https://nodejs.org/dist/v11.0.0/docs/api/vm.html
- 5: https://dev.to/dendrite_soup/nodevm-is-not-a-sandbox-stop-using-it-like-one-2f74
- 6: https://snyk.io/blog/security-concerns-javascript-sandbox-node-js-vm-module/
- 7: https://www.csoonline.com/article/4168568/13-new-critical-holes-in-javascript-sandbox-allow-execution-of-arbitrary-code.html
- 8: https://undercodenews.com/critical-vm2-sandbox-disaster-exposes-millions-of-nodejs-systems-to-remote-code-execution/
- 9: https://undercodenews.com/critical-vm2-sandbox-escape-vulnerabilities-expose-nodejs-systems-to-full-remote-code-execution/
- 10: https://b2bdaily.com/it/can-a-single-sandboxjs-flaw-lead-to-full-host-takeover/
🏁 Script executed:
# Find and read the pr-report.mjs file
fd -t f "pr-report.mjs" --exec cat -n {} \;Repository: TanStack/router
Length of output: 11479
🏁 Script executed:
# Look for context around lines 71-74 and check data source
fd -t f "pr-report.mjs" --exec sed -n '1,80p' {} \;Repository: TanStack/router
Length of output: 2125
🏁 Script executed:
# Search for other vm module usage in the codebase
rg "vm\." --type javascript --type typescript -B 2 -A 2Repository: TanStack/router
Length of output: 94
Replace vm.runInNewContext() with regex extraction and JSON.parse().
The node:vm module provides no security boundary against untrusted code execution. Data files read from external sources should be parsed safely without code execution.
Safer parsing approach
function parseMaybeDataJs(raw) {
const trimmed = raw.trim()
if (trimmed.startsWith('window.BENCHMARK_DATA')) {
- const sandbox = { window: {} }
- vm.runInNewContext(trimmed, sandbox, { timeout: 1000 })
- return sandbox.window.BENCHMARK_DATA
+ const match = trimmed.match(
+ /^window\.BENCHMARK_DATA\s*=\s*(\{[\s\S]*\})\s*;?\s*$/,
+ )
+ if (!match) {
+ throw new Error('Invalid data.js format for window.BENCHMARK_DATA')
+ }
+ return JSON.parse(match[1])
}
return JSON.parse(trimmed)
}📝 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.
| if (trimmed.startsWith('window.BENCHMARK_DATA')) { | |
| return JSON.parse( | |
| trimmed | |
| .replace(/^window\.BENCHMARK_DATA\s*=\s*/, '') | |
| .replace(/;\s*$/, ''), | |
| ) | |
| const sandbox = { window: {} } | |
| vm.runInNewContext(trimmed, sandbox, { timeout: 1000 }) | |
| return sandbox.window.BENCHMARK_DATA | |
| if (trimmed.startsWith('window.BENCHMARK_DATA')) { | |
| const match = trimmed.match( | |
| /^window\.BENCHMARK_DATA\s*=\s*(\{[\s\S]*\})\s*;?\s*$/, | |
| ) | |
| if (!match) { | |
| throw new Error('Invalid data.js format for window.BENCHMARK_DATA') | |
| } | |
| return JSON.parse(match[1]) |
🤖 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 `@scripts/benchmarks/bundle-size/pr-report.mjs` around lines 71 - 74, The code
currently executes untrusted text via vm.runInNewContext(trimmed, sandbox...) to
obtain window.BENCHMARK_DATA; replace this with a safe extraction: use a regular
expression against trimmed to capture the JSON assigned to
window.BENCHMARK_DATA, validate the match, and then parse the captured JSON with
JSON.parse() to return the data instead of executing code. Locate the block
using trimmed and vm.runInNewContext and update it to extract the RHS of the
window.BENCHMARK_DATA assignment via regex, check for a match, and call
JSON.parse on the captured string (handle parse errors) before returning the
resulting object.
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 not alter performance
Comparing 
Footnotes
|
schiller-manuel
force-pushed
the
bundle-size-skill
branch
from
3dda1ba to
1f28eef
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/benchmarks/bundle-size/query.mjs (1)
25-27: ⚡ Quick winConsider listing available metric IDs in the error message.
When an unknown ID is provided, showing the available options would improve discoverability.
💡 Proposed enhancement
if (values.id && metrics.length === 0) { - throw new Error(`Unknown bundle-size metric: ${values.id}`) + const availableIds = (current.metrics || []).map((m) => m.id).join(', ') + throw new Error( + `Unknown bundle-size metric: ${values.id}\nAvailable: ${availableIds}` + ) }🤖 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 `@scripts/benchmarks/bundle-size/query.mjs` around lines 25 - 27, The error thrown for an unknown metric currently uses only values.id; update the throw in the check that uses values.id and metrics (the block that currently does throw new Error(`Unknown bundle-size metric: ${values.id}`)) to include the available metric IDs by deriving them from the metrics collection (e.g., map metrics to their id strings and join them), and include that list in the Error message so it reads like "Unknown bundle-size metric: <id>. Available metrics: <id1, id2, ...>".
🤖 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 `@scripts/benchmarks/bundle-size/query.mjs`:
- Around line 19-20: Wrap the synchronous file read and JSON.parse around a
try-catch to provide clear error handling: when resolving currentPath, reading
via fs.readFileSync and parsing into current, catch errors (file not found,
permission, or invalid JSON), log a descriptive message including currentPath
and the caught error (e.g., via console.error) and exit with a non-zero code;
optionally pre-check with fs.existsSync(currentPath) to give a specific "file
not found" message before parsing.
In `@scripts/ts-symbol-references.mjs`:
- Around line 152-160: The current CLI parses line/column into
lineNumber/columnNumber and only checks Number.isInteger, allowing 0 or
negatives which become invalid after decrement; update the validation around the
conversion of line and column to ensure both are integers >= 1 and call fail
with a clear message if not (e.g., "--line and --column must be positive 1-based
integers"), before calling sourceFile.getPositionOfLineAndCharacter (so adjust
the checks around lineNumber, columnNumber, Number.isInteger and the fail
invocation).
In `@skills/bundle-size-optimization/SKILL.md`:
- Line 31: Replace the machine-specific hardcoded temp path in SKILL.md ("
/var/folders/6f/2t42ntqs4yv4h6qwzbh5pmcm0000gn/T/opencode") with a generic
placeholder (e.g., "<temp-dir>/opencode" or "/tmp/opencode") so instructions are
portable; update the sentence that references the path to read something like
"run the same scenario in a separate worktree under <temp-dir>/opencode and diff
the two `current.json` files" and keep the backticked `current.json` reference
unchanged.
---
Nitpick comments:
In `@scripts/benchmarks/bundle-size/query.mjs`:
- Around line 25-27: The error thrown for an unknown metric currently uses only
values.id; update the throw in the check that uses values.id and metrics (the
block that currently does throw new Error(`Unknown bundle-size metric:
${values.id}`)) to include the available metric IDs by deriving them from the
metrics collection (e.g., map metrics to their id strings and join them), and
include that list in the Error message so it reads like "Unknown bundle-size
metric: <id>. Available metrics: <id1, id2, ...>".
🪄 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: 52f96b00-26a7-4e1e-9499-d00234ba6c2b
📒 Files selected for processing (12)
AGENTS.mdbenchmarks/bundle-size/README.mdbenchmarks/bundle-size/package.jsonpackage.jsonscripts/benchmarks/bundle-size/analyze.mjsscripts/benchmarks/bundle-size/diff.mjsscripts/benchmarks/bundle-size/history.mjsscripts/benchmarks/bundle-size/measure.mjsscripts/benchmarks/bundle-size/pr-report.mjsscripts/benchmarks/bundle-size/query.mjsscripts/ts-symbol-references.mjsskills/bundle-size-optimization/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- benchmarks/bundle-size/README.md
| const currentPath = path.resolve(values.current) | ||
| const current = JSON.parse(fs.readFileSync(currentPath, 'utf8')) |
There was a problem hiding this comment.
Add error handling for file read and JSON parse operations.
The script will crash with a generic Node.js error if the file doesn't exist or contains invalid JSON. Adding a try-catch block would provide clearer feedback.
🛡️ Proposed fix to add error handling
-const currentPath = path.resolve(values.current)
-const current = JSON.parse(fs.readFileSync(currentPath, 'utf8'))
+const currentPath = path.resolve(values.current)
+let current
+try {
+ current = JSON.parse(fs.readFileSync(currentPath, 'utf8'))
+} catch (error) {
+ console.error(`Failed to read results from ${currentPath}: ${error.message}`)
+ process.exit(1)
+}📝 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.
| const currentPath = path.resolve(values.current) | |
| const current = JSON.parse(fs.readFileSync(currentPath, 'utf8')) | |
| const currentPath = path.resolve(values.current) | |
| let current | |
| try { | |
| current = JSON.parse(fs.readFileSync(currentPath, 'utf8')) | |
| } catch (error) { | |
| console.error(`Failed to read results from ${currentPath}: ${error.message}`) | |
| process.exit(1) | |
| } |
🤖 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 `@scripts/benchmarks/bundle-size/query.mjs` around lines 19 - 20, Wrap the
synchronous file read and JSON.parse around a try-catch to provide clear error
handling: when resolving currentPath, reading via fs.readFileSync and parsing
into current, catch errors (file not found, permission, or invalid JSON), log a
descriptive message including currentPath and the caught error (e.g., via
console.error) and exit with a non-zero code; optionally pre-check with
fs.existsSync(currentPath) to give a specific "file not found" message before
parsing.
| const lineNumber = Number(line) | ||
| const columnNumber = Number(column) | ||
| if (!Number.isInteger(lineNumber) || !Number.isInteger(columnNumber)) { | ||
| fail('--line and --column must be integers.') | ||
| } | ||
|
|
||
| const position = sourceFile.getPositionOfLineAndCharacter( | ||
| lineNumber - 1, | ||
| columnNumber - 1, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TypeScript 6.x, what are the expected input constraints for SourceFile.getPositionOfLineAndCharacter(line, character), and how does it behave for negative line/character values?
💡 Result:
In TypeScript 6.x, SourceFile.getPositionOfLineAndCharacter(line, character) expects zero-based line and character inputs, with constraints that the line and character must be within the bounds of the source file; if an input is outside those bounds, the behavior is not defined in the public docs and internal callers typically guard against invalid line/character values. For negative line/character values: TypeScript does not provide a documented/guaranteed contract for negative line or character. The method is implemented in a way that assumes non-negative inputs (the underlying line-start lookup/assertions treat negative positions as invalid). As a result, passing negative values can lead to incorrect results or runtime failures depending on the exact TypeScript version/build and whether internal assertions are enabled. Evidence: internal TypeScript code paths that compute positions enforce non-negative constraints (e.g., asserts like “generatedCharacter cannot be negative” in the sourcemap pipeline) and the general compiler API guidance assumes valid, non-negative line/character usage when converting between lines/characters and positions. [1] [2]
Citations:
- 1: https://github.com/microsoft/TypeScript/blob/main/src/compiler/sourcemap.ts
- 2: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API
🏁 Script executed:
# Check if the file exists and view the specific lines
if [ -f "scripts/ts-symbol-references.mjs" ]; then
echo "=== File exists ==="
wc -l scripts/ts-symbol-references.mjs
echo ""
echo "=== Lines 145-170 (context around lines 152-160) ==="
sed -n '145,170p' scripts/ts-symbol-references.mjs
else
echo "File not found at scripts/ts-symbol-references.mjs"
# Try to find it
find . -name "ts-symbol-references.mjs" -type f 2>/dev/null
fiRepository: TanStack/router
Length of output: 980
🏁 Script executed:
# Search for similar input validation patterns in the codebase to match style
echo "=== Search for Number.isInteger validation patterns ==="
rg "Number\.isInteger" -A 3 -B 1 scripts/Repository: TanStack/router
Length of output: 848
Validate --line/--column as 1-based positive integers.
0 or negative values currently pass integer checks and then get decremented, which can lead to invalid position lookup behavior instead of a clear CLI error.
Suggested fix
const lineNumber = Number(line)
const columnNumber = Number(column)
- if (!Number.isInteger(lineNumber) || !Number.isInteger(columnNumber)) {
- fail('--line and --column must be integers.')
+ if (
+ !Number.isInteger(lineNumber) ||
+ !Number.isInteger(columnNumber) ||
+ lineNumber < 1 ||
+ columnNumber < 1
+ ) {
+ fail('--line and --column must be positive integers (1-based).')
}📝 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.
| const lineNumber = Number(line) | |
| const columnNumber = Number(column) | |
| if (!Number.isInteger(lineNumber) || !Number.isInteger(columnNumber)) { | |
| fail('--line and --column must be integers.') | |
| } | |
| const position = sourceFile.getPositionOfLineAndCharacter( | |
| lineNumber - 1, | |
| columnNumber - 1, | |
| const lineNumber = Number(line) | |
| const columnNumber = Number(column) | |
| if ( | |
| !Number.isInteger(lineNumber) || | |
| !Number.isInteger(columnNumber) || | |
| lineNumber < 1 || | |
| columnNumber < 1 | |
| ) { | |
| fail('--line and --column must be positive integers (1-based).') | |
| } | |
| const position = sourceFile.getPositionOfLineAndCharacter( | |
| lineNumber - 1, | |
| columnNumber - 1, |
🤖 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 `@scripts/ts-symbol-references.mjs` around lines 152 - 160, The current CLI
parses line/column into lineNumber/columnNumber and only checks
Number.isInteger, allowing 0 or negatives which become invalid after decrement;
update the validation around the conversion of line and column to ensure both
are integers >= 1 and call fail with a clear message if not (e.g., "--line and
--column must be positive 1-based integers"), before calling
sourceFile.getPositionOfLineAndCharacter (so adjust the checks around
lineNumber, columnNumber, Number.isInteger and the fail invocation).
| - Track `gzipBytes` first; also inspect `initialGzipBytes`, `rawBytes`, `brotliBytes`, `jsFiles`, and per-file `files`. | ||
| - Dist paths use `scenarioDir`/`outDir`, not metric ids: `react-router.minimal` maps to `dist/react-router-minimal/`. | ||
| - For tiny changes, measure after each candidate; gzip can move opposite raw bytes. | ||
| - To compare a base commit, run the same scenario in a separate worktree under `/var/folders/6f/2t42ntqs4yv4h6qwzbh5pmcm0000gn/T/opencode` and diff the two `current.json` files. |
There was a problem hiding this comment.
Replace machine-specific path with a generic placeholder.
The hardcoded path /var/folders/6f/2t42ntqs4yv4h6qwzbh5pmcm0000gn/T/opencode appears to be a macOS-specific temporary directory from a particular machine. This could confuse users trying to follow the instructions literally.
📝 Proposed fix
-- To compare a base commit, run the same scenario in a separate worktree under `/var/folders/6f/2t42ntqs4yv4h6qwzbh5pmcm0000gn/T/opencode` and diff the two `current.json` files.
+- To compare a base commit, run the same scenario in a separate worktree (e.g., `/tmp/base-worktree`) and diff the two `current.json` files.📝 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.
| - To compare a base commit, run the same scenario in a separate worktree under `/var/folders/6f/2t42ntqs4yv4h6qwzbh5pmcm0000gn/T/opencode` and diff the two `current.json` files. | |
| - To compare a base commit, run the same scenario in a separate worktree (e.g., `/tmp/base-worktree`) and diff the two `current.json` files. |
🤖 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 `@skills/bundle-size-optimization/SKILL.md` at line 31, Replace the
machine-specific hardcoded temp path in SKILL.md ("
/var/folders/6f/2t42ntqs4yv4h6qwzbh5pmcm0000gn/T/opencode") with a generic
placeholder (e.g., "<temp-dir>/opencode" or "/tmp/opencode") so instructions are
portable; update the sentence that references the path to read something like
"run the same scenario in a separate worktree under <temp-dir>/opencode and diff
the two `current.json` files" and keep the backticked `current.json` reference
unchanged.
schiller-manuel
force-pushed
the
bundle-size-skill
branch
from
3f3405a to
a48f9ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/bundle-size/package.json`:
- Line 22: The dependency entry "`@tanstack/vue-start`": "workspace:^" in
package.json uses the wrong workspace protocol; update the version specifier to
use the repo-required workspace protocol by changing the dependency value for
"`@tanstack/vue-start`" from "workspace:^" to "workspace:*" so internal package
resolution follows the project's guidelines.
In `@benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts`:
- Around line 7-15: The env value BUNDLE_SIZE_RSB_CLIENT_OUTPUT is being
force-cast to 'module'|'iife' when building startOptions; instead narrow and
validate it first to preserve TypeScript safety: check that
process.env.BUNDLE_SIZE_RSB_CLIENT_OUTPUT is one of the allowed strings
('module' or 'iife') (or use a small type-guard/lookup) and only then assign it
to clientOutput so startOptions.rsbuild.client.output receives a properly typed
value; refer to startOptions, rsbuild.client.output and the runtime z.enum in
tanStackStartRsbuild when adding the validation.
In `@benchmarks/bundle-size/scenarios/vue-start-full/src/routes/__root.tsx`:
- Around line 78-89: The hook calls currently use "as any" which bypasses strict
types; remove those "as any" assertions and pass properly typed option objects
or explicit generics that match each hook's signature (e.g., for useLinkProps,
useMatch, useLoaderDeps, useLoaderData, useParams, useSearch, useRouteContext
and useRouterState) or use a narrower cast like Partial<ExpectedOptionsType>
when needed; locate the calls to useLinkProps, useMatchRoute, useMatches,
useParentMatches, useChildMatches, useMatch, useLoaderDeps, useLoaderData,
useParams, useSearch, useRouteContext and useRouterState in the file and replace
the blanket any casts with correct option types or explicit generic parameters
so the compiler enforces strict typing.
🪄 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: 71efcb55-8ae4-46b8-ad43-02a009436bba
📒 Files selected for processing (13)
.agents/skills/bundle-size-optimizationAGENTS.mdbenchmarks/bundle-size/README.mdbenchmarks/bundle-size/package.jsonbenchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.tsbenchmarks/bundle-size/scenarios/vue-start-full/src/router.tsxbenchmarks/bundle-size/scenarios/vue-start-full/src/routes/__root.tsxbenchmarks/bundle-size/scenarios/vue-start-full/src/routes/index.tsxbenchmarks/bundle-size/scenarios/vue-start-full/vite.config.tsbenchmarks/bundle-size/scenarios/vue-start-minimal/src/router.tsxbenchmarks/bundle-size/scenarios/vue-start-minimal/src/routes/__root.tsxbenchmarks/bundle-size/scenarios/vue-start-minimal/src/routes/index.tsxbenchmarks/bundle-size/scenarios/vue-start-minimal/vite.config.ts
✅ Files skipped from review due to trivial changes (8)
- benchmarks/bundle-size/scenarios/vue-start-minimal/src/router.tsx
- benchmarks/bundle-size/scenarios/vue-start-minimal/src/routes/index.tsx
- benchmarks/bundle-size/scenarios/vue-start-full/src/routes/index.tsx
- benchmarks/bundle-size/scenarios/vue-start-minimal/vite.config.ts
- .agents/skills/bundle-size-optimization
- benchmarks/bundle-size/scenarios/vue-start-full/vite.config.ts
- benchmarks/bundle-size/README.md
- AGENTS.md
| "@tanstack/vue-router": "workspace:^", | ||
| "@tanstack/react-start": "workspace:^", | ||
| "@tanstack/solid-start": "workspace:^", | ||
| "@tanstack/vue-start": "workspace:^", |
There was a problem hiding this comment.
Use workspace:* for the newly added internal dependency.
Line 22 uses workspace:^, but repo rules require workspace:* for internal packages.
Proposed fix
- "`@tanstack/vue-start`": "workspace:^",
+ "`@tanstack/vue-start`": "workspace:*",📝 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.
| "@tanstack/vue-start": "workspace:^", | |
| "`@tanstack/vue-start`": "workspace:*", |
🤖 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/bundle-size/package.json` at line 22, The dependency entry
"`@tanstack/vue-start`": "workspace:^" in package.json uses the wrong workspace
protocol; update the version specifier to use the repo-required workspace
protocol by changing the dependency value for "`@tanstack/vue-start`" from
"workspace:^" to "workspace:*" so internal package resolution follows the
project's guidelines.
| const startOptions = clientOutput | ||
| ? { | ||
| rsbuild: { | ||
| client: { | ||
| output: clientOutput as 'module' | 'iife', | ||
| }, | ||
| }, | ||
| } | ||
| : undefined |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the current versions of @tanstack/react-start/plugin/rsbuildand@rsbuild/core, what exact values are supported for rsbuild.client.output?
💡 Result:
As of May 2026, there is no configuration property named rsbuild.client.output in either @rsbuild/core or the @tanstack/react-start/plugin/rsbuild plugin [1][2][3]. The confusion likely stems from the Rsbuild Environments API, which allows for separate configurations for different build targets (such as client and server) [4]. In Rsbuild, configuration for the client-side build is typically defined within the environments.client object, and output-related settings are managed under the standard output configuration object [4][3][5]. If you are looking to configure output settings specifically for a client environment, you should use the following structure in your rsbuild.config.ts: export default defineConfig({ environments: { client: { output: { // Your output configuration here (e.g., distPath, filename) }, }, }, }); For authoritative documentation on available output options, refer to the official Rsbuild configuration guide, which details properties such as distPath, filename, and assetPrefix [3][5][6]. The TanStack Start Rsbuild plugin manages its own internal configuration and does not expose a custom rsbuild.client.output property [1][2].
Citations:
- 1: feat: add rsbuild support for TanStack Start #6623
- 2: 174ff3f
- 3: https://rsbuild.rs/guide/configuration/rsbuild
- 4: https://rsbuild.rs/blog/v2-0
- 5: https://rsbuild.rs/guide/basic/output-files.md
- 6: https://rslib.rs/config/rsbuild/output.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant config file
FILE="benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts"
if [ -f "$FILE" ]; then
echo "=== $FILE ==="
nl -ba "$FILE" | sed -n '1,120p'
else
echo "File not found: $FILE"
ls -R benchmarks/bundle-size/scenarios/react-start-minimal | head
fi
echo
echo "=== Search for BUNDLE_SIZE_RSB_CLIENT_OUTPUT usage ==="
rg -n "BUNDLE_SIZE_RSB_CLIENT_OUTPUT" -S .
echo
echo "=== Search for 'tanstackStart(' usage in this config ==="
rg -n "tanstackStart\(" -S benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts
echo
echo "=== Search for 'output:' with module/iife strings in repo ==="
rg -n "output:\s*['\"](module|iife)['\"]" -S . || true
echo
echo "=== Search for 'client: {' and rsbuild: structure patterns ==="
rg -n "rsbuild:\s*{\s*client:\s*{|\bclient:\s*{\s*output" -S benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts || trueRepository: TanStack/router
Length of output: 199
🌐 Web query:
tanstack react-start rsbuild plugin client output module iife
💡 Result:
Answer: In Rsbuid (Rspack-based), “client output module iife” doesn’t correspond to a single TanStack Start setting. What you actually control is Rsbuild output format: 1) Default: IIFE for web targets Rsbuild states that when building a web application, it generates output in the IIFE format by default [1]. 2) Switching away from IIFE to ES modules If you want ES module output instead of IIFE, Rsbuild provides output.module (boolean). Rsbuild says: “When building a web application, Rsbuild generates the output in the IIFE format by default. If you want to output ES Modules instead, set output.module to true.” [1]. Example: export default { output: { module: true, }, }; When output.module is enabled, Rsbuild also adds type="module" to generated script tags [1]. 3) Node target default differs For completeness: Rsbuild says Node.js apps output ES modules format by default, and you can change to CommonJS via output.module = false when output.target is node [1]. 4) How this relates to TanStack Start “rsbuild plugin” TanStack Start’s rsbuild integration is implemented as Rsbuild plugins (e.g., Start rsbuild plugin core plus framework-specific rsbuild plugin entries) [2][3]. However, the IIFE-vs-ESM detail is governed by Rsbuild’s output configuration (not an “iife” option specific to TanStack Start, based on the Rsbuild output.module documentation) [1]. So, to get “client module” output (ESM) rather than IIFE in a TanStack Start + Rsbuild web build, set Rsbuild output.module=true; otherwise you’ll get IIFE by default for web [1].
Citations:
- 1: https://rsbuild.rs/config/output/module
- 2: rsbuild plugin #7228
- 3: feat: add rsbuild support for TanStack Start #6623
🌐 Web query:
@tanstack/react-start plugin rsbuild output module iife
💡 Result:
In Rsbuild, the default web-app JavaScript format is IIFE. If you want an ES-module output instead, set output.module = true; this results in ES modules and Rsbuild sets scriptLoading to module in the generated HTML. [1] So for the TanStack React Start rsbuild plugin: it doesn’t (by itself) switch you away from IIFE. To get module output (instead of IIFE), you configure Rsbuild’s output.module. [1] Example (rsbuild.config.ts): import { defineConfig } from '@rsbuild/core' import { pluginReact } from '@rsbuild/plugin-react' export default defineConfig({ plugins: [pluginReact], output: { module: true, }, }) (With output.module enabled, Rsbuild also adds type="module" to generated script tags.) [1] Separately, note that Rspack has explicit output.iife and output.module controls; output.iife defaults to true, and output.module toggles module output and implies output.iife=false. [2]
Citations:
🌐 Web query:
rsbuild client output module iife configuration
💡 Result:
In Rsbuild, “IIFE” output is controlled by building for the web target, not by an explicit “IIFE” flag. 1) When does Rsbuild emit IIFE? - For web applications (output.target = 'web', which is the default), Rsbuild generates output in the IIFE format by default. [1] 2) How to configure client output “module” (ESM) vs IIFE - Rsbuild has output.module (boolean). It is described as outputting JavaScript in ES modules format, and it is only available when output.target is 'web' or 'node'. [1] - If you want ES Modules instead of the default IIFE for web apps, set output.module = true. [1] Example (web/client => IIFE default): export default { output: { target: 'web', // output.module omitted/false => IIFE by default }, }; Example (web/client => ESM, not IIFE): export default { output: { target: 'web', module: true, }, }; 3) Node client (if you’re using environments for client/server) - For output.target = 'node', Rsbuild outputs ES modules by default, and you can set output.module = false to output CommonJS instead. [1] If you share your exact “rsbuild client” setup (e.g., whether you use environments and what you set for output.target), I can map it precisely to the correct config block.
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant config file
FILE="benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts"
if [ -f "$FILE" ]; then
echo "=== $FILE ==="
nl -ba "$FILE" | sed -n '1,120p'
else
echo "File not found: $FILE"
ls -R benchmarks/bundle-size/scenarios/react-start-minimal | head
fi
echo
echo "=== Search for BUNDLE_SIZE_RSB_CLIENT_OUTPUT usage ==="
rg -n "BUNDLE_SIZE_RSB_CLIENT_OUTPUT" -S .
echo
echo "=== Search for 'tanstackStart(' usage in this config ==="
rg -n "tanstackStart\(" -S benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts
echo
echo "=== Search for 'output:' with module/iife strings in repo ==="
rg -n "output:\s*['\"](module|iife)['\"]" -S . || true
echo
echo "=== Search for 'client: {' and rsbuild: structure patterns ==="
rg -n "rsbuild:\s*{\s*client:\s*{|\bclient:\s*{\s*output" -S benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts || trueRepository: TanStack/router
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts"
echo "=== FILE CONTENTS: $FILE ==="
if [ -f "$FILE" ]; then
sed -n '1,160p' "$FILE" | cat
else
echo "MISSING: $FILE"
exit 1
fi
echo
echo "=== SHOW PACKAGE JSON(S) FOR DEPENDENCY VERSIONS ==="
# Try common locations; if missing, still show what exists.
for p in \
"benchmarks/bundle-size/package.json" \
"benchmarks/package.json" \
"package.json" \
; do
if [ -f "$p" ]; then
echo "--- $p ---"
cat "$p"
fi
done
echo
echo "=== RIPGREP: look for tanstackStart and rsbuild.client.output usage ==="
rg -n "tanstackStart\\(" benchmarks/bundle-size/scenarios/react-start-minimal -S || true
rg -n "rsbuild:\\s*\\{[\\s\\S]*client[\\s\\S]*output" benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts -S || true
rg -n "client:\\s*\\{[\\s\\S]*output" benchmarks/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts -S || trueRepository: TanStack/router
Length of output: 7409
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find implementation/types for tanstackStart in the monorepo
rg -n "export (function|const|default) tanstackStart|tanstackStart\\s*\\(" packages -S || true
rg -n "tanstackStart" packages/@tanstack -S || true
# Look for rsbuild plugin option typing, especially rsbuild.client.output
rg -n "rsbuild\\s*:\\s*\\{" packages -S || true
rg -n "client\\s*:\\s*\\{[\\s\\S]*output" packages -S || true
rg -n "output\\s*:\\s*'module'\\s*\\|\\s*'iife'|\"module\"\\s*\\|\\s*\"iife\"" -S packages || true
rg -n "module'\\s*\\|\\s*'iife|iife'\\s*\\|" -S packages || true
# Locate files under `@tanstack/react-start/plugin/rsbuild`
fd -t f "rsbuild" packages | head -n 50
fd -t f "*react-start*rsbuild*" packages | head -n 50Repository: TanStack/router
Length of output: 5395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find implementation/types for tanstackStart in the monorepo
rg -n "export (function|const|default) tanstackStart|tanstackStart\\s*\\(" packages -S || true
rg -n "tanstackStart" packages/@tanstack -S || true
# Look for rsbuild plugin option typing, especially rsbuild.client.output
rg -n "rsbuild\\s*:\\s*\\{" packages -S || true
rg -n "client\\s*:\\s*\\{[\\s\\S]*output" packages -S || true
rg -n "output\\s*:\\s*'module'\\s*\\|\\s*'iife'|\"module\"\\s*\\|\\s*\"iife\"" -S packages || true
rg -n "module'\\s*\\|\\s*'iife|iife'\\s*\\|" -S packages || true
# Locate files under `@tanstack/react-start/plugin/rsbuild`
fd -t f "rsbuild" packages | head -n 50
fd -t f "*react-start*rsbuild*" packages | head -n 50Repository: TanStack/router
Length of output: 5395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print relevant parts of TanStack Start rsbuild plugin implementation and schema
sed -n '1,220p' packages/react-start/src/plugin/rsbuild.ts | cat
echo "-----"
sed -n '1,220p' packages/start-plugin-core/src/rsbuild/schema.ts | cat
echo "-----"
sed -n '1,120p' packages/router-core/src/manifest.ts | catRepository: TanStack/router
Length of output: 7144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "parseStartConfig\\(" packages/start-plugin-core -S
rg -n "tanstackStartRsbuildOptionsSchema\\.parse\\(" packages/start-plugin-core -S
rg -n "tanStackStartRsbuild\\(" packages/start-plugin-core -S
rg -n "tanStackStartRsbuild" packages/start-plugin-core -SRepository: TanStack/router
Length of output: 684
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' packages/start-plugin-core/src/rsbuild/plugin.ts | cat
echo "-----"
sed -n '1,120p' packages/start-plugin-core/src/rsbuild/index.ts | catRepository: TanStack/router
Length of output: 9554
Validate BUNDLE_SIZE_RSB_CLIENT_OUTPUT before assigning (avoid unsafe cast)
Line 11 force-casts the env value to 'module' | 'iife'. tanStackStartRsbuild already validates rsbuild.client.output at runtime with z.enum(['module', 'iife']), so invalid values won’t silently proceed—but the as assertion still bypasses the intended TypeScript type safety. Narrow the env value first.
Proposed fix
const outDir = process.env.BUNDLE_SIZE_DIST_DIR ?? 'dist-rsbuild'
const clientOutput = process.env.BUNDLE_SIZE_RSB_CLIENT_OUTPUT
+const isValidClientOutput =
+ clientOutput === 'module' || clientOutput === 'iife'
+
+if (clientOutput && !isValidClientOutput) {
+ throw new Error(
+ `Invalid BUNDLE_SIZE_RSB_CLIENT_OUTPUT: ${clientOutput}. Expected "module" or "iife".`,
+ )
+}
+
const startOptions = clientOutput
? {
rsbuild: {
client: {
- output: clientOutput as 'module' | 'iife',
+ output: clientOutput,
},
},
}
: undefined🤖 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/bundle-size/scenarios/react-start-minimal/rsbuild.config.ts`
around lines 7 - 15, The env value BUNDLE_SIZE_RSB_CLIENT_OUTPUT is being
force-cast to 'module'|'iife' when building startOptions; instead narrow and
validate it first to preserve TypeScript safety: check that
process.env.BUNDLE_SIZE_RSB_CLIENT_OUTPUT is one of the allowed strings
('module' or 'iife') (or use a small type-guard/lookup) and only then assign it
to clientOutput so startOptions.rsbuild.client.output receives a properly typed
value; refer to startOptions, rsbuild.client.output and the runtime z.enum in
tanStackStartRsbuild when adding the validation.
| const linkProps = useLinkProps({ to: '/' } as any) | ||
| const matchRoute = useMatchRoute() | ||
| const matches = useMatches() | ||
| const parentMatches = useParentMatches() | ||
| const childMatches = useChildMatches() | ||
| const match = useMatch({ strict: false, shouldThrow: false } as any) | ||
| const loaderDeps = useLoaderDeps({ strict: false } as any) | ||
| const loaderData = useLoaderData({ strict: false } as any) | ||
| const params = useParams({ strict: false } as any) | ||
| const search = useSearch({ strict: false } as any) | ||
| const routeContext = useRouteContext({ strict: false } as any) | ||
| const routerState = useRouterState({ select: (state) => state.status } as any) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify strict TS settings and enumerate current `as any` usages in the changed benchmark surfaces.
set -euo pipefail
echo "== tsconfig files with strict options =="
fd -i 'tsconfig.*json' | xargs -I{} sh -c 'echo "--- {}"; rg -n "\"strict\"|\"noImplicitAny\"|\"strictNullChecks\"" "{}" || true'
echo
echo "== 'as any' usages in bundle-size scenarios =="
rg -n --type=ts --type=tsx '\bas any\b' benchmarks/bundle-size/scenariosRepository: TanStack/router
Length of output: 25034
Avoid as any in strict benchmark router-start calls
benchmarks/bundle-size/tsconfig.json enables "strict": true, but benchmarks/bundle-size/scenarios/vue-start-full/src/routes/__root.tsx uses as any in the core hook calls (lines 78-89), which bypasses type checking and can hide real API mismatches. Replace these as any assertions on useLinkProps, useMatch, useLoaderDeps, useLoaderData, useParams, useSearch, and useRouteContext with properly typed options (or narrower casts) so strict TS protections remain effective.
🤖 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/bundle-size/scenarios/vue-start-full/src/routes/__root.tsx` around
lines 78 - 89, The hook calls currently use "as any" which bypasses strict
types; remove those "as any" assertions and pass properly typed option objects
or explicit generics that match each hook's signature (e.g., for useLinkProps,
useMatch, useLoaderDeps, useLoaderData, useParams, useSearch, useRouteContext
and useRouterState) or use a narrower cast like Partial<ExpectedOptionsType>
when needed; locate the calls to useLinkProps, useMatchRoute, useMatches,
useParentMatches, useChildMatches, useMatch, useLoaderDeps, useLoaderData,
useParams, useSearch, useRouteContext and useRouterState in the file and replace
the blanket any casts with correct option types or explicit generic parameters
so the compiler enforces strict typing.
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud has identified a possible root cause for your failed CI:
We investigated the CI failures and determined they are not caused by this PR's changes. The local test failure is a known-flaky test (2.27% rate) with no relation to the bundle-size benchmark additions, and the remote build failures stem from an ENOTEMPTY filesystem error on the CI runner that prevented @tanstack/start-plugin-core from clearing its dist directory — a pre-existing environment issue. A rerun should resolve both failures.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
2 participants
Summary by CodeRabbit
New Features
Documentation