fix: handle import attributes with type: "text" correctly #23300
Conversation
When a TypeScript file was imported with `type: "text"` import attribute,
the bundler would incorrectly reuse a cached parsed version of the file
that had been compiled as TypeScript, instead of treating it as plain text.
Root cause: The bundler's path-to-source-index cache didn't account for
files being imported with different loaders via import attributes.
Fix: Skip the path cache lookup when an import has an explicit loader
from import attributes (e.g., `with { type: "text" }`). This ensures
files are parsed with the correct loader even if they were previously
parsed with a different loader.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughImplements explicit loader-aware import handling in the bundler by bypassing cached source indices and avoiding parse-task reuse when loader types differ. Adjusts resolution to compare loaders before reuse. Adds regression tests ensuring TypeScript imports with with { type: "text" } are treated as text and not executed. Changes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/bundler/bundle_v2.zig (1)
3474-3487: Comment is misleading about control flow.The comment on line 3482 states "Fall through" but the code executes
continueon line 3486, which does not fall through—it skips to the next loop iteration. This creates confusion about the intended behavior.Additionally, this code path reveals a limitation: within a single file, if the same path is imported multiple times with different explicit loaders, only the first loader will be used. Consider clarifying the comment to explain this is a known limitation rather than suggesting a fall-through behavior.
Apply this diff to clarify the comment:
- // Fall through: Different loader required, but we can't add to resolve_queue - // with the same key. Set the path and move on - the loader mismatch will - // be handled when this import is resolved. + // Known limitation: Different loader required, but we can't add another entry + // to resolve_queue with the same key. The first loader wins for this path. + // This limitation only affects files that import the same path multiple times + // with different explicit loaders, which is uncommon. import_record.path = resolve_entry.value_ptr.*.path; continue;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/bundler/bundle_v2.zig(2 hunks)test/regression/issue/23299.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/23299.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/regression/issue/23299.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/regression/issue/23299.test.ts
test/regression/issue/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place regression tests under
test/regression/issue/and organize by issue numberPlace regression tests under test/regression/issue/ (one per bug fix)
Files:
test/regression/issue/23299.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Files:
test/regression/issue/23299.test.ts
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: In Zig source files, place all @import statements at the bottom of the file
Use @import("bun") instead of @import("root").bun when importing Bun in Zig
Files:
src/bundler/bundle_v2.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bundler/bundle_v2.zig
🧠 Learnings (10)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-10-04T09:51:30.265Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.265Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer running tests via bun bd test <file> and use provided harness utilities (bunEnv, bunExe, tempDir)
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-10-04T09:51:30.265Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.265Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-10-04T09:51:30.265Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.265Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-10-04T09:51:30.265Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.265Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/regression/issue/23299.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`
Applied to files:
test/regression/issue/23299.test.ts
🧬 Code graph analysis (1)
test/regression/issue/23299.test.ts (1)
test/harness.ts (3)
tempDir(278-285)bunExe(103-106)normalizeBunSnapshot(1809-1840)
🔇 Additional comments (3)
test/regression/issue/23299.test.ts (2)
4-51: LGTM! Well-structured regression test.The test correctly verifies that TypeScript files imported with
type: "text"are treated as plain text and not executed. Good use of harness utilities, proper resource cleanup withusing, and appropriate assertions including snapshot verification.Based on learnings.
53-80: LGTM! Clean and focused test.The test properly verifies that TypeScript source code is preserved as text in the bundle when imported with
type: "text", ensuring no compilation occurs.Based on learnings.
src/bundler/bundle_v2.zig (1)
3448-3467: Core fix correctly skips cache for explicit loaders.The introduction of
has_explicit_loaderand the conditional cache lookup properly address the issue where cached parse results were reused regardless of the loader specified in import attributes. When an explicit loader is present (e.g.,with { type: "text" }), the cache is bypassed, ensuring the file is parsed with the correct loader.
|
I tested this PR using |
|
Closing PR - user @TNTSuperMan tested and confirmed the issue still persists. The fix doesn't actually work. Additionally, CI is failing on unrelated tests. |
Fixes #23299
What does this PR do?
Fixes a bug where TypeScript files imported with
type: "text"import attributes were incorrectly executed as TypeScript instead of being treated as plain text.Root cause: The bundler's
pathToSourceIndexMapcache didn't account for files being imported with different loaders via import attributes. When the same file was encountered with a different loader, it would skip re-parsing and reuse the cached version with the wrong loader.Solution: Skip the path cache lookup when an import has an explicit loader from import attributes (e.g.,
with { type: "text" }). This ensures files are parsed with the correct loader even if they were previously parsed with a different loader.How did you verify your code works?
Added regression tests in
test/regression/issue/23299.test.ts:.tsfiles imported withtype: "text"are treated as text, not executed as TypeScriptAll tests pass:
🤖 Generated with Claude Code