Repository navigation
Conversation
Previously, importing an invalid bun: module like `import foo from "bun:apskdaposkdpok"` would strip the prefix and show a confusing error: error: Cannot find package 'apskdaposkdpok' from '<file>' This was misleading since the user wrote `bun:apskdaposkdpok`, not `apskdaposkdpok`. The code now validates that a bun: module exists in HardcodedModule.map before stripping the prefix during transpilation/linking. Invalid modules preserve their full name, producing clearer errors: error: Cannot resolve invalid URL 'bun:apskdaposkdpok' from '<file>' Implementation: - Added validation check using HardcodedModule.map.has() before stripping prefix - Applied consistently across ModuleLoader.zig, linker.zig, and bundle_v2.zig - Uses ComptimeStringMap for O(1) validation with no runtime overhead - Clear comments explain the validation purpose Testing: - Added regression test: test/regression/issue/invalid-bun-module-prefix.test.ts - Verified existing test passes: test/js/node/missing-module.test.js - Confirmed valid bun: modules still work correctly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Updated 3:30 AM PT - Oct 10th, 2025
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 23439That installs a local version of the PR into your bun-23439 --bun |
WalkthroughValidate bun: imports against HardcodedModule.map before normalizing. Known builtin bun: paths are stripped and marked external; unknown bun: imports are preserved (full bun: name) to improve error reporting. bun:test tagging applies only when the validated name is "test". Adds a regression test for invalid bun: modules. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)test/**📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.test.ts📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/regression/issue/**📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-09-03T17:10:13.486ZApplied to files:
📚 Learning: 2025-10-04T09:51:30.294ZApplied to files:
📚 Learning: 2025-09-03T17:10:13.486ZApplied to files:
📚 Learning: 2025-10-04T09:51:30.294ZApplied to files:
📚 Learning: 2025-10-04T09:51:30.294ZApplied to files:
📚 Learning: 2025-10-04T09:51:30.294ZApplied to files:
📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-10-04T09:52:49.414ZApplied to files:
📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-09-03T17:10:13.486ZApplied to files:
📚 Learning: 2025-09-03T17:10:13.486ZApplied to files:
📚 Learning: 2025-10-04T09:51:30.294ZApplied to files:
📚 Learning: 2025-09-03T17:10:13.486ZApplied to files:
📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-10-08T13:48:02.430ZApplied to files:
🧬 Code graph analysis (1)test/regression/issue/invalid-bun-module-prefix.test.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (4)
src/bun.js/ModuleLoader.zig(1 hunks)src/bundler/bundle_v2.zig(1 hunks)src/linker.zig(1 hunks)test/regression/issue/invalid-bun-module-prefix.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/invalid-bun-module-prefix.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/invalid-bun-module-prefix.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/invalid-bun-module-prefix.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/invalid-bun-module-prefix.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/invalid-bun-module-prefix.test.ts
**/*.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/bun.js/ModuleLoader.zigsrc/linker.zigsrc/bundler/bundle_v2.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/ModuleLoader.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bunWhen adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
Files:
src/bun.js/ModuleLoader.zigsrc/linker.zigsrc/bundler/bundle_v2.zig
🧠 Learnings (17)
📚 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/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/regression/issue/invalid-bun-module-prefix.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/invalid-bun-module-prefix.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/js/bun/** : Place Bun-specific API tests under test/js/bun/
Applied to files:
test/regression/issue/invalid-bun-module-prefix.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/invalid-bun-module-prefix.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
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/invalid-bun-module-prefix.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
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/invalid-bun-module-prefix.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
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/invalid-bun-module-prefix.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/invalid-bun-module-prefix.test.ts
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/
Applied to files:
test/regression/issue/invalid-bun-module-prefix.test.tssrc/bun.js/ModuleLoader.zig
📚 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/invalid-bun-module-prefix.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/node/test/** : Vendored Node.js tests under test/js/node/test/ are exceptions and do not follow Bun’s test style
Applied to files:
test/regression/issue/invalid-bun-module-prefix.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/invalid-bun-module-prefix.test.ts
📚 Learning: 2025-10-07T03:19:53.031Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/CLAUDE.md:0-0
Timestamp: 2025-10-07T03:19:53.031Z
Learning: Applies to src/**/*.zig : Use import("bun") instead of import("root").bun
Applied to files:
src/bun.js/ModuleLoader.zigsrc/linker.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to src/js/bun/** : Organize Bun-specific built-in modules under src/js/bun/ (e.g., bun:ffi, bun:sqlite)
Applied to files:
src/bun.js/ModuleLoader.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
src/bun.js/ModuleLoader.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect
Applied to files:
src/bun.js/ModuleLoader.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
PR: oven-sh/bun#22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/linker.zigsrc/bundler/bundle_v2.zig
🧬 Code graph analysis (1)
test/regression/issue/invalid-bun-module-prefix.test.ts (1)
test/harness.ts (1)
bunExe(102-105)
🔇 Additional comments (3)
src/bun.js/ModuleLoader.zig (1)
2556-2570: LGTM! Correct validation and conditional handling of bun: prefixed imports.The implementation properly validates bun: prefixed modules against
HardcodedModule.mapbefore stripping the prefix. Invalid modules are left unchanged, ensuring error messages display the fullbun:invalidspecifier rather than justinvalid. The conditionalbun_testtagging (only when the stripped path equals "test") is also correct.src/linker.zig (1)
178-192: LGTM! Consistent validation logic with ModuleLoader.zig.The linker implementation mirrors the validation approach in
ModuleLoader.zig, checkingHardcodedModule.mapbefore stripping thebun:prefix. Thecontinuestatement at line 190 ensures that only validated bun: modules skip the linking process, while invalid ones fall through to generate proper error messages with the fullbun:invalidspecifier.src/bundler/bundle_v2.zig (1)
3177-3193: LGTM! Defensive validation improves error messages.The conditional check before stripping the
bun:prefix is a good defensive programming practice. By validating that the module exists inHardcodedModule.mapbefore processing, invalid modules likebun:apskdaposkdpokwill preserve their full prefix in error messages, making it clearer to users what they attempted to import.The logic correctly:
- Preserves the prefix for invalid modules, allowing them to fall through to normal resolution
- Strips the prefix only for valid builtin modules
- Sets the
bun_testtag after stripping, so it checks the base name "test" rather than "bun:test"
- Remove manual mkdtempSync/writeFileSync usage - Use tempDirWithFiles for automatic cleanup - Follows harness testing guidelines
|
Closing this PR because it has been inactive for more than 90 days. |
1 participant
What does this PR do?
Fixes the confusing error message when importing invalid
bun:modules by preserving the full module name in error messages.Before:
After:
The code previously stripped the
bun:prefix unconditionally during transpilation/linking, causing the resolver to report errors with the stripped name instead of what the user actually wrote.This fix adds validation using
HardcodedModule.map.has()before stripping the prefix in three locations:src/bun.js/ModuleLoader.zig- Runtime transpilersrc/linker.zig- Module linkersrc/bundler/bundle_v2.zig- Bundler v2Invalid modules are now left unchanged so the resolver can report the full
bun:...name in error messages.How did you verify your code works?
Added regression test:
test/regression/issue/invalid-bun-module-prefix.test.tsbun:modules preserve the full prefix in errorstempDirWithFilesharness utility for proper cleanupVerified existing tests pass:
test/js/node/missing-module.test.jsManual testing:
bun:modules (bun:sqlite,bun:ffi, etc.) still work correctlybun:doesnotexist) show the full name in errorsPerformance: No runtime overhead - uses
ComptimeStringMapwhich compiles to O(1) lookups.🤖 Generated with Claude Code