Fix hot reload for files loaded via Bun.plugin onLoad #24432
Conversation
When using `bun --hot` with plugins that use `onLoad`, only the entrypoint's changes would trigger hot reloads. Files imported and transformed by plugins were not being watched, so changes to them wouldn't trigger reloads. The issue was that `Bun__runVirtualModule` (called for all module imports) would run plugin `onLoad` callbacks but never added the source files to the hot reloader's watch list. This fix adds file watching for plugin-loaded modules when: 1. A plugin's onLoad callback successfully handles the module 2. Hot reloading is enabled 3. The file is in the "file" namespace (real filesystem) 4. The path is absolute and not in node_modules Added test to verify files loaded through plugins now trigger hot reloads. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Updated 4:40 PM PT - Nov 6th, 2025
❌ @RiskyMH, your commit 341fa14 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 24432That installs a local version of the PR into your bun-24432 --bun |
WalkthroughEnhances plugin-loaded module hot-reload support by integrating file watcher registration into the module loading flow. When plugins handle file loading, the system now registers the file with the watcher to enable hot-reload detection when the file changes, with conditions ensuring only valid filesystem paths are watched. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (2)
src/bun.js/ModuleLoader.zig(1 hunks)test/cli/hot/hot-plugin-onload.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.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.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 (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use Zig private fields with the # prefix for encapsulation (e.g., struct { #foo: u32 })
Prefer Decl literals for initialization (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Place @import statements at the bottom of the file (formatter will handle ordering)
src/**/*.zig: In Zig code, manage memory carefully: use appropriate allocators and defer for cleanup
Cache JavaScriptCore class structures in ZigGlobalObject when adding new classes
Files:
src/bun.js/ModuleLoader.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/cli/hot/hot-plugin-onload.test.ts
test/cli/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/cli/**/*.{js,ts}: Place CLI command tests (e.g., bun install, bun init) under test/cli/
When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Files:
test/cli/hot/hot-plugin-onload.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/cli/hot/hot-plugin-onload.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/cli/hot/hot-plugin-onload.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/cli/hot/hot-plugin-onload.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 invent random port functions
Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests
Never write tests that assert absence of crashes (e.g., no "panic" or "uncaught exception") in output
Use tempDir from "harness" for temporary directories; do not use tmpdirSync or fs.mkdtempSync in tests
When spawning processes in tests, assert on stdout before asserting exitCode
Do not use setTimeout in tests; await conditions instead to avoid flakiness
Files:
test/cli/hot/hot-plugin-onload.test.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid shell commands in tests (e.g., find, grep); use Bun's Glob and built-in tools
Files:
test/cli/hot/hot-plugin-onload.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : When a change causes a hard reload, wrap it in c.expectReload to annotate the single expected reload
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Run bun run zig:check-all for cross-platform Zig checks when making platform-specific changes
Learnt from: CR
Repo: oven-sh/bun PR: 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
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 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:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
src/bun.js/ModuleLoader.zigtest/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 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.zig
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/js/node/test/{parallel,sequential}/*.js : For `test/js/node/test/{parallel,sequential}/*.js` (no .test extension), run with `bun bd <file>` instead of `bun bd test <file>`
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/js/{bun,node}/** : Organize unit tests by module under `/test/js/bun/` and `/test/js/node/`
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 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.zig
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Applied to files:
src/bun.js/ModuleLoader.zig
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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:
src/bun.js/ModuleLoader.zigtest/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/plugins.test.ts : plugins.test.ts should contain plugin-related development-mode tests
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : When a change causes a hard reload, wrap it in c.expectReload to annotate the single expected reload
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeouts
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/cli/hot/hot-plugin-onload.test.ts
🧬 Code graph analysis (1)
test/cli/hot/hot-plugin-onload.test.ts (1)
test/harness.ts (2)
tmpdirSync(1211-1213)bunExe(102-105)
🔇 Additional comments (1)
src/bun.js/ModuleLoader.zig (1)
1117-1136: Double-check Windows paths before registering watchers
On Windows the"file"namespace specifiers come through as URLs likefile:///C:/foo/custom.ts. After slicing off the namespace we end up with///C:/foo/custom.ts, whichstd.fs.path.isAbsoluteaccepts, butaddFileByPathSlowwill treat as a UNC path and fail to watch the file. Can we normalize the string to an OS path (e.g. viabun.path.fromFileURLor equivalent) before handing it to the watcher? Please verify this on a Windows build.
| let cwd = ""; | ||
| beforeEach(() => { | ||
| cwd = tmpdirSync(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use tempDir from the harness instead of tmpdirSync
Harness guidance says to prefer tempDir() so temporary directories are tracked and cleaned up automatically. Switching the beforeEach to cwd = await tempDir(); keeps the test compliant and avoids leaking dirs.
🤖 Prompt for AI Agents
In test/cli/hot/hot-plugin-onload.test.ts around lines 9 to 12, the test uses
tmpdirSync to create a temp dir; change the beforeEach to an async function and
assign cwd = await tempDir(); so the harness tracks and cleans the directory
automatically; ensure tempDir is imported/available from the test harness and
adjust any typing if needed.
| const timeout = setTimeout(() => { | ||
| finished = true; | ||
| runner.kill(9); | ||
| }, 5000); | ||
|
|
||
| var str = ""; | ||
| for await (const line of runner.stdout) { | ||
| if (finished) break; | ||
| str += new TextDecoder().decode(line); | ||
| var any = false; | ||
| if (!/\[#!root\] Value:/g.test(str)) continue; | ||
|
|
||
| for (let line of str.split("\n")) { | ||
| if (!line.includes("[#!root]")) continue; | ||
| reloadCounter++; | ||
| str = ""; | ||
|
|
||
| if (reloadCounter === 3) { | ||
| clearTimeout(timeout); | ||
| runner.unref(); | ||
| runner.kill(); | ||
| finished = true; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Avoid setTimeout in tests to keep them deterministic
The hot-reload harness shouldn’t rely on timers; the repo guidance asks us to avoid setTimeout in tests because it introduces flakiness. Instead of scheduling a kill, track a deadline inside the read loop and bail once it’s exceeded. For example:
- const timeout = setTimeout(() => {
- finished = true;
- runner.kill(9);
- }, 5000);
+ const deadline = Date.now() + 5_000;
...
- if (finished) break;
+ if (finished) break;
str += new TextDecoder().decode(line);
var any = false;
if (!/\[#!root\] Value:/g.test(str)) continue;
...
- if (reloadCounter === 3) {
- clearTimeout(timeout);
+ if (reloadCounter === 3) {
runner.unref();
runner.kill();
finished = true;
break;
}
...
- if (any) await onReload();
+ if (any) {
+ await onReload();
+ } else if (Date.now() > deadline) {
+ finished = true;
+ runner.kill(9);
+ break;
+ }That keeps the safety net without the forbidden timer.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In test/cli/hot/hot-plugin-onload.test.ts around lines 73 to 96, remove the
setTimeout-based safety kill and instead implement a deadline check inside the
for-await loop: capture a deadline timestamp (e.g. Date.now() + 5000) before
entering the loop, drop the timeout/clearTimeout logic, and on each iteration
check if Date.now() > deadline; if exceeded, perform the same shutdown sequence
(runner.kill(9), runner.unref(), mark finished) and break. Ensure you only call
kill/unref once and preserve the existing reloadCounter logic and the 5s safety
window.
- Use test.each to test both --hot and --watch flags - Rename file to plugin-onload.test.ts (more generic name) - Use tempDir with 'using' for automatic cleanup - Remove unnecessary beforeEach hook - Simplify counter logic
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (1)
test/cli/hot/plugin-onload.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/cli/hot/plugin-onload.test.ts
test/cli/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/cli/**/*.{js,ts}: Place CLI command tests (e.g., bun install, bun init) under test/cli/
When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Files:
test/cli/hot/plugin-onload.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/cli/hot/plugin-onload.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/cli/hot/plugin-onload.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/cli/hot/plugin-onload.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 invent random port functions
Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests
Never write tests that assert absence of crashes (e.g., no "panic" or "uncaught exception") in output
Use tempDir from "harness" for temporary directories; do not use tmpdirSync or fs.mkdtempSync in tests
When spawning processes in tests, assert on stdout before asserting exitCode
Do not use setTimeout in tests; await conditions instead to avoid flakiness
Files:
test/cli/hot/plugin-onload.test.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid shell commands in tests (e.g., find, grep); use Bun's Glob and built-in tools
Files:
test/cli/hot/plugin-onload.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : When a change causes a hard reload, wrap it in c.expectReload to annotate the single expected reload
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/plugins.test.ts : plugins.test.ts should contain plugin-related development-mode tests
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : When a change causes a hard reload, wrap it in c.expectReload to annotate the single expected reload
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeouts
Applied to files:
test/cli/hot/plugin-onload.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/cli/hot/plugin-onload.test.ts
🧬 Code graph analysis (1)
test/cli/hot/plugin-onload.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
| const timeout = isDebug ? Infinity : 10_000; | ||
|
|
||
| test.each(["--hot", "--watch"])( |
There was a problem hiding this comment.
Remove custom per-test timeout override.
Hard-coding a 10 s/∞ timeout conflicts with our testing guideline to avoid explicit test timeouts. It also masks hung runs—Infinity in debug mode can leave the suite stuck—and bun:test already enforces a 5 s default per test (and offers overrides like setDefaultTimeout when you truly need them). Please drop the helper and rely on the built-in behaviour.
-const timeout = isDebug ? Infinity : 10_000;
-
test.each(["--hot", "--watch"])(
"should reload imported files when using Bun.plugin onLoad with %s",
async flag => {
@@
- },
- timeout,
+ },
);
test.each(["--hot", "--watch"])(
"should reload imported files when NOT using Bun.plugin (control test) with %s",
async flag => {
@@
- },
- timeout,
+ },
);As per coding guidelines.
Also applies to: 99-100, 164-165
🤖 Prompt for AI Agents
In test/cli/hot/plugin-onload.test.ts around lines 7-9 (also remove related uses
at lines 99-100 and 164-165), remove the custom per-test timeout override (the
isDebug/timeout constant and any per-test application of it) so tests rely on
the test runner's default timeout; delete the timeout variable declaration and
all references where it is passed to test/timeout helpers, and ensure no other
code depends on that variable (replace with no-arg test calls or use
setDefaultTimeout only if truly necessary).
| const killTimeout = setTimeout(() => { | ||
| finished = true; | ||
| runner.kill(9); | ||
| }, 5000); |
There was a problem hiding this comment.
Replace setTimeout watchdog with deterministic deadline check.
Using setTimeout in tests is explicitly discouraged, and the current watchdog introduces timer-based flakiness. We can enforce the same 5 s guard by tracking a deadline inside the read loop, which keeps the behaviour deterministic and guideline-compliant.
- const killTimeout = setTimeout(() => {
- finished = true;
- runner.kill(9);
- }, 5000);
+ const abortAt = Date.now() + 5_000;
@@
- if (reloadCounter === 3) {
- clearTimeout(killTimeout);
+ if (reloadCounter === 3) {
runner.unref();
runner.kill();
finished = true;
break;
}
@@
- if (any) await onReload();
+ if (any) await onReload();
+
+ if (!finished && Date.now() >= abortAt) {
+ finished = true;
+ runner.kill(9);
+ break;
+ }As per coding guidelines.
Also applies to: 75-88
🤖 Prompt for AI Agents
In test/cli/hot/plugin-onload.test.ts around lines 58 to 61 (and similarly at
75–88), replace the non-deterministic setTimeout watchdog with a deterministic
deadline check: remove the setTimeout/killTimeout, create a const deadline =
Date.now() + 5000 before the read loop, and inside the loop check if (Date.now()
> deadline) then mark finished = true, call runner.kill(9) and break out of the
loop; ensure any clearTimeout/killTimeout references are deleted and the logic
uses the deadline check to enforce the same 5s guard deterministically.
Summary
Fixes a bug where files transformed by
Bun.pluginonLoadcallbacks wouldn't trigger hot reloads when changed, even though--hotwas enabled.Problem
When using
bun --hotwith plugins:onLoadwere not watchedRoot Cause
Bun__runVirtualModule(called for all module imports) would execute pluginonLoadcallbacks but never added the source files to the hot reloader's watch list.Solution
Added file watching in
Bun__runVirtualModulewhen:onLoadcallback successfully handles a module--hotflag)node_modulesTest Plan
Added test
test/cli/hot/hot-plugin-onload.test.tsthat verifies:Ran existing hot reload tests - all pass except one pre-existing flaky test.
🤖 Generated with Claude Code