Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ Syntax reminders:

Conventions:

- Prefer `@import` at the **bottom** of the file.
- It's `@import("bun")` not `@import("root").bun`
- Prefer `@import` at the **bottom** of the file, but the auto formatter will move them so you don't need to worry about it.
- Prefer `@import("bun")`. Not `@import("root").bun` or `@import("../bun.zig")`.
- You must be patient with the build.
99 changes: 69 additions & 30 deletions src/Watcher.zig
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,39 @@ fn watchLoop(this: *Watcher) bun.sys.Maybe(void) {
return .success;
}

pub fn addFileDescriptorToKQueueWithoutChecks(this: *Watcher, fd: bun.FileDescriptor, watchlist_id: usize) void {
const KEvent = std.c.Kevent;

// https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/kqueue.2.html
var event = std.mem.zeroes(KEvent);

event.flags = std.c.EV.ADD | std.c.EV.CLEAR | std.c.EV.ENABLE;
// we want to know about the vnode
event.filter = std.c.EVFILT.VNODE;

event.fflags = std.c.NOTE.WRITE | std.c.NOTE.RENAME | std.c.NOTE.DELETE;

// id
event.ident = @intCast(fd.native());

// Store the index for fast filtering later
event.udata = @as(usize, @intCast(watchlist_id));
var events: [1]KEvent = .{event};

// This took a lot of work to figure out the right permutation
// Basically:
// - We register the event here.
// our while(true) loop above receives notification of changes to any of the events created here.
_ = std.posix.system.kevent(
this.platform.fd.unwrap().?.native(),
@as([]KEvent, events[0..1]).ptr,
1,
@as([]KEvent, events[0..1]).ptr,
0,
null,
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
fn appendFileAssumeCapacity(
this: *Watcher,
fd: bun.FileDescriptor,
Expand Down Expand Up @@ -350,36 +383,7 @@ fn appendFileAssumeCapacity(
};

if (comptime Environment.isMac) {
const KEvent = std.c.Kevent;

// https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/kqueue.2.html
var event = std.mem.zeroes(KEvent);

event.flags = std.c.EV.ADD | std.c.EV.CLEAR | std.c.EV.ENABLE;
// we want to know about the vnode
event.filter = std.c.EVFILT.VNODE;

event.fflags = std.c.NOTE.WRITE | std.c.NOTE.RENAME | std.c.NOTE.DELETE;

// id
event.ident = @intCast(fd.native());

// Store the hash for fast filtering later
event.udata = @as(usize, @intCast(watchlist_id));
var events: [1]KEvent = .{event};

// This took a lot of work to figure out the right permutation
// Basically:
// - We register the event here.
// our while(true) loop above receives notification of changes to any of the events created here.
_ = std.posix.system.kevent(
this.platform.fd.unwrap().?.native(),
@as([]KEvent, events[0..1]).ptr,
1,
@as([]KEvent, events[0..1]).ptr,
0,
null,
);
this.addFileDescriptorToKQueueWithoutChecks(fd, watchlist_id);
} else if (comptime Environment.isLinux) {
// var file_path_to_use_ = std.mem.trimRight(u8, file_path_, "/");
// var buf: [bun.MAX_PATH_BYTES+1]u8 = undefined;
Expand Down Expand Up @@ -612,6 +616,41 @@ pub fn addDirectory(
return this.appendDirectoryAssumeCapacity(fd, file_path, hash, clone_file_path);
}

pub fn addFileByPathSlow(
this: *Watcher,
file_path: string,
loader: options.Loader,
) bool {
const hash = getHash(file_path);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Fast path: check if already watching without opening the file
{
this.mutex.lock();
defer this.mutex.unlock();

if (this.indexOf(hash) != null) {
return true;
}
}
Comment on lines +648 to +656

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a nested block?


var fd: bun.FileDescriptor = bun.invalid_fd;
if (Environment.isMac) {
const path_z = std.posix.toPosixPath(file_path) catch return false;
switch (bun.sys.open(&path_z, bun.c.O_EVTONLY, 0)) {
.result => |opened| fd = opened,
.err => return false,
}
}
const res = this.addFile(fd, file_path, hash, loader, bun.invalid_fd, null, true);
switch (res) {
.result => return true,
.err => {
if (fd.isValid()) fd.close();
return false;
},
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub fn addFile(
this: *Watcher,
fd: bun.FileDescriptor,
Expand Down
11 changes: 6 additions & 5 deletions src/bun.js.zig
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ pub const Run = struct {
}

switch (this.ctx.debug.hot_reload) {
.hot => jsc.hot_reloader.HotReloader.enableHotModuleReloading(vm),
.watch => jsc.hot_reloader.WatchReloader.enableHotModuleReloading(vm),
.hot => jsc.hot_reloader.HotReloader.enableHotModuleReloading(vm, this.entry_path),
.watch => jsc.hot_reloader.WatchReloader.enableHotModuleReloading(vm, this.entry_path),
else => {},
}

Expand All @@ -328,6 +328,7 @@ pub const Run = struct {
promise.setHandled(vm.global.vm());

if (vm.hot_reload != .none or handled) {
vm.addMainToWatcherIfNeeded();
vm.eventLoop().tick();
vm.eventLoop().tickPossiblyForever();
} else {
Expand Down Expand Up @@ -389,21 +390,21 @@ pub const Run = struct {

{
if (this.vm.isWatcherEnabled()) {
vm.handlePendingInternalPromiseRejection();
vm.reportExceptionInHotReloadedModuleIfNeeded();

while (true) {
while (vm.isEventLoopAlive()) {
vm.tick();

// Report exceptions in hot-reloaded modules
vm.handlePendingInternalPromiseRejection();
vm.reportExceptionInHotReloadedModuleIfNeeded();

vm.eventLoop().autoTickActive();
}

vm.onBeforeExit();

vm.handlePendingInternalPromiseRejection();
vm.reportExceptionInHotReloadedModuleIfNeeded();

vm.eventLoop().tickPossiblyForever();
}
Expand Down
20 changes: 18 additions & 2 deletions src/bun.js/VirtualMachine.zig
Original file line number Diff line number Diff line change
Expand Up @@ -677,12 +677,28 @@ pub fn uncaughtException(this: *jsc.VirtualMachine, globalObject: *JSGlobalObjec
return handled;
}

pub fn handlePendingInternalPromiseRejection(this: *jsc.VirtualMachine) void {
var promise = this.pending_internal_promise.?;
pub fn reportExceptionInHotReloadedModuleIfNeeded(this: *jsc.VirtualMachine) void {
const promise_opt = this.pending_internal_promise;
if (promise_opt == null) {
this.addMainToWatcherIfNeeded();
return;
}
var promise = promise_opt.?;

@taylordotfish taylordotfish Oct 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: see followup suggestion; this can be even simpler

Suggested change
if (promise_opt == null) {
this.addMainToWatcherIfNeeded();
return;
}
var promise = promise_opt.?;
var promise = promise_opt orelse {
this.addMainToWatcherIfNeeded();
return;
};


if (promise.status(this.global.vm()) == .rejected and !promise.isHandled(this.global.vm())) {
this.unhandledRejection(this.global, promise.result(this.global.vm()), promise.asValue());
promise.setHandled(this.global.vm());
}

this.addMainToWatcherIfNeeded();
Comment thread
Jarred-Sumner marked this conversation as resolved.
Outdated
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
pub fn addMainToWatcherIfNeeded(this: *jsc.VirtualMachine) void {
if (this.isWatcherEnabled()) {
const main = this.main;
if (main.len == 0) return;
_ = this.bun_watcher.addFileByPathSlow(main, this.transpiler.options.loader(std.fs.path.extension(main)));
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub fn defaultOnUnhandledRejection(this: *jsc.VirtualMachine, _: *JSGlobalObject, value: JSValue) void {
Expand Down
97 changes: 90 additions & 7 deletions src/bun.js/hot_reloader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ pub const ImportWatcher = union(enum) {
};
}

pub inline fn addFileByPathSlow(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure this function needs to be semantically inlined?

this: ImportWatcher,
file_path: string,
loader: options.Loader,
) bool {
return switch (this) {
inline .hot, .watch => |w| w.addFileByPathSlow(file_path, loader),
else => true,
};
}
Comment on lines +28 to +37

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider removing the inline keyword.

The function is a simple delegation that switches on a union type. While inlining may provide minor performance benefits, the inline keyword forces semantic inlining which prevents the compiler from making optimization decisions. Unless profiling shows this is a hot path where inlining provides measurable benefit, consider removing inline and let the compiler decide.

As noted in past review comments, this was previously questioned by taylordotfish.

🤖 Prompt for AI Agents
In src/bun.js/hot_reloader.zig around lines 28 to 37, the function is declared
as "pub inline fn addFileByPathSlow" which forces semantic inlining; remove the
inline keyword so the compiler can decide whether to inline (change the
signature to "pub fn addFileByPathSlow(...)") and then rebuild and run tests to
ensure no performance regressions or regressions in behavior.


pub inline fn addFile(
this: ImportWatcher,
fd: bun.FD,
Expand Down Expand Up @@ -63,6 +74,8 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
verbose: bool = false,
pending_count: std.atomic.Value(u32) = std.atomic.Value(u32).init(0),

main: MainFile = .{},

tombstones: bun.StringHashMapUnmanaged(*bun.fs.FileSystem.RealFS.EntriesOption) = .{},

pub fn init(ctx: *Ctx, fs: *bun.fs.FileSystem, verbose: bool, clear_screen_flag: bool) *Watcher {
Expand Down Expand Up @@ -105,6 +118,44 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime

pub var clear_screen = false;

const MainFile = struct {
/// Includes a trailing "/"
dir: []const u8 = "",
dir_hash: Watcher.HashType = 0,

file: []const u8 = "",
hash: Watcher.HashType = 0,

/// On macOS, vim's atomic save triggers a race condition:
/// 1. Old file gets NOTE_RENAME (inode deleted)
/// 2. We receive the event and would normally trigger reload immediately
/// 3. But the new file isn't renamed into place yet - reload fails with ENOENT
/// 4. New file gets renamed into place (a.js~ -> a.js)
/// 5. Parent directory gets NOTE_WRITE
Comment thread
Jarred-Sumner marked this conversation as resolved.
Outdated
///
/// To fix this: when the entrypoint gets NOTE_RENAME, we set this flag
/// and skip the reload. Then when the parent directory gets NOTE_WRITE,
/// we check if the file exists and trigger the reload.
is_waiting_for_dir_change: bool = false,

pub fn init(file: []const u8) MainFile {
var main = MainFile{
.file = file,
.hash = if (file.len > 0) Watcher.getHash(file) else 0,
.is_waiting_for_dir_change = false,
};

if (std.fs.path.dirname(file)) |dir| {
bun.assert(bun.isSliceInBuffer(dir, file));
bun.assert(file.len > dir.len + 1);
main.dir = file[0 .. dir.len + 1];
main.dir_hash = Watcher.getHash(main.dir);
}
Comment on lines +148 to +153

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix directory hash mismatch so the macOS rename slow-path ever fires.

MainFile.init hashes the dirname without its trailing separator, but Watcher.appendFileMaybeLock records directory hashes on dirWithTrailingSlash(). Because of the mismatch, this.main.dir_hash == current_hash never becomes true, so the NOTE_WRITE from the parent directory is ignored, is_waiting_for_dir_change stays stuck, and the entry file is removed from the watchlist without ever scheduling a reload. After the first vim-style atomic save the hot-reload loop stalls permanently. Please hash the exact slice stored in main.dir (with its separator) so the comparison matches and the reload resumes.

-                    main.dir_hash = Watcher.getHash(dir);
+                    main.dir_hash = if (main.dir.len > 0) Watcher.getHash(main.dir) else 0;
📝 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.

Suggested change
if (std.fs.path.dirname(file)) |dir| {
bun.assert(bun.isSliceInBuffer(dir, file));
bun.assert(file.len > dir.len + 1);
main.dir = file[0 .. dir.len + 1];
main.dir_hash = Watcher.getHash(dir);
}
if (std.fs.path.dirname(file)) |dir| {
bun.assert(bun.isSliceInBuffer(dir, file));
bun.assert(file.len > dir.len + 1);
main.dir = file[0 .. dir.len + 1];
main.dir_hash = if (main.dir.len > 0) Watcher.getHash(main.dir) else 0;
}
🤖 Prompt for AI Agents
In src/bun.js/hot_reloader.zig around lines 148 to 153, MainFile.init currently
computes main.dir_hash from the dirname slice without its trailing separator but
stores main.dir as the dirname with the separator; update the logic so the hash
is computed over the exact slice stored in main.dir (i.e., include the trailing
separator) so the stored main.dir_hash matches the Watcher records; ensure you
still assert the slice is in the buffer and lengths are consistent before
assigning main.dir and then call Watcher.getHash on the same slice with the
separator.


return main;
}
};

pub const Task = struct {
count: u8 = 0,
hashes: [8]u32,
Expand Down Expand Up @@ -184,7 +235,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
}
};

pub fn enableHotModuleReloading(this: *Ctx) void {
pub fn enableHotModuleReloading(this: *Ctx, entry_path: ?[]const u8) void {
if (comptime @TypeOf(this.bun_watcher) == ImportWatcher) {
if (this.bun_watcher != .none)
return;
Expand All @@ -197,6 +248,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
reloader.* = .{
.ctx = this,
.verbose = Environment.enable_logs or if (@hasField(Ctx, "log")) this.log.level.atLeast(.info) else false,
.main = MainFile.init(entry_path orelse ""),
};

if (comptime @TypeOf(this.bun_watcher) == ImportWatcher) {
Expand Down Expand Up @@ -312,7 +364,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime

switch (kind) {
.file => {
if (event.op.delete or event.op.rename) {
if (event.op.delete or (event.op.rename and Environment.isMac)) {
ctx.removeAtIndex(
event.index,
0,
Expand All @@ -322,13 +374,29 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
}

if (this.verbose)
debug("File changed: {s}", .{fs.relativeTo(file_path)});
debug("File changed: {s} ({})", .{ fs.relativeTo(file_path), event });

if (event.op.write or event.op.delete or event.op.rename) {
if (comptime Environment.isMac) {
if (event.op.rename) {
// Special case for entrypoint: defer reload until we get
// a directory write event confirming the file exists.
// This handles vim's atomic save which deletes the old inode
// before the new file is renamed into place.
Comment thread
Jarred-Sumner marked this conversation as resolved.
Outdated
if (this.main.hash == current_hash and !reload_immediately) {
this.main.is_waiting_for_dir_change = true;
continue;
}
}

// If we got a write event after rename, the file is back - proceed with reload
if (this.main.is_waiting_for_dir_change and this.main.hash == current_hash) {
this.main.is_waiting_for_dir_change = false;
}
}

current_task.append(current_hash);
}

// TODO: delete events?
},
.directory => {
if (comptime Environment.isWindows) {
Expand All @@ -350,6 +418,19 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
entries_option = existing;
}

if (event.op.write) {
// Check if the entrypoint now exists after an atomic save.
// If we previously got a NOTE_RENAME on the entrypoint (vim deleted
// the old inode), this directory write event signals that the new
// file has been renamed into place. Verify it exists and trigger reload.
Comment thread
Jarred-Sumner marked this conversation as resolved.
Outdated
if (this.main.is_waiting_for_dir_change and this.main.dir_hash == current_hash) {
if (bun.sys.faccessat(file_descriptors[event.index], std.fs.path.basename(this.main.file)) == .result) {
this.main.is_waiting_for_dir_change = false;
current_task.append(this.main.hash);
}
}
}

var affected_i: usize = 0;

// if a file descriptor is stale, we need to close it
Expand Down Expand Up @@ -397,7 +478,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
bun.asByteSlice(changed_name_.?);
if (changed_name.len == 0 or changed_name[0] == '~' or changed_name[0] == '.') continue;

const loader = (this.ctx.getLoaders().get(Fs.PathName.init(changed_name).ext) orelse .file);
const loader = (this.ctx.getLoaders().get(Fs.PathName.findExtname(changed_name)) orelse .file);
var prev_entry_id: usize = std.math.maxInt(usize);
if (loader != .file) {
var path_string: bun.PathString = undefined;
Expand All @@ -414,6 +495,8 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
if (file_descriptors[entry_id].isValid()) {
if (prev_entry_id != entry_id) {
current_task.append(hashes[entry_id]);
if (this.verbose)
debug("Removing file: {s}", .{path_string.slice()});
ctx.removeAtIndex(
@as(u16, @truncate(entry_id)),
0,
Expand Down Expand Up @@ -452,7 +535,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
}

if (this.verbose) {
debug("Dir change: {s}", .{fs.relativeTo(file_path)});
debug("Dir change: {s} (affecting {d}, {})", .{ fs.relativeTo(file_path), affected.len, event });
}
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/bundler/bundle_v2.zig
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ pub const BundleV2 = struct {

const pool = try this.allocator().create(ThreadPool);
if (cli_watch_flag) {
Watcher.enableHotModuleReloading(this);
Watcher.enableHotModuleReloading(this, null);
}
// errdefer pool.destroy();
errdefer this.graph.heap.deinit();
Expand Down
Loading