Resolve per-file JSX settings from nearest enclosing tsconfig.json at runtime#28606
Resolve per-file JSX settings from nearest enclosing tsconfig.json at runtime#28606
Conversation
… runtime Runtime transpilation (bun run) was using global JSX settings derived from the root directory's tsconfig.json for all files. When a source file had a closer tsconfig.json with a different jsxImportSource, it was ignored. Look up the enclosing tsconfig.json for each file's directory during runtime transpilation in ModuleLoader and RuntimeTranspilerStore, matching the behavior already used by the bundler path.
|
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:
WalkthroughPer-file TypeScript/transpile parse options are now resolved by locating the nearest enclosing Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/RuntimeTranspilerStore.zig (1)
378-404: 🧹 Nitpick | 🔵 TrivialAdd one post-startup
import()regression for this path.Both new fixtures use only static imports from the entry module.
Bun__transpileFile()only routes non-main modules throughRuntimeTranspilerStoreaftervm.has_loadedis true, so these tests likely validateModuleLoader.transpileSourceCode()but never executeTranspilerJob.run(). A smallawait import("./packages/ui/component.tsx")case after startup would pin the worker-thread merge too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/RuntimeTranspilerStore.zig` around lines 378 - 404, Tests only use static imports so Bun__transpileFile routes modules before vm.has_loaded and never exercises RuntimeTranspilerStore/TranspilerJob.run; add a post-startup dynamic import to the fixture entry to force runtime transpilation—e.g., after startup await import("./packages/ui/component.tsx")—so Bun__transpileFile will route the module through RuntimeTranspilerStore and exercise ModuleLoader.transpileSourceCode and TranspilerJob.run; update the test fixture entry module (the file that currently only does static imports) to include this single await import line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/bun.js/RuntimeTranspilerStore.zig`:
- Around line 378-404: Tests only use static imports so Bun__transpileFile
routes modules before vm.has_loaded and never exercises
RuntimeTranspilerStore/TranspilerJob.run; add a post-startup dynamic import to
the fixture entry to force runtime transpilation—e.g., after startup await
import("./packages/ui/component.tsx")—so Bun__transpileFile will route the
module through RuntimeTranspilerStore and exercise
ModuleLoader.transpileSourceCode and TranspilerJob.run; update the test fixture
entry module (the file that currently only does static imports) to include this
single await import line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c1dd150-aa3f-4177-aee3-cdf33c38059b
📒 Files selected for processing (3)
src/bun.js/ModuleLoader.zigsrc/bun.js/RuntimeTranspilerStore.zigtest/regression/issue/28605.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward bug fix that mirrors the bundler's existing per-file tsconfig resolution, with regression tests.
Extended reasoning...
Overview
This PR adds per-file JSX and decorator settings resolution to the runtime transpiler paths (ModuleLoader.zig and RuntimeTranspilerStore.zig). Before each file is transpiled, it now calls readDirInfo on the file's directory, finds the nearest enclosing tsconfig.json, and merges its JSX/decorator settings using the same mergeJSX + boolean-OR pattern the bundler already uses in finalizeResult. Two regression tests in test/regression/issue/28605.test.ts cover the no-root-tsconfig and nested-override scenarios.
Security risks
None. This change only affects compile-time transpilation settings (JSX transform mode and decorator flags). No user-controlled data flows into security-sensitive paths.
Level of scrutiny
Low. The code change is small (~15 lines duplicated in two symmetric locations), follows a well-established pattern already present in the resolver, and the DirInfo cache means the extra readDirInfo call is effectively free.
Other factors
The only noted concern — decorator boolean-OR preventing a nested tsconfig from disabling root-enabled decorators — is a pre-existing limitation that predates this PR and matches the bundler's long-standing behavior. Fixing it properly would require a broader TSConfigJSON data-model change. The PR improves the important case (nested enables what root omits) without regressing the edge case.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/regression/issue/28605.test.ts`:
- Around line 64-122: The dynamic import in entry.ts currently re-imports the
same module (packages/ui/component.tsx) so Bun may return a cached transpiled
module and not exercise the RuntimeTranspilerStore path; create a new TSX module
(e.g., packages/ui/dynamic-component.tsx) exporting Component (same
implementation as packages/ui/component.tsx) and update entry.ts to dynamically
import "./packages/ui/dynamic-component.tsx" for the post-startup import so the
dynamic path transpires a distinct file.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 38687bf6-f840-4adb-b22e-f2fce02507d3
📒 Files selected for processing (1)
test/regression/issue/28605.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/regression/issue/28605.test.ts`:
- Around line 31-35: The test only covers merging of JSX-related tsconfig fields
(jsx and jsxImportSource) but not the decorator branch; update the test to also
exercise merging of experimentalDecorators and emitDecoratorMetadata by adding a
nested tsconfig fixture that sets those two flags (similar to the existing
nested tsconfig JSON snippet with jsx) or alternatively refactor the runtime
merge logic out so the decorator-path is tested separately; ensure the new
nested tsconfig uses experimentalDecorators: true and emitDecoratorMetadata:
true and update the corresponding assertion block (the other similar JSON
snippet later in the test) so both branches are validated.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1318da5c-d613-4813-b681-596a5815e38e
📒 Files selected for processing (1)
test/regression/issue/28605.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/ModuleLoader.zig`:
- Around line 226-239: The current code seeds
jsx/emit_decorator_metadata/experimental_decorators from
jsc_vm.transpiler.options (e.g., jsc_vm.transpiler.options.jsx) which lets the
root tsconfig leak into nested files; change the initialization to use default
values (or only explicit process-wide overrides) instead of
jsc_vm.transpiler.options, then apply the enclosing tsconfig merge
(tsconfig.mergeJSX and the
tsconfig.emit_decorator_metadata/tsconfig.experimental_decorators) so that an
enclosing tsconfig can only set or clear fields relative to fresh defaults; make
the identical change in the twin block in RuntimeTranspilerStore.zig to ensure
consistent nearest-enclosing precedence.
In `@src/bun.js/RuntimeTranspilerStore.zig`:
- Around line 378-391: The tsconfig lookup inside RuntimeTranspilerStore's async
job hot path (the block calling transpiler.resolver.readDirInfo and reading
dir_info.enclosing_tsconfig_json to mergeJSX and set
emit_decorator_metadata/experimental_decorators) is causing resolver mutex
contention; move that lookup out of the worker-thread transpile job by resolving
and stashing the enclosing tsconfig during module resolution (where the resolver
already runs), compute the derived JSX and decorator booleans there (using
tsconfig.mergeJSX, tsconfig.emit_decorator_metadata,
tsconfig.experimental_decorators), and pass those derived values into the async
transpile job interface instead of calling readDirInfo from inside
RuntimeTranspilerStore.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62083a18-35c2-4a66-b6e1-db103182154c
📒 Files selected for processing (2)
src/bun.js/ModuleLoader.zigsrc/bun.js/RuntimeTranspilerStore.zig
…pment, include runtime in cache hash - Skip readDirInfo when path.name.dir is empty (worker blobs, eval) to avoid assertf panic in the resolver - After mergeJSX, restore jsx.development from force_node_env to prevent tsconfig's react-jsx mapping from overriding production mode, matching the bundle_v2.zig correction pattern - Include JSX runtime enum in hashForRuntimeTranspiler to prevent cache collisions between files with different JSX runtimes, and bump cache version to invalidate stale entries
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/RuntimeTranspilerCache.zig (1)
1-18:⚠️ Potential issue | 🟡 MinorAdd a Version 19 ledger entry above
expected_version.Line 18 bumps the format version, but the in-file version history still stops at Version 18. Please add a Version 19 note so stale-cache investigations stay traceable.
🧾 Suggested patch
/// Version 18: Include ESM record (module info) with an ES Module, see `#15758` +/// Version 19: Include JSX runtime in runtime-transpiler feature hashing (per-file JSX runtime cache separation). const expected_version = 19;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/RuntimeTranspilerCache.zig` around lines 1 - 18, Add a "Version 19" ledger entry immediately above the expected_version constant in RuntimeTranspilerCache.zig to match the bumped format number; update the comment block that lists previous versions (the multiline history above expected_version) to include a new line beginning with "Version 19:" and a short note describing the change that caused the bump so the in-file history stays in sync with the expected_version symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/bun.js/RuntimeTranspilerCache.zig`:
- Around line 1-18: Add a "Version 19" ledger entry immediately above the
expected_version constant in RuntimeTranspilerCache.zig to match the bumped
format number; update the comment block that lists previous versions (the
multiline history above expected_version) to include a new line beginning with
"Version 19:" and a short note describing the change that caused the bump so the
in-file history stays in sync with the expected_version symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d24a94a7-4995-4ec8-a6d8-093df5b96931
📒 Files selected for processing (4)
src/bun.js/ModuleLoader.zigsrc/bun.js/RuntimeTranspilerCache.zigsrc/bun.js/RuntimeTranspilerStore.zigsrc/options.zig
There was a problem hiding this comment.
LGTM — all reported issues have been addressed.
Extended reasoning...
Overview
PR fixes issue #28605: runtime transpiler (ModuleLoader.zig, RuntimeTranspilerStore.zig) now resolves per-file JSX/decorator settings from the nearest enclosing tsconfig.json, matching the bundler's existing finalizeResult behavior. Changes touch RuntimeTranspilerCache.zig (version bump 18→19) and options.zig (hash improvements).
Security risks
No security-sensitive code paths (auth, crypto, permissions) are touched. The readDirInfo call acquires the resolver mutex but only performs a cached hash-map lookup. No injection vectors introduced.
Level of scrutiny
High — this is production-critical transpilation code. The PR went through multiple fix iterations:
- Accidental revert of the core fix was caught and restored
- Empty-path crash for worker/blob scripts (bun.assertf panic that catch null cannot intercept) was caught and fixed with the path.name.dir.len > 0 guard
- Production mode JSX corruption (mergeJSX overwriting development=false with true) was caught and fixed with the force_node_env correction
- Cache hash collisions (missing runtime and development fields) were caught and both fields were added
All four issues have been addressed in dccddaf and 413117b. The fix correctly mirrors the bundler path in resolver.zig:finalizeResult.
Other factors
Two regression tests cover both the static import path (ModuleLoader.zig) and the dynamic import path (RuntimeTranspilerStore.zig) with distinct files to avoid module cache interference. CI retriggers appear to be infrastructure-related (TLS certificate issue per commit f396fa2 message), not code failures. All inline reviewer comments are resolved.
1 participant
Fixes #28605
Problem
When running
bun run ./services/connect/src/app.tsfrom a workspace root that has notsconfig.json, Bun ignoredjsxImportSource: "chat"from the nestedservices/connect/tsconfig.jsonand fell back to the defaultreact, causing:Cause
Runtime transpilation in
ModuleLoader.zigandRuntimeTranspilerStore.zigalways used the globaltranspiler.options.jsxsettings — derived once at startup from the root directory's tsconfig.json — for every file. The per-file tsconfig resolution viaenclosing_tsconfig_json(which walks up from the file's directory) was only used in the bundler code path, not the runtimebun runpath.Fix
Before transpiling each file at runtime, look up its directory's
DirInfoviareadDirInfoand merge theenclosing_tsconfig_json's JSX/decorator settings, matching the behavior the bundler already uses infinalizeResult.Verification
USE_SYSTEM_BUN=1 bun test test/regression/issue/28605.test.ts→ 2 fail (bug exists)bun bd test test/regression/issue/28605.test.ts→ 2 pass (fix works)chat/jsx-dev-runtime