From d1c77f5061bc17daa0d9db69d2418e39846f2f64 Mon Sep 17 00:00:00 2001 From: chloe caruso Date: Fri, 14 Mar 2025 21:24:14 -0700 Subject: [PATCH] fix dev server regressions from 1.2.5's hmr rewrite (#18109) Co-authored-by: Zack Radisic Co-authored-by: zackradisic <56137411+zackradisic@users.noreply.github.com> --- packages/bun-types/devserver.d.ts | 3 +- src/bake/DevServer.zig | 195 ++++++++++-- src/bake/bake.private.d.ts | 13 +- src/bake/client/css-reloader.ts | 3 +- src/bake/debug.ts | 14 + src/bake/hmr-module.ts | 181 +++++------ src/bake/hmr-runtime-client.ts | 7 + src/bake/hmr-runtime-error.ts | 1 + src/bake/hmr-runtime-server.ts | 1 + src/bun.js/api/server.zig | 8 +- src/bundler/bundle_v2.zig | 34 +-- src/codegen/bake-codegen.ts | 4 +- src/js_ast.zig | 18 +- src/js_parser.zig | 177 ++++------- src/ptr/CowSlice.zig | 2 +- src/ptr/tagged_pointer.zig | 21 +- src/string_immutable.zig | 7 +- test/bake/bake-harness.ts | 286 ++++++++++++++---- test/bake/client-fixture.mjs | 95 ++++-- test/bake/dev/bundle.test.ts | 62 ++++ test/bake/dev/css.test.ts | 6 +- test/bake/dev/ecosystem.test.ts | 26 +- test/bake/dev/esm.test.ts | 57 ++++ test/bake/dev/hot.test.ts | 16 +- test/bake/dev/react-spa.test.ts | 2 +- test/bake/dev/sourcemap.test.ts | 4 + .../pages/_Counter.svelte | 2 +- test/bundler/bundler_drop.test.ts | 8 + 28 files changed, 875 insertions(+), 378 deletions(-) create mode 100644 src/bake/debug.ts diff --git a/packages/bun-types/devserver.d.ts b/packages/bun-types/devserver.d.ts index cfb0ebe9eb..53df578834 100644 --- a/packages/bun-types/devserver.d.ts +++ b/packages/bun-types/devserver.d.ts @@ -3,6 +3,7 @@ export {}; declare global { namespace Bun { type HMREventNames = + | "bun:ready" | "bun:beforeUpdate" | "bun:afterUpdate" | "bun:beforeFullReload" @@ -49,8 +50,6 @@ declare global { * * In production, `data` is inlined to be `{}`. This is handy because Bun * knows it can minify `{}.prop ??= value` into `value` in production. - * - * */ data: any; diff --git a/src/bake/DevServer.zig b/src/bake/DevServer.zig index afcea6413b..392f2b760b 100644 --- a/src/bake/DevServer.zig +++ b/src/bake/DevServer.zig @@ -100,6 +100,11 @@ server_register_update_callback: JSC.Strong, bun_watcher: *bun.Watcher, directory_watchers: DirectoryWatchStore, watcher_atomics: WatcherAtomics, +testing_batch_events: union(enum) { + disabled, + enable_after_bundle, + enabled: TestingBatch, +}, /// Number of bundles that have been executed. This is currently not read, but /// will be used later to determine when to invoke graph garbage collection. @@ -166,6 +171,9 @@ deferred_request_pool: bun.HiveArray(DeferredRequest.Node, DeferredRequest.max_p /// UWS can handle closing the websocket connections themselves active_websocket_connections: std.AutoHashMapUnmanaged(*HmrSocket, void), +relative_path_buf_lock: bun.DebugThreadLock, +relative_path_buf: bun.PathBuffer, + // Debugging dump_dir: if (bun.FeatureFlags.bake_debugging_features) ?std.fs.Dir else void, @@ -410,6 +418,8 @@ pub fn init(options: Options) bun.JSOOM!*DevServer { true else bun.getRuntimeFeatureFlag("BUN_ASSUME_PERFECT_INCREMENTAL"), + .relative_path_buf_lock = .unlocked, + .testing_batch_events = .disabled, .server_transpiler = undefined, .client_transpiler = undefined, @@ -420,6 +430,7 @@ pub fn init(options: Options) bun.JSOOM!*DevServer { .watcher_atomics = undefined, .log = undefined, .deferred_request_pool = undefined, + .relative_path_buf = undefined, }); errdefer bun.destroy(dev); const allocator = dev.allocation_scope.allocator(); @@ -646,6 +657,8 @@ pub fn deinit(dev: *DevServer) void { .framework = {}, .bundler_options = {}, .assume_perfect_incremental_bundling = {}, + .relative_path_buf = {}, + .relative_path_buf_lock = {}, .graph_safety_lock = dev.graph_safety_lock.lock(), .bun_watcher = dev.bun_watcher.deinit(true), @@ -740,6 +753,13 @@ pub fn deinit(dev: *DevServer) void { event.aligned.files.deinit(dev.allocator); event.aligned.extra_files.deinit(dev.allocator); }, + .testing_batch_events = switch (dev.testing_batch_events) { + .disabled => {}, + .enabled => |*batch| { + batch.entry_points.deinit(allocator); + }, + .enable_after_bundle => {}, + }, }; dev.allocation_scope.deinit(); bun.destroy(dev); @@ -776,6 +796,8 @@ pub fn memoryCost(dev: *DevServer) usize { .server_register_update_callback = {}, .deferred_request_pool = {}, .assume_perfect_incremental_bundling = {}, + .relative_path_buf = {}, + .relative_path_buf_lock = {}, // pointers that are not considered a part of DevServer .vm = {}, @@ -887,6 +909,13 @@ pub fn memoryCost(dev: *DevServer) usize { .route_lookup = { cost += memoryCostArrayHashMap(dev.route_lookup); }, + .testing_batch_events = switch (dev.testing_batch_events) { + .disabled => {}, + .enabled => |batch| { + cost += memoryCostArrayHashMap(batch.entry_points.set); + }, + .enable_after_bundle => {}, + }, }; return cost; } @@ -1360,6 +1389,7 @@ fn onFrameworkRequestWithBundle( router_type.server_file_string.get() orelse str: { const name = dev.server_graph.bundled_files.keys()[fromOpaqueFileId(.server, router_type.server_file).get()]; const str = bun.String.createUTF8ForJS(dev.vm.global, dev.relativePath(name)); + dev.releaseRelativePathBuf(); router_type.server_file_string = JSC.Strong.create(str, dev.vm.global); break :str str; }, @@ -1377,10 +1407,12 @@ fn onFrameworkRequestWithBundle( route = dev.router.routePtr(bundle.route_index); var route_name = bun.String.createUTF8(dev.relativePath(keys[fromOpaqueFileId(.server, route.file_page.unwrap().?).get()])); arr.putIndex(global, 0, route_name.transferToJS(global)); + dev.releaseRelativePathBuf(); n = 1; while (true) { if (route.file_layout.unwrap()) |layout| { var layout_name = bun.String.createUTF8(dev.relativePath(keys[fromOpaqueFileId(.server, layout).get()])); + defer dev.releaseRelativePathBuf(); arr.putIndex(global, @intCast(n), layout_name.transferToJS(global)); n += 1; } @@ -1861,7 +1893,7 @@ fn generateClientBundle(dev: *DevServer, route_bundle: *RouteBundle) bun.OOM![]u break :brk; if (!dev.client_graph.stale_files.isSet(rfr_index.get())) { try dev.client_graph.traceImports(rfr_index, >s, .find_client_modules); - react_fast_refresh_id = dev.relativePath(rfr.import_source); + react_fast_refresh_id = rfr.import_source; } } @@ -1884,7 +1916,7 @@ fn generateClientBundle(dev: *DevServer, route_bundle: *RouteBundle) bun.OOM![]u const client_bundle = dev.client_graph.takeJSBundle(.{ .kind = .initial_response, .initial_response_entry_point = if (client_file) |index| - dev.relativePath(dev.client_graph.bundled_files.keys()[index.get()]) + dev.client_graph.bundled_files.keys()[index.get()] else "", .react_refresh_entry_point = react_fast_refresh_id, @@ -1964,6 +1996,7 @@ fn makeArrayForServerComponentsPatch(dev: *DevServer, global: *JSC.JSGlobalObjec const names = dev.server_graph.bundled_files.keys(); for (items, 0..) |item, i| { const str = bun.String.createUTF8(dev.relativePath(names[item.get()])); + defer dev.releaseRelativePathBuf(); defer str.deref(); arr.putIndex(global, @intCast(i), str.toJS(global)); } @@ -2019,6 +2052,7 @@ pub fn finalizeBundle( bv2: *bun.bundle_v2.BundleV2, result: bun.bundle_v2.DevServerOutput, ) bun.OOM!void { + var had_sent_hmr_event = false; defer { bv2.deinit(); dev.current_bundle = null; @@ -2027,6 +2061,20 @@ pub fn finalizeBundle( // not fatal: the assets may be reindexed some time later. }; + // Signal for testing framework where it is in synchronization + if (dev.testing_batch_events == .enable_after_bundle) { + dev.testing_batch_events = .{ .enabled = .empty }; + dev.publish(.testing_watch_synchronization, &.{ + MessageId.testing_watch_synchronization.char(), + 0, + }, .binary); + } else { + dev.publish(.testing_watch_synchronization, &.{ + MessageId.testing_watch_synchronization.char(), + if (had_sent_hmr_event) 4 else 3, + }, .binary); + } + dev.startNextBundleIfPresent(); // Unref the ref added in `startAsyncBundle` @@ -2252,6 +2300,11 @@ pub fn finalizeBundle( } // Index all failed files now that the incremental graph has been updated. + if (dev.incremental_result.failures_removed.items.len > 0 or + dev.incremental_result.failures_added.items.len > 0) + { + had_sent_hmr_event = true; + } try dev.indexFailures(); try dev.client_graph.ensureStaleBitCapacity(false); @@ -2459,10 +2512,10 @@ pub fn finalizeBundle( } try w.writeInt(i32, -1, .little); - // Send CSS mutations const css_chunks = result.cssChunks(); if (will_hear_hot_update) { if (dev.client_graph.current_chunk_len > 0 or css_chunks.len > 0) { + // Send CSS mutations const asset_values = dev.assets.files.values(); try w.writeInt(u32, @intCast(css_chunks.len), .little); const sources = bv2.graph.input_files.items(.source); @@ -2474,6 +2527,7 @@ pub fn finalizeBundle( try w.writeAll(css_data); } + // Send the JS chunk if (dev.client_graph.current_chunk_len > 0) { const hash = hash: { var source_map_hash: bun.bundle_v2.ContentHasher.Hash = .init(0x4b12); // arbitrarily different seed than what .initial_response uses @@ -2499,6 +2553,7 @@ pub fn finalizeBundle( } dev.publish(.hot_update, hot_update_payload.items, .binary); + had_sent_hmr_event = true; } if (dev.incremental_result.failures_added.items.len > 0) { @@ -2578,6 +2633,7 @@ pub fn finalizeBundle( break :file_name dev.relativePath(abs_path); }, }; + defer dev.releaseRelativePathBuf(); const total_count = bv2.graph.entry_points.items.len; if (file_name) |name| { Output.prettyError(": {s}", .{name}); @@ -2645,7 +2701,9 @@ fn startNextBundleIfPresent(dev: *DevServer) void { dev.appendRouteEntryPointsIfNotStale(&entry_points, temp_alloc, route_bundle_index) catch bun.outOfMemory(); } - dev.startAsyncBundle(entry_points, is_reload, timer) catch bun.outOfMemory(); + if (entry_points.set.count() > 0) { + dev.startAsyncBundle(entry_points, is_reload, timer) catch bun.outOfMemory(); + } dev.next_bundle.route_queue.clearRetainingCapacity(); } @@ -4338,6 +4396,7 @@ pub fn IncrementalGraph(side: bake.Side) type { dev.relativePath(gop.key_ptr.*), log.msgs.items, ); + defer dev.releaseRelativePathBuf(); const fail_gop = try dev.bundling_failures.getOrPut(dev.allocator, failure); try dev.incremental_result.failures_added.append(dev.allocator, failure); if (fail_gop.found_existing) { @@ -4556,6 +4615,7 @@ pub fn IncrementalGraph(side: bake.Side) type { w, .utf8, ); + g.owner().releaseRelativePathBuf(); } else { try w.writeAll("null"); } @@ -4571,6 +4631,7 @@ pub fn IncrementalGraph(side: bake.Side) type { w, .utf8, ); + g.owner().releaseRelativePathBuf(); } try w.writeAll("\n"); }, @@ -4648,21 +4709,32 @@ pub fn IncrementalGraph(side: bake.Side) type { var source_map_strings = std.ArrayList(u8).init(arena); defer source_map_strings.deinit(); + const dev = g.owner(); + dev.relative_path_buf_lock.lock(); + defer dev.relative_path_buf_lock.unlock(); + + const buf = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(buf); + var path_count: usize = 0; for (g.current_chunk_parts.items) |entry| { path_count += 1; try source_map_strings.appendSlice(","); - const path = paths[entry.get()]; + const path = if (Environment.isWindows) + bun.path.pathToPosixBuf(u8, paths[entry.get()], buf) + else + paths[entry.get()]; + if (std.fs.path.isAbsolute(path)) { - const is_windows_drive_path = Environment.isWindows and bun.path.isSepAny(path[0]); + const is_windows_drive_path = Environment.isWindows and path[0] != '/'; try source_map_strings.appendSlice(if (is_windows_drive_path) - "file:///" + "\"file:///" else "\"file://"); if (Environment.isWindows and !is_windows_drive_path) { // UNC namespace -> file://server/share/path.ext bun.strings.percentEncodeWrite( - if (path.len > 2 and bun.path.isSepAny(path[0]) and bun.path.isSepAny(path[1])) + if (path.len > 2 and path[0] == '/' and path[1] == '/') path[2..] else path, // invalid but must not crash @@ -5639,6 +5711,7 @@ fn writeVisualizerMessage(dev: *DevServer, payload: *std.ArrayList(u8)) !void { 0.., ) |k, v, i| { const normalized_key = dev.relativePath(k); + defer dev.releaseRelativePathBuf(); try w.writeInt(u32, @intCast(normalized_key.len), .little); if (k.len == 0) continue; try w.writeAll(normalized_key); @@ -5704,12 +5777,13 @@ pub fn onWebSocketUpgrade( /// Every message is to use `.binary`/`ArrayBuffer` transport mode. The first byte /// indicates a Message ID; see comments on each type for how to interpret the rest. +/// Avoid changing message ID values, as some of these are hard-coded in tests. /// /// This format is only intended for communication via the browser and DevServer. /// Server-side HMR is implemented using a different interface. This API is not /// versioned alongside Bun; breaking changes may occur at any point. /// -/// All integers are sent in little-endian +/// All integers are sent in little-endian. pub const MessageId = enum(u8) { /// Version payload. Sent on connection startup. The client should issue a /// hard-reload when it mismatches with its `config.version`. @@ -5797,13 +5871,15 @@ pub const MessageId = enum(u8) { set_url_response = 'n', /// Used for synchronization in DevServer tests, to identify when a update was /// acknowledged by the watcher but intentionally took no action. - redundant_watch = 'r', + /// - `u8`: See bake-harness.ts WatchSynchronization enum. + testing_watch_synchronization = 'r', pub inline fn char(id: MessageId) u8 { return @intFromEnum(id); } }; +/// Avoid changing message ID values, as some of these are hard-coded in tests. pub const IncomingMessageId = enum(u8) { /// Subscribe to an event channel. Payload is a sequence of chars available /// in HmrTopic. @@ -5811,6 +5887,8 @@ pub const IncomingMessageId = enum(u8) { /// Emitted on client-side navigations. /// Rest of payload is a UTF-8 string. set_url = 'n', + /// Tells the DevServer to batch events together. + testing_batch_events = 'H', /// Invalid data _, @@ -5821,7 +5899,7 @@ const HmrTopic = enum(u8) { errors = 'e', browser_error = 'E', visualizer = 'v', - redundant_watch = 'r', + testing_watch_synchronization = 'r', /// Invalid data _, @@ -5926,6 +6004,44 @@ const HmrSocket = struct { var response: [5]u8 = .{MessageId.set_url_response.char()} ++ std.mem.toBytes(rbi.get()); _ = ws.send(&response, .binary, false, true); }, + .testing_batch_events => switch (s.dev.testing_batch_events) { + .disabled => { + if (s.dev.current_bundle != null) { + s.dev.testing_batch_events = .enable_after_bundle; + } else { + s.dev.testing_batch_events = .{ .enabled = .empty }; + s.dev.publish(.testing_watch_synchronization, &.{ + MessageId.testing_watch_synchronization.char(), + 0, + }, .binary); + } + }, + .enable_after_bundle => { + // do not expose a websocket event that panics a release build + bun.debugAssert(false); + ws.close(); + }, + .enabled => |event_const| { + var event = event_const; + s.dev.testing_batch_events = .disabled; + + if (event.entry_points.set.count() == 0) { + s.dev.publish(.testing_watch_synchronization, &.{ + MessageId.testing_watch_synchronization.char(), + 2, + }, .binary); + return; + } + + s.dev.startAsyncBundle( + event.entry_points, + true, + std.time.Timer.start() catch @panic("timers unsupported"), + ) catch bun.outOfMemory(); + + event.entry_points.deinit(s.dev.allocator); + }, + }, _ => ws.close(), } } @@ -6183,7 +6299,10 @@ pub const HotReloadEvent = struct { bun.fmt.fmtSlice(event.dirs.keys(), ", "), }); - dev.publish(.redundant_watch, &.{MessageId.redundant_watch.char()}, .binary); + dev.publish(.testing_watch_synchronization, &.{ + MessageId.testing_watch_synchronization.char(), + 1, + }, .binary); return; } @@ -6202,6 +6321,7 @@ pub const HotReloadEvent = struct { defer debug.log("HMR Task end", .{}); const dev = first.owner; + if (Environment.isDebug) { assert(first.debug_mutex.tryLock()); assert(first.contention_indicator.load(.seq_cst) == 0); @@ -6214,7 +6334,7 @@ pub const HotReloadEvent = struct { var sfb = std.heap.stackFallback(4096, dev.allocator); const temp_alloc = sfb.get(); - var entry_points: EntryPointList = EntryPointList.empty; + var entry_points: EntryPointList = .empty; defer entry_points.deinit(temp_alloc); first.processFileList(dev, &entry_points, temp_alloc); @@ -6233,6 +6353,19 @@ pub const HotReloadEvent = struct { return; } + switch (dev.testing_batch_events) { + .disabled => {}, + .enabled => |*ev| { + ev.append(dev, entry_points) catch bun.outOfMemory(); + dev.publish(.testing_watch_synchronization, &.{ + MessageId.testing_watch_synchronization.char(), + 1, + }, .binary); + return; + }, + .enable_after_bundle => bun.debugAssert(false), + } + dev.startAsyncBundle( entry_points, true, @@ -6514,6 +6647,8 @@ pub fn onRouterCollisionError(dev: *DevServer, rel_path: []const u8, other_id: O dev.relativePath(dev.server_graph.bundled_files.keys()[fromOpaqueFileId(.server, other_id).get()]), }); Output.flush(); + + dev.releaseRelativePathBuf(); } fn toOpaqueFileId(comptime side: bake.Side, index: IncrementalGraph(side).FileIndex) OpaqueFileId { @@ -6537,7 +6672,9 @@ fn fromOpaqueFileId(comptime side: bake.Side, id: OpaqueFileId) IncrementalGraph } /// Returns posix style path, suitible for URLs and reproducible hashes. -fn relativePath(dev: *const DevServer, path: []const u8) []const u8 { +/// To avoid overwriting memory, this has a lock for the buffer. +fn relativePath(dev: *DevServer, path: []const u8) []const u8 { + dev.relative_path_buf_lock.lock(); bun.assert(dev.root[dev.root.len - 1] != '/'); if (!std.fs.path.isAbsolute(path)) { @@ -6550,15 +6687,20 @@ fn relativePath(dev: *const DevServer, path: []const u8) []const u8 { { return path[dev.root.len + 1 ..]; } - const relative_path_buf = &struct { - threadlocal var buf: bun.PathBuffer = undefined; - }.buf; - const rel = bun.path.relativePlatformBuf(relative_path_buf, dev.root, path, .auto, true); - // @constCast: `rel` is owned by a mutable threadlocal buffer above + + const rel = bun.path.relativePlatformBuf(&dev.relative_path_buf, dev.root, path, .auto, true); + // @constCast: `rel` is owned by a buffer on `dev`, which is mutable bun.path.platformToPosixInPlace(u8, @constCast(rel)); return rel; } +fn releaseRelativePathBuf(dev: *DevServer) void { + dev.relative_path_buf_lock.unlock(); + if (bun.Environment.isDebug) { + dev.relative_path_buf = undefined; + } +} + fn dumpStateDueToCrash(dev: *DevServer) !void { comptime assert(bun.FeatureFlags.bake_debugging_features); @@ -7190,6 +7332,7 @@ const ErrorReportRequest = struct { const abs_path = result.file_paths[@intCast(index - 1)]; frame.source_url = .init(abs_path); const rel_path = ctx.dev.relativePath(abs_path); + defer ctx.dev.releaseRelativePathBuf(); if (bun.strings.eql(frame.function_name.value.ZigString.slice(), rel_path)) { frame.function_name = .empty; } @@ -7279,6 +7422,7 @@ const ErrorReportRequest = struct { const src_to_write = frame.source_url.value.ZigString.slice(); if (bun.strings.hasPrefixComptime(src_to_write, "/")) { const file = ctx.dev.relativePath(src_to_write); + defer ctx.dev.releaseRelativePathBuf(); try w.writeInt(u32, @intCast(file.len), .little); try w.writeAll(file); } else { @@ -7413,6 +7557,19 @@ fn readString32(reader: anytype, alloc: Allocator) ![]const u8 { return memory; } +const TestingBatch = struct { + entry_points: EntryPointList, + + const empty: @This() = .{ .entry_points = .empty }; + + pub fn append(self: *@This(), dev: *DevServer, entry_points: EntryPointList) !void { + assert(entry_points.set.count() > 0); + for (entry_points.set.keys(), entry_points.set.values()) |k, v| { + try self.entry_points.append(dev.allocator, k, v); + } + } +}; + /// userland implementation of https://github.com/ziglang/zig/issues/21879 fn VoidFieldTypes(comptime T: type) type { const fields = @typeInfo(T).@"struct".fields; diff --git a/src/bake/bake.private.d.ts b/src/bake/bake.private.d.ts index fced2377a6..c4b66b1d15 100644 --- a/src/bake/bake.private.d.ts +++ b/src/bake/bake.private.d.ts @@ -26,6 +26,14 @@ interface Config { roots: FileIndex[]; } +declare namespace DEBUG { + /** + * Set globally in debug builds. + * Removed using --drop=DEBUG.ASSERT in releases. + */ + declare function ASSERT(condition: any, message?: string): asserts condition; +} + /** All modules for the initial bundle. */ declare const unloadedModuleRegistry: Record; declare type UnloadedModule = UnloadedESM | UnloadedCommonJS; @@ -40,11 +48,12 @@ declare type EncodedDependencyArray = (string | number)[]; declare type UnloadedCommonJS = ( hmr: import("./hmr-module").HMRModule, module: import("./hmr-module").HMRModule["cjs"], -) => any; + exports: unknown, +) => unknown; declare type CommonJSModule = { id: Id; exports: any; - require: (id: Id) => any; + require: (id: Id) => unknown; }; declare const config: Config; diff --git a/src/bake/client/css-reloader.ts b/src/bake/client/css-reloader.ts index 7fb19976b1..c22530d8de 100644 --- a/src/bake/client/css-reloader.ts +++ b/src/bake/client/css-reloader.ts @@ -180,7 +180,8 @@ export function editCssContent(id: string, newContent: string) { // Disable the link tag if it exists const linkSheet = entry.link?.sheet; if (linkSheet) linkSheet.disabled = true; - return; + return false; } sheet!.replace(newContent); + return !sheet!.disabled; } diff --git a/src/bake/debug.ts b/src/bake/debug.ts new file mode 100644 index 0000000000..64c96497e4 --- /dev/null +++ b/src/bake/debug.ts @@ -0,0 +1,14 @@ +if (IS_BUN_DEVELOPMENT) { + // After 1.2.6 is released, this can just be `ASSERT` + globalThis.DEBUG = { + ASSERT: function ASSERT(condition: any, message?: string): asserts condition { + if (!condition) { + if (typeof Bun === "undefined") { + console.assert(false, "ASSERTION FAILED" + (message ? `: ${message}` : "")); + } else { + console.error("ASSERTION FAILED" + (message ? `: ${message}` : "")); + } + } + }, + }; +} diff --git a/src/bake/hmr-module.ts b/src/bake/hmr-module.ts index 05241ada89..940a209c64 100644 --- a/src/bake/hmr-module.ts +++ b/src/bake/hmr-module.ts @@ -58,6 +58,12 @@ interface HotAccept { single: boolean; } +interface CJSModule { + id: Id; + exports: unknown; + require: (id: Id) => unknown; +} + /** Implementation details must remain in sync with js_parser.zig and bundle_v2.zig */ export class HMRModule { /** Key in `registry` */ @@ -69,10 +75,10 @@ export class HMRModule { exports: any = null; /** For ESM, this is the converted CJS exports. * For CJS, this is the `module` object. */ - cjs: any; + cjs: CJSModule | any | null; /** When a module fails to load, trying to load it again * should throw the same error */ - failure: any = null; + failure: unknown = null; /** Two purposes: * 1. HMRModule[] - List of parsed imports. indexOf is used to go from HMRModule -> updater function * 2. any[] - List of module namespace objects. Read by the ESM module's load function. @@ -129,27 +135,6 @@ export class HMRModule { : import(id); } - /** - * Files which only export functions (and have no other statements) are - * implicitly `import.meta.hot.accept`ed, however it is done in a special way - * where functions are proxied. This is special behavior to make stuff "just - * work". - */ - implicitlyAccept(exports) { - if (IS_BUN_DEVELOPMENT) assert(this.esm); - this.selfAccept ??= implicitAcceptFunction; - const current = ((this.selfAccept as any).current ??= {}); - if (IS_BUN_DEVELOPMENT) assert(typeof exports === "object"); - const moduleExports = (this.exports = {}); - for (const exportName in exports) { - const source = (current[exportName] = exports[exportName]); - if (IS_BUN_DEVELOPMENT) assert(typeof source === "function"); - const proxied = (moduleExports[exportName] ??= proxyFn(current, exportName)); - Object.defineProperty(proxied, "name", { value: source.name }); - Object.defineProperty(proxied, "length", { value: source.length }); - } - } - reactRefreshAccept() { if (isReactRefreshBoundary(this.exports)) { this.accept(); @@ -297,15 +282,24 @@ export function loadModuleSync(id: Id, isUserDynamic: boolean, importer: HMRModu if (!mod) { mod = new HMRModule(id, true); registry.set(id, mod); + } else if (mod.esm) { + mod.esm = false; + mod.cjs = { + id, + exports: {}, + require: mod.require.bind(this), + }; + mod.exports = null; } if (importer) { mod.importers.add(importer); } try { - loadOrEsmModule(mod, mod.cjs); + const cjs = mod.cjs; + loadOrEsmModule(mod, cjs, cjs.exports); } catch (e) { - mod.state = State.Error; - mod.failure = e; + mod.state = State.Stale; + mod.cjs.exports = {}; throw e; } mod.state = State.Loaded; @@ -313,11 +307,11 @@ export function loadModuleSync(id: Id, isUserDynamic: boolean, importer: HMRModu // ESM if (IS_BUN_DEVELOPMENT) { try { - assert(Array.isArray(loadOrEsmModule[ESMProps.imports])); - assert(Array.isArray(loadOrEsmModule[ESMProps.exports])); - assert(Array.isArray(loadOrEsmModule[ESMProps.stars])); - assert(typeof loadOrEsmModule[ESMProps.load] === "function"); - assert(typeof loadOrEsmModule[ESMProps.isAsync] === "boolean"); + DEBUG.ASSERT(Array.isArray(loadOrEsmModule[ESMProps.imports])); + DEBUG.ASSERT(Array.isArray(loadOrEsmModule[ESMProps.exports])); + DEBUG.ASSERT(Array.isArray(loadOrEsmModule[ESMProps.stars])); + DEBUG.ASSERT(typeof loadOrEsmModule[ESMProps.load] === "function"); + DEBUG.ASSERT(typeof loadOrEsmModule[ESMProps.isAsync] === "boolean"); } catch (e) { console.warn(id, loadOrEsmModule); throw e; @@ -330,6 +324,10 @@ export function loadModuleSync(id: Id, isUserDynamic: boolean, importer: HMRModu if (!mod) { mod = new HMRModule(id, false); registry.set(id, mod); + } else if (!mod.esm) { + mod.esm = true; + mod.cjs = null; + mod.exports = null; } if (importer) { mod.importers.add(importer); @@ -384,29 +382,37 @@ export function loadModuleAsync( if (!mod) { mod = new HMRModule(id, true); registry.set(id, mod); + } else if (mod.esm) { + mod.esm = false; + mod.cjs = { + id, + exports: {}, + require: mod.require.bind(this), + }; + mod.exports = null; } if (importer) { mod.importers.add(importer); } try { - loadOrEsmModule(mod, mod.cjs); + const cjs = mod.cjs; + loadOrEsmModule(mod, cjs, cjs.exports); } catch (e) { - mod.state = State.Error; - mod.failure = e; + mod.state = State.Stale; + mod.cjs.exports = {}; throw e; } mod.state = State.Loaded; - return mod; } else { // ESM if (IS_BUN_DEVELOPMENT) { try { - assert(Array.isArray(loadOrEsmModule[0])); - assert(Array.isArray(loadOrEsmModule[1])); - assert(Array.isArray(loadOrEsmModule[2])); - assert(typeof loadOrEsmModule[3] === "function"); - assert(typeof loadOrEsmModule[4] === "boolean"); + DEBUG.ASSERT(Array.isArray(loadOrEsmModule[0])); + DEBUG.ASSERT(Array.isArray(loadOrEsmModule[1])); + DEBUG.ASSERT(Array.isArray(loadOrEsmModule[2])); + DEBUG.ASSERT(typeof loadOrEsmModule[3] === "function"); + DEBUG.ASSERT(typeof loadOrEsmModule[4] === "boolean"); } catch (e) { console.warn(id, loadOrEsmModule); throw e; @@ -417,19 +423,21 @@ export function loadModuleAsync( if (!mod) { mod = new HMRModule(id, false); registry.set(id, mod); + } else if (!mod.esm) { + mod.esm = true; + mod.exports = null; + mod.cjs = null; } if (importer) { mod.importers.add(importer); } const { list, isAsync } = parseEsmDependencies(mod, deps, loadModuleAsync); - if (IS_BUN_DEVELOPMENT) { - if (isAsync) { - assert(list.some(x => x instanceof Promise)); - } else { - assert(list.every(x => x instanceof HMRModule)); - } - } + DEBUG.ASSERT( + isAsync // + ? list.some(x => x instanceof Promise) + : list.every(x => x instanceof HMRModule) + ); // Running finishLoadModuleAsync synchronously when there are no promises is // not a performance optimization but a behavioral correctness issue. @@ -481,7 +489,7 @@ function finishLoadModuleAsync(mod: HMRModule, load: UnloadedESM[3], modules: HM type GenericModuleLoader = (id: Id, isUserDynamic: false, importer: HMRModule) => R; // TODO: This function is currently recursive. function parseEsmDependencies>( - mod: HMRModule, + parent: HMRModule, deps: (string | number)[], enqueueModuleLoad: T, ) { @@ -491,10 +499,11 @@ function parseEsmDependencies>( const { length } = deps; while (i < length) { const dep = deps[i] as string; - if (IS_BUN_DEVELOPMENT) assert(typeof dep === "string"); + DEBUG.ASSERT(typeof dep === "string"); let expectedExportKeyEnd = i + 2 + (deps[i + 1] as number); - if (IS_BUN_DEVELOPMENT) assert(typeof deps[i + 1] === "number"); - list.push(enqueueModuleLoad(dep, false, mod)); + DEBUG.ASSERT(typeof deps[i + 1] === "number"); + const promiseOrModule = enqueueModuleLoad(dep, false, parent); + list.push(promiseOrModule); const unloadedModule = unloadedModuleRegistry[dep]; if (!unloadedModule) { @@ -505,18 +514,27 @@ function parseEsmDependencies>( i += 2; while (i < expectedExportKeyEnd) { const key = deps[i] as string; - if (IS_BUN_DEVELOPMENT) assert(typeof key === "string"); - if (!availableExportKeys.includes(key)) { - if (!hasExportStar(unloadedModule[ESMProps.stars], key)) { - throw new SyntaxError(`Module "${dep}" does not export key "${key}"`); - } - } + DEBUG.ASSERT(typeof key === "string"); + // TODO: there is a bug in the way exports are verified. Additionally a + // possible performance issue. For the meantime, this is disabled since + // it was not shipped in the initial 1.2.3 HMR, and real issues will + // just throw 'undefined is not a function' or so on. + + // if (!availableExportKeys.includes(key)) { + // if (!hasExportStar(unloadedModule[ESMProps.stars], key)) { + // throw new SyntaxError(`Module "${dep}" does not export key "${key}"`); + // } + // } i++; } - isAsync ||= unloadedModule[ESMProps.isAsync]; + isAsync ||= promiseOrModule instanceof Promise; } else { - if (IS_BUN_DEVELOPMENT) assert(!registry.get(dep)?.esm); + DEBUG.ASSERT(!registry.get(dep)?.esm); i = expectedExportKeyEnd; + + if (IS_BUN_DEVELOPMENT) { + DEBUG.ASSERT(list[list.length - 1] as any instanceof HMRModule); + } } } return { list, isAsync }; @@ -531,7 +549,7 @@ function hasExportStar(starImports: Id[], key: string) { if (visited.has(starImport)) continue; visited.add(starImport); const mod = unloadedModuleRegistry[starImport]; - if (IS_BUN_DEVELOPMENT) assert(mod, `Module "${starImport}" not found`); + DEBUG.ASSERT(mod, `Module "${starImport}" not found`); if (typeof mod === "function") { return true; } @@ -562,6 +580,7 @@ type HotEventHandler = (data: any) => void; // If updating this, make sure the `devserver.d.ts` types are // kept in sync. type HMREvent = + | "bun:ready" | "bun:beforeUpdate" | "bun:afterUpdate" | "bun:beforeFullReload" @@ -651,11 +670,9 @@ export async function replaceModules(modules: Record) { for (const boundary of failures) { const path: Id[] = []; let current = registry.get(boundary)!; - if (IS_BUN_DEVELOPMENT) { - assert(!boundary.endsWith(".html")); // caller should have already reloaded - assert(current); - assert(current.selfAccept === null); - } + DEBUG.ASSERT(!boundary.endsWith(".html")); // caller should have already reloaded + DEBUG.ASSERT(current); + DEBUG.ASSERT(current.selfAccept === null); if (current.importers.size === 0) { message += `Module "${boundary}" is a root module that does not self-accept.\n`; continue; @@ -668,13 +685,11 @@ export async function replaceModules(modules: Record) { current = importer; continue outer; } - if (IS_BUN_DEVELOPMENT) assert(false); + DEBUG.ASSERT(false); break; } path.push(current.id); - if (IS_BUN_DEVELOPMENT) { - assert(path.length > 0); - } + DEBUG.ASSERT(path.length > 0); message += `Module "${boundary}" is not accepted by ${path[1]}${path.length > 1 ? "," : "."}\n`; for (let i = 2, len = path.length; i < len; i++) { const isLast = i === len - 1; @@ -728,7 +743,7 @@ export async function replaceModules(modules: Record) { selfAccept(getEsmExports(mod)); } } else { - if (IS_BUN_DEVELOPMENT) assert(modOrPromise instanceof Promise); + DEBUG.ASSERT(modOrPromise instanceof Promise); promises.push( (modOrPromise as Promise).then(mod => { if (selfAccept) { @@ -776,7 +791,7 @@ function createAcceptArray(modules: string[], key: Id) { const arr = new Array(modules.length); arr.fill(undefined); const i = modules.indexOf(key); - if (IS_BUN_DEVELOPMENT) assert(i !== -1); + DEBUG.ASSERT(i !== -1); arr[i] = getEsmExports(registry.get(key)!); return arr; } @@ -852,12 +867,6 @@ function registerSynthetic(id: Id, esmExports) { registry.set(id, module); } -function assert(condition: any, message?: string): asserts condition { - if (!condition) { - console.assert(false, "ASSERTION FAILED" + (message ? `: ${message}` : "")); - } -} - export function setRefreshRuntime(runtime: HMRModule) { refreshRuntime = getEsmExports(runtime); @@ -902,14 +911,6 @@ function isReactRefreshBoundary(esmExports): boolean { function implicitAcceptFunction() {} -const apply = Function.prototype.apply; -function proxyFn(target: any, key: string) { - const f = function () { - return apply.call(target[key], this, arguments); - }; - return f; -} - declare global { interface Error { asyncId?: string; @@ -940,3 +941,13 @@ if (side === "client") { onServerSideReload: cb => (onServerSideReload = cb), }); } + +// The following API may be altered at any point. +// Thankfully, you can just call `import.meta.hot.on` +let testingHook = globalThis['bun do not use this outside of internal testing or else i\'ll cry']; +testingHook?.({ + onEvent(event: HMREvent, cb) { + eventHandlers[event] ??= []; + eventHandlers[event]!.push(cb); + }, +}); \ No newline at end of file diff --git a/src/bake/hmr-runtime-client.ts b/src/bake/hmr-runtime-client.ts index d5ce1b4113..804d0aecf6 100644 --- a/src/bake/hmr-runtime-client.ts +++ b/src/bake/hmr-runtime-client.ts @@ -1,5 +1,6 @@ // This file is the entrypoint to the hot-module-reloading runtime // In the browser, this uses a WebSocket to communicate with the bundler. +import './debug'; import { loadModuleAsync, replaceModules, @@ -153,8 +154,12 @@ const handlers = { onRuntimeError(e, true, false); return; } + emitEvent("bun:error", e); throw e; } + } else { + // Needed for testing. + emitEvent("bun:afterUpdate", null); } }, [MessageId.set_url_response](view) { @@ -216,6 +221,8 @@ try { } await loadModuleAsync(config.main, false, null); + + emitEvent("bun:ready", null); } catch (e) { console.error(e); onRuntimeError(e, true, false); diff --git a/src/bake/hmr-runtime-error.ts b/src/bake/hmr-runtime-error.ts index ab0de38fac..20957a10c9 100644 --- a/src/bake/hmr-runtime-error.ts +++ b/src/bake/hmr-runtime-error.ts @@ -6,6 +6,7 @@ // This is embedded in `DevServer.sendSerializedFailures`. SSR is // left unused for simplicity; a flash of unstyled content is // stopped by the fact this script runs synchronously. +import './debug'; import { decodeAndAppendServerError, onServerErrorPayload, updateErrorOverlay } from "./client/overlay"; import { DataViewReader } from "./client/data-view"; import { initWebSocket } from "./client/websocket"; diff --git a/src/bake/hmr-runtime-server.ts b/src/bake/hmr-runtime-server.ts index eae7d974ec..cd76fea987 100644 --- a/src/bake/hmr-runtime-server.ts +++ b/src/bake/hmr-runtime-server.ts @@ -1,5 +1,6 @@ // This file is the entrypoint to the hot-module-reloading runtime. // On the server, communication is established with `server_exports`. +import './debug'; import type { Bake } from "bun"; import { loadExports, replaceModules, ssrManifest, serverManifest, HMRModule } from "./hmr-module"; diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index 36e3049e80..3521dab350 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -8706,7 +8706,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp } const result: JSValue = onNodeHTTPRequestFn( - @bitCast(AnyServer.from(this)), + @intFromPtr(AnyServer.from(this).ptr.ptr()), globalThis, thisObject, this.config.onNodeHTTPRequest, @@ -9610,7 +9610,7 @@ pub const HTTPServer = NewServer(JSC.Codegen.JSHTTPServer, false, false); pub const HTTPSServer = NewServer(JSC.Codegen.JSHTTPSServer, true, false); pub const DebugHTTPServer = NewServer(JSC.Codegen.JSDebugHTTPServer, false, true); pub const DebugHTTPSServer = NewServer(JSC.Codegen.JSDebugHTTPSServer, true, true); -pub const AnyServer = packed struct { +pub const AnyServer = struct { ptr: Ptr, const Ptr = bun.TaggedPointerUnion(.{ @@ -9862,7 +9862,7 @@ comptime { } extern fn NodeHTTPServer__onRequest_http( - any_server: u64, + any_server: usize, globalThis: *JSC.JSGlobalObject, this: JSC.JSValue, callback: JSC.JSValue, @@ -9873,7 +9873,7 @@ extern fn NodeHTTPServer__onRequest_http( ) JSC.JSValue; extern fn NodeHTTPServer__onRequest_https( - any_server: u64, + any_server: usize, globalThis: *JSC.JSGlobalObject, this: JSC.JSValue, callback: JSC.JSValue, diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig index ee7951011c..5afdd4a7d7 100644 --- a/src/bundler/bundle_v2.zig +++ b/src/bundler/bundle_v2.zig @@ -3207,16 +3207,22 @@ pub const BundleV2 = struct { ) catch bun.outOfMemory(); } } else { + const buf = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(buf); + const specifier_to_use = if (loader == .html and bun.strings.hasPrefix(import_record.path.text, bun.fs.FileSystem.instance.top_level_dir)) brk: { + const specifier_to_use = import_record.path.text[bun.fs.FileSystem.instance.top_level_dir.len..]; + if (Environment.isWindows) { + break :brk bun.path.pathToPosixBuf(u8, specifier_to_use, buf); + } + break :brk specifier_to_use; + } else import_record.path.text; addError( log, source, import_record.range, this.graph.allocator, "Could not resolve: \"{s}\"", - .{if (loader == .html and bun.strings.hasPrefix(import_record.path.text, bun.fs.FileSystem.instance.top_level_dir)) - import_record.path.text[bun.fs.FileSystem.instance.top_level_dir.len..] - else - import_record.path.text}, + .{specifier_to_use}, import_record.kind, ) catch bun.outOfMemory(); } @@ -13790,33 +13796,18 @@ pub const LinkerContext = struct { }) catch unreachable; // is within bounds if (ast.flags.uses_module_ref or ast.flags.uses_exports_ref) { - clousure_args.appendAssumeCapacity( + clousure_args.appendSliceAssumeCapacity(&.{ .{ .binding = Binding.alloc(temp_allocator, B.Identifier{ .ref = ast.module_ref, }, Logger.Loc.Empty), - .default = Expr.allocate(temp_allocator, E.Dot, .{ - .target = Expr.initIdentifier(hmr_api_ref, Logger.Loc.Empty), - .name = "cjs", - .name_loc = Logger.Loc.Empty, - }, Logger.Loc.Empty), }, - ); - } - - if (ast.flags.uses_exports_ref) { - clousure_args.appendAssumeCapacity( .{ .binding = Binding.alloc(temp_allocator, B.Identifier{ .ref = ast.exports_ref, }, Logger.Loc.Empty), - .default = Expr.allocate(temp_allocator, E.Dot, .{ - .target = Expr.initIdentifier(ast.module_ref, Logger.Loc.Empty), - .name = "exports", - .name_loc = Logger.Loc.Empty, - }, Logger.Loc.Empty), }, - ); + }); } stmts.all_stmts.appendAssumeCapacity(Stmt.allocateExpr(temp_allocator, Expr.init(E.Function, .{ .func = .{ @@ -17764,7 +17755,6 @@ pub const AstBuilder = struct { try p.symbols.append(p.allocator, .{ .kind = kind, .original_name = identifier, - .debug_mode_source_index = if (Environment.allow_assert) @intCast(p.source_index) else 0, }); const ref: Ref = .{ .inner_index = inner_index, diff --git a/src/codegen/bake-codegen.ts b/src/codegen/bake-codegen.ts index ecb90c8652..c3da5d97bf 100644 --- a/src/codegen/bake-codegen.ts +++ b/src/codegen/bake-codegen.ts @@ -59,6 +59,7 @@ async function run() { syntax: !debug, }, target: side === "server" ? "bun" : "browser", + drop: debug ? [] : ["DEBUG"], }); if (!result.success) throw new AggregateError(result.logs); assert(result.outputs.length === 1, "must bundle to a single file"); @@ -90,6 +91,7 @@ async function run() { result = await Bun.build({ entrypoints: [generated_entrypoint], minify: !debug, + drop: debug ? [] : ["DEBUG"], }); if (!result.success) throw new AggregateError(result.logs); assert(result.outputs.length === 1, "must bundle to a single file"); @@ -131,7 +133,7 @@ async function run() { : `${code};return ${outName("server_exports")};`; const params = `${outName("$separateSSRGraph")},${outName("$importMeta")}`; - code = code.replaceAll("import.meta", outName("$importMeta")); + code = code.replaceAll("import.meta", outName("$importMeta")).replaceAll(outName("$importMeta") + ".hot", "import.meta.hot"); code = `let ${outName("unloadedModuleRegistry")}={},${outName("config")}={separateSSRGraph:${outName("$separateSSRGraph")}},${outName("server_exports")};${code}`; code = debug ? `((${params}) => {${code}})\n` : `((${params})=>{${code}})\n`; diff --git a/src/js_ast.zig b/src/js_ast.zig index c5a5a6d735..02d40cc204 100644 --- a/src/js_ast.zig +++ b/src/js_ast.zig @@ -969,7 +969,7 @@ pub const Symbol = struct { /// This is the name that came from the parser. Printed names may be renamed /// during minification or to avoid name collisions. Do not use the original /// name during printing. - original_name: string, + original_name: []const u8, /// This is used for symbols that represent items in the import clause of an /// ES6 import statement. These should always be referenced by EImportIdentifier @@ -1103,15 +1103,13 @@ pub const Symbol = struct { remove_overwritten_function_declaration: bool = false, - /// In debug mode, sometimes its helpful to know what source file - /// A symbol came from. This is used for that. - /// - /// We don't want this in non-debug mode because it increases the size of - /// the symbol table. - debug_mode_source_index: if (Environment.allow_assert) - Index.Int - else - u0 = 0, + /// Used in HMR to decide when live binding code is needed. + has_been_assigned_to: bool = false, + + comptime { + bun.assert_eql(@sizeOf(Symbol), 88); + bun.assert_eql(@alignOf(Symbol), @alignOf([]const u8)); + } const invalid_chunk_index = std.math.maxInt(u32); pub const invalid_nested_scope_slot = std.math.maxInt(u32); diff --git a/src/js_parser.zig b/src/js_parser.zig index 4e6f6a420a..f2bfc9ba87 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -9289,7 +9289,6 @@ fn NewParser_( try p.symbols.append(Symbol{ .kind = kind, .original_name = identifier, - .debug_mode_source_index = if (comptime Environment.allow_assert) p.source.index.get() else 0, }); if (is_typescript_enabled) { @@ -16232,6 +16231,8 @@ fn NewParser_( // `module.exports` -> `exports` optimization. p.commonjs_module_exports_assigned_deoptimized = true; } + + p.symbols.items[result.ref.innerIndex()].has_been_assigned_to = true; } var original_name: ?string = null; @@ -17318,7 +17319,7 @@ fn NewParser_( p.method_call_must_be_replaced_with_undefined = false; switch (e_.target.data) { // If we're removing this call, don't count any arguments as symbol uses - .e_index, .e_dot => { + .e_index, .e_dot, .e_identifier => { p.is_control_flow_dead = true; }, // Special case from `import.meta.hot.*` functions. @@ -18341,13 +18342,6 @@ fn NewParser_( } fn selectLocalKind(p: *P, kind: S.Local.Kind) S.Local.Kind { - // When using Kit's HMR implementation, we need to preserve the local kind - // if possible, as more efficient code can be generated if something is known - // not to be an ESM live binding. - if (p.options.features.hot_module_reloading) { - return kind; - } - // Use "var" instead of "let" and "const" if the variable declaration may // need to be separated from the initializer. This allows us to safely move // this declaration into a nested scope. @@ -24322,7 +24316,6 @@ pub const ConvertESMExportsForHmr = struct { export_star_props: std.ArrayListUnmanaged(G.Property) = .{}, export_props: std.ArrayListUnmanaged(G.Property) = .{}, stmts: std.ArrayListUnmanaged(Stmt) = .{}, - can_implicitly_accept: bool = true, const ImportRef = struct { /// Index into ConvertESMExportsForHmr.stmts @@ -24332,66 +24325,56 @@ pub const ConvertESMExportsForHmr = struct { fn convertStmt(ctx: *ConvertESMExportsForHmr, p: anytype, stmt: Stmt) !void { const new_stmt = switch (stmt.data) { else => brk: { - ctx.can_implicitly_accept = false; break :brk stmt; }, .s_local => |st| stmt: { if (!st.is_export) { - ctx.can_implicitly_accept = false; break :stmt stmt; } st.is_export = false; - if (st.kind.isReassignable()) { - ctx.can_implicitly_accept = false; - for (st.decls.slice()) |decl| { - try ctx.visitBindingToExport(p, decl.binding, true); - } - } else { - var new_len: usize = 0; - for (st.decls.slice()) |*decl_ptr| { - const decl = decl_ptr.*; // explicit copy to avoid aliasinng - bun.assert(decl.value != null); // const must be initialized + var new_len: usize = 0; + for (st.decls.slice()) |*decl_ptr| { + const decl = decl_ptr.*; // explicit copy to avoid aliasinng + const value = decl.value orelse { + st.decls.mut(new_len).* = decl; + new_len += 1; + try ctx.visitBindingToExport(p, decl.binding); + continue; + }; - switch (decl.binding.data) { - .b_missing => {}, + switch (decl.binding.data) { + .b_missing => {}, - .b_identifier => |id| { - const symbol = p.symbols.items[id.ref.inner_index]; + .b_identifier => |id| { + const symbol = p.symbols.items[id.ref.inner_index]; - if (ctx.can_implicitly_accept) switch (decl.value.?.data) { - .e_function, .e_arrow => {}, - else => ctx.can_implicitly_accept = false, - }; - - // if the symbol is not used, we don't need to preserve - // a binding in this scope. we can move it to the exports object. - if (symbol.use_count_estimate == 0 and decl.value.?.canBeMoved()) { - try ctx.export_props.append(p.allocator, .{ - .key = Expr.init(E.String, .{ .data = symbol.original_name }, decl.binding.loc), - .value = decl.value, - }); - } else { - st.decls.mut(new_len).* = decl; - new_len += 1; - try ctx.visitBindingToExport(p, decl.binding, false); - } - }, - - else => { - ctx.can_implicitly_accept = false; + // if the symbol is not used, we don't need to preserve + // a binding in this scope. we can move it to the exports object. + if (symbol.use_count_estimate == 0 and value.canBeMoved()) { + try ctx.export_props.append(p.allocator, .{ + .key = Expr.init(E.String, .{ .data = symbol.original_name }, decl.binding.loc), + .value = value, + }); + } else { st.decls.mut(new_len).* = decl; new_len += 1; - try ctx.visitBindingToExport(p, decl.binding, false); - }, - } + try ctx.visitBindingToExport(p, decl.binding); + } + }, + + else => { + st.decls.mut(new_len).* = decl; + new_len += 1; + try ctx.visitBindingToExport(p, decl.binding); + }, } - if (new_len == 0) { - return; - } - st.decls.len = @intCast(new_len); } + if (new_len == 0) { + return; + } + st.decls.len = @intCast(new_len); break :stmt stmt; }, @@ -24415,11 +24398,6 @@ pub const ConvertESMExportsForHmr = struct { // All other functions can be properly moved. } - if (ctx.can_implicitly_accept and - !((st.value == .stmt and st.value.stmt.data == .s_function) or - (st.value == .expr and st.value.expr.data == .e_arrow))) - ctx.can_implicitly_accept = false; - // Try to move the export default expression to the end. const can_be_moved_to_inner_scope = switch (st.value) { .stmt => |s| switch (s.data) { @@ -24484,7 +24462,6 @@ pub const ConvertESMExportsForHmr = struct { } }, .s_class => |st| stmt: { - ctx.can_implicitly_accept = false; // Strip the "export" keyword if (!st.is_export) { @@ -24509,13 +24486,13 @@ pub const ConvertESMExportsForHmr = struct { st.func.flags.remove(.is_export); - // Export as CommonJS - try ctx.export_props.append(p.allocator, .{ - .key = Expr.init(E.String, .{ - .data = p.symbols.items[st.func.name.?.ref.?.inner_index].original_name, - }, stmt.loc), - .value = Expr.initIdentifier(st.func.name.?.ref.?, stmt.loc), - }); + try ctx.visitRefToExport( + p, + st.func.name.?.ref.?, + null, + stmt.loc, + false, + ); break :stmt stmt; }, @@ -24523,21 +24500,11 @@ pub const ConvertESMExportsForHmr = struct { for (st.items) |item| { const ref = item.name.ref.?; try ctx.visitRefToExport(p, ref, item.alias, item.name.loc, false); - - if (ctx.can_implicitly_accept) { - const symbol: *Symbol = &p.symbols.items[ref.inner_index]; - switch (symbol.kind) { - .hoisted_function, .generator_or_async_function => {}, - else => ctx.can_implicitly_accept = false, - } - } } return; // do not emit a statement here }, .s_export_from => |st| { - ctx.can_implicitly_accept = false; - const namespace_ref = try ctx.deduplicatedImport( p, st.import_record_index, @@ -24577,8 +24544,6 @@ pub const ConvertESMExportsForHmr = struct { return; }, .s_export_star => |st| { - ctx.can_implicitly_accept = false; - const namespace_ref = try ctx.deduplicatedImport( p, st.import_record_index, @@ -24688,25 +24653,20 @@ pub const ConvertESMExportsForHmr = struct { return namespace_ref; } - fn visitBindingToExport( - ctx: *ConvertESMExportsForHmr, - p: anytype, - binding: Binding, - is_live_binding: bool, - ) !void { + fn visitBindingToExport(ctx: *ConvertESMExportsForHmr, p: anytype, binding: Binding) !void { switch (binding.data) { .b_missing => {}, .b_identifier => |id| { - try ctx.visitRefToExport(p, id.ref, null, binding.loc, is_live_binding); + try ctx.visitRefToExport(p, id.ref, null, binding.loc, false); }, .b_array => |array| { for (array.items) |item| { - try ctx.visitBindingToExport(p, item.binding, is_live_binding); + try ctx.visitBindingToExport(p, item.binding); } }, .b_object => |object| { for (object.properties) |item| { - try ctx.visitBindingToExport(p, item.value, is_live_binding); + try ctx.visitBindingToExport(p, item.value); } }, } @@ -24725,9 +24685,7 @@ pub const ConvertESMExportsForHmr = struct { Expr.init(E.ImportIdentifier, .{ .ref = ref }, loc) else Expr.initIdentifier(ref, loc); - if (is_live_binding_source or (symbol.kind == .import and !ctx.is_in_node_modules)) { - ctx.can_implicitly_accept = false; - + if (is_live_binding_source or (symbol.kind == .import and !ctx.is_in_node_modules) or symbol.has_been_assigned_to) { // TODO (2024-11-24) instead of requiring getters for live-bindings, // a callback propagation system should be considered. mostly // because here, these might not even be live bindings, and @@ -24749,7 +24707,6 @@ pub const ConvertESMExportsForHmr = struct { try ctx.last_part.symbol_uses.putNoClobber(p.allocator, arg1, .{ .count_estimate = 1 }); try p.current_scope.generated.push(p.allocator, arg1); - // Live bindings need to update the value internally and externally. // 'get abc() { return abc }' try ctx.export_props.append(p.allocator, .{ .kind = .get, @@ -24794,38 +24751,24 @@ pub const ConvertESMExportsForHmr = struct { .properties = G.Property.List.fromList(ctx.export_props), }, logger.Loc.Empty); - if (ctx.can_implicitly_accept) { - // `hmr.implicitlyAccept(...)` - try ctx.stmts.append(p.allocator, Stmt.alloc(S.SExpr, .{ - .value = Expr.init(E.Call, .{ - .target = Expr.init(E.Dot, .{ - .target = Expr.initIdentifier(p.hmr_api_ref, logger.Loc.Empty), - .name = "implicitlyAccept", - .name_loc = logger.Loc.Empty, - }, logger.Loc.Empty), - .args = try .fromSlice(p.allocator, &.{obj}), + // `hmr.exports = ...` + try ctx.stmts.append(p.allocator, Stmt.alloc(S.SExpr, .{ + .value = Expr.assign( + Expr.init(E.Dot, .{ + .target = Expr.initIdentifier(p.hmr_api_ref, logger.Loc.Empty), + .name = "exports", + .name_loc = logger.Loc.Empty, }, logger.Loc.Empty), - }, logger.Loc.Empty)); - } else { - // `hmr.exports = ...` - try ctx.stmts.append(p.allocator, Stmt.alloc(S.SExpr, .{ - .value = Expr.assign( - Expr.init(E.Dot, .{ - .target = Expr.initIdentifier(p.hmr_api_ref, logger.Loc.Empty), - .name = "exports", - .name_loc = logger.Loc.Empty, - }, logger.Loc.Empty), - obj, - ), - }, logger.Loc.Empty)); - } + obj, + ), + }, logger.Loc.Empty)); // mark a dependency on module_ref so it is renamed try ctx.last_part.symbol_uses.put(p.allocator, p.module_ref, .{ .count_estimate = 1 }); try ctx.last_part.declared_symbols.append(p.allocator, .{ .ref = p.module_ref, .is_top_level = true }); } - if (p.options.features.react_fast_refresh and p.react_refresh.register_used and !ctx.can_implicitly_accept) { + if (p.options.features.react_fast_refresh and p.react_refresh.register_used) { try ctx.stmts.append(p.allocator, Stmt.alloc(S.SExpr, .{ .value = Expr.init(E.Call, .{ .target = Expr.init(E.Dot, .{ diff --git a/src/ptr/CowSlice.zig b/src/ptr/CowSlice.zig index 81c61c46c4..224f6b5a12 100644 --- a/src/ptr/CowSlice.zig +++ b/src/ptr/CowSlice.zig @@ -189,7 +189,6 @@ pub fn CowSliceZ(T: type, comptime sentinel: ?T) type { pub fn deinit(str: Self, allocator: Allocator) void { if (comptime cow_str_assertions) if (str.debug) |debug| { debug.mutex.lock(); - defer debug.mutex.unlock(); bun.assertf( debug.allocator.ptr == allocator.ptr and debug.allocator.vtable == allocator.vtable, "CowSlice.deinit called with a different allocator than the one used to create it", @@ -205,6 +204,7 @@ pub fn CowSliceZ(T: type, comptime sentinel: ?T) type { bun.destroy(debug); } else { debug.borrows -= 1; // double deinit of a borrowed string + debug.mutex.unlock(); } }; if (str.flags.is_owned) { diff --git a/src/ptr/tagged_pointer.zig b/src/ptr/tagged_pointer.zig index 17409abe30..a14eac48b0 100644 --- a/src/ptr/tagged_pointer.zig +++ b/src/ptr/tagged_pointer.zig @@ -8,16 +8,15 @@ const strings = bun.strings; const default_allocator = bun.default_allocator; const C = bun.C; -const TagSize = u15; const AddressableSize = u49; pub const TaggedPointer = packed struct { _ptr: AddressableSize, - data: TagSize, + data: Tag, - pub const Tag = TagSize; + pub const Tag = u15; - pub inline fn init(ptr: anytype, data: TagSize) TaggedPointer { + pub inline fn init(ptr: anytype, data: Tag) TaggedPointer { const Ptr = @TypeOf(ptr); if (comptime Ptr == @TypeOf(null)) { return .{ ._ptr = 0, .data = data }; @@ -42,19 +41,19 @@ pub const TaggedPointer = packed struct { pub inline fn from(val: anytype) TaggedPointer { const ValueType = @TypeOf(val); return switch (ValueType) { - f64, i64, u64 => @as(TaggedPointer, @bitCast(val)), - ?*anyopaque, *anyopaque => @as(TaggedPointer, @bitCast(@intFromPtr(val))), + f64, i64, u64 => @bitCast(val), + ?*anyopaque, *anyopaque => @bitCast(@intFromPtr(val)), else => @compileError("Unsupported type: " ++ @typeName(ValueType)), }; } pub inline fn to(this: TaggedPointer) *anyopaque { - return @as(*anyopaque, @ptrFromInt(@as(u64, @bitCast(this)))); + return @ptrFromInt(@as(u64, @bitCast(this))); } }; const TypeMapT = struct { - value: TagSize, + value: TaggedPointer.Tag, ty: type, name: []const u8, }; @@ -84,7 +83,7 @@ pub fn TagTypeEnumWithTypeMap(comptime Types: anytype) struct { return .{ .tag_type = @Type(.{ .@"enum" = .{ - .tag_type = TagSize, + .tag_type = TaggedPointer.Tag, .fields = &enumFields, .decls = &.{}, .is_exhaustive = false, @@ -99,9 +98,9 @@ pub fn TaggedPointerUnion(comptime Types: anytype) type { const TagType: type = result.tag_type; - return packed struct { + return struct { pub const Tag = TagType; - pub const TagInt = TagSize; + pub const TagInt = TaggedPointer.Tag; pub const type_map: TypeMap(Types) = result.ty_map; repr: TaggedPointer, diff --git a/src/string_immutable.zig b/src/string_immutable.zig index 6eb62ba086..5ad2dd5616 100644 --- a/src/string_immutable.zig +++ b/src/string_immutable.zig @@ -894,7 +894,9 @@ pub fn withoutTrailingSlashWindowsPath(input: string) []const u8 { path.len -= 1; } - bun.assert(!isWindowsAbsolutePathMissingDriveLetter(u8, path)); + if (Environment.isDebug) + bun.debugAssert(!std.fs.path.isAbsolute(path) or + !isWindowsAbsolutePathMissingDriveLetter(u8, path)); return path; } @@ -4382,6 +4384,7 @@ pub fn indexOfNeedsURLEncode(slice: []const u8) ?u32 { if (remaining[0] >= 127 or remaining[0] < 0x20 or + remaining[0] == '%' or remaining[0] == '\\' or remaining[0] == '"' or remaining[0] == '#' or @@ -4402,6 +4405,7 @@ pub fn indexOfNeedsURLEncode(slice: []const u8) ?u32 { @as(AsciiVectorU1, @bitCast(vec > max_16_ascii)) | @as(AsciiVectorU1, @bitCast((vec < min_16_ascii))) | @as(AsciiVectorU1, @bitCast(vec == @as(AsciiVector, @splat('%')))) | + @as(AsciiVectorU1, @bitCast(vec == @as(AsciiVector, @splat('\\')))) | @as(AsciiVectorU1, @bitCast(vec == @as(AsciiVector, @splat('"')))) | @as(AsciiVectorU1, @bitCast(vec == @as(AsciiVector, @splat('#')))) | @as(AsciiVectorU1, @bitCast(vec == @as(AsciiVector, @splat('?')))) | @@ -4425,6 +4429,7 @@ pub fn indexOfNeedsURLEncode(slice: []const u8) ?u32 { const char = char_.*; if (char > 127 or char < 0x20 or char == '\\' or + char == '%' or char == '"' or char == '#' or char == '?' or diff --git a/test/bake/bake-harness.ts b/test/bake/bake-harness.ts index 930cac9e61..b6d6997f4e 100644 --- a/test/bake/bake-harness.ts +++ b/test/bake/bake-harness.ts @@ -24,6 +24,21 @@ import { exitCodeMapStrings } from "./exit-code-map.mjs"; const isDebugBuild = Bun.version.includes("debug"); +const verboseSynchronization = process.env.BUN_DEV_SERVER_VERBOSE_SYNC + ? (arg: string) => { + console.log("\x1b[36m" + arg + "\x1b[0m"); + } + : () => {}; + +/** + * Can be set in fast development environments to improve iteration time. + * In CI/Windows it appears that sometimes these tests dont wait enough + * for things to happen, so the extra delay reduces flakiness. + * + * Needs much more investigation. + */ +const fastBatches = !!process.env.BUN_DEV_SERVER_FAST_BATCHES; + /** For testing bundler related bugs in the DevServer */ export const minimalFramework: Bake.Framework = { fileSystemRouterTypes: [ @@ -104,6 +119,8 @@ export interface DevServerTest { * Avoid if possible, this is to reproduce specific bugs. */ mainDir?: string; + + skip?: ('win32'|'darwin'|'linux'|'ci')[], } let interactive = false; @@ -147,6 +164,19 @@ type ErrorSpec = string; type FileObject = Record; +enum WatchSynchronization { + // Callback for starting a batch + Started = 0, + // During a batch, files were seen. Batch is still running. + SeenFiles = 1, + // Batch no longer running, files seen! + ResultDidNotBundle = 2, + // Sent on every build finished: + AnyBuildFinished = 3, + // Sent on every build finished, you must wait for web sockets: + AnyBuildFinishedWaitForWebSockets = 4, +} + export class Dev extends EventEmitter { rootDir: string; port: number; @@ -155,6 +185,7 @@ export class Dev extends EventEmitter { connectedClients: Set = new Set(); options: { files: Record }; nodeEnv: "development" | "production"; + batchingChanges: { write?: () => void } | null = null; socket?: WebSocket; @@ -193,7 +224,8 @@ export class Dev extends EventEmitter { connected.resolve(); } if (data[0] === "r".charCodeAt(0)) { - this.emit("redundant_watch"); + verboseSynchronization("watch_synchronization: " + WatchSynchronization[data[1]]); + this.emit("watch_synchronization", data[1]); } this.emit("hmr", data); }; @@ -217,25 +249,105 @@ export class Dev extends EventEmitter { }); } + #waitForSyncEvent(event: WatchSynchronization) { + return new Promise((resolve, reject) => { + let dev = this; + function handle(kind: WatchSynchronization) { + if (kind === event) { + dev.off("watch_synchronization", handle); + resolve(); + } + } + dev.on("watch_synchronization", handle); + }); + } + + async batchChanges(options: { errors?: null | ErrorSpec[]; snapshot?: string } = {}) { + if (this.batchingChanges) { + this.batchingChanges.write?.(); + return null; + } + this.batchingChanges = {}; + + let dev = this; + const initWait = this.#waitForSyncEvent(WatchSynchronization.Started); + this.socket!.send("H"); + await initWait; + + let hasSeenFiles = true; + let seenFiles: PromiseWithResolvers; + function onSeenFiles(ev: WatchSynchronization) { + if (ev === WatchSynchronization.SeenFiles) { + hasSeenFiles = true; + seenFiles.resolve(); + dev.off("watch_synchronization", onSeenFiles); + } + } + function resetSeenFilesWithResolvers() { + if (!hasSeenFiles) return; + seenFiles = Promise.withResolvers(); + dev.on("watch_synchronization", onSeenFiles); + } + resetSeenFilesWithResolvers(); + + let wantsHmrEvent = true; + for (const client of dev.connectedClients) { + if (!client.webSocketMessagesAllowed) { + wantsHmrEvent = false; + break; + } + } + + const wait = this.waitForHotReload(wantsHmrEvent); + const b = { + write: resetSeenFilesWithResolvers, + [Symbol.asyncDispose]: async() => { + if (wantsHmrEvent && interactive) { + await seenFiles.promise; + } else if (wantsHmrEvent) { + await Promise.race([ + seenFiles.promise, + Bun.sleep(1000), + ]); + } + if (!fastBatches) { + // Wait an extra delay to avoid double-triggering events. + await Bun.sleep(300); + } + + dev.off("watch_synchronization", onSeenFiles); + + this.socket!.send("H"); + await wait; + + let errors = options.errors; + if (errors !== null) { + errors ??= []; + for (const client of this.connectedClients) { + await client.expectErrorOverlay(errors, options.snapshot); + } + } + this.batchingChanges = null; + }, + }; + this.batchingChanges = b; + return b; + } + write(file: string, contents: string, options: { errors?: null | ErrorSpec[]; dedent?: boolean } = {}) { const snapshot = snapshotCallerLocation(); return withAnnotatedStack(snapshot, async () => { await maybeWaitInteractive("write " + file); const isDev = this.nodeEnv === "development"; - const wait = isDev && this.waitForHotReload(); + await using _wait = isDev ? await this.batchChanges({ + errors: options.errors, + snapshot: snapshot, + }) : null; + await Bun.write( this.join(file), ((typeof contents === "string" && options.dedent) ?? true) ? dedent(contents) : contents, ); - await wait; - - let errors = options.errors; - if (isDev && errors !== null) { - errors ??= []; - for (const client of this.connectedClients) { - await client.expectErrorOverlay(errors, snapshot); - } - } }); } @@ -258,12 +370,15 @@ export class Dev extends EventEmitter { * @param options Options for handling errors after deletion * @returns Promise that resolves when the file is deleted and hot reload is complete (if applicable) */ - delete(file: string, options: { errors?: null | ErrorSpec[]; wait?: boolean } = {}) { + delete(file: string, options: { errors?: null | ErrorSpec[] } = {}) { const snapshot = snapshotCallerLocation(); return withAnnotatedStack(snapshot, async () => { await maybeWaitInteractive("delete " + file); const isDev = this.nodeEnv === "development"; - const wait = isDev && options.wait && this.waitForHotReload(); + await using _wait = isDev ? await this.batchChanges({ + errors: options.errors, + snapshot: snapshot, + }) : null; const filePath = this.join(file); if (!fs.existsSync(filePath)) { @@ -271,15 +386,6 @@ export class Dev extends EventEmitter { } fs.unlinkSync(filePath); - await wait; - - let errors = options.errors; - if (isDev && options.wait && errors !== null) { - errors ??= []; - for (const client of this.connectedClients) { - await client.expectErrorOverlay(errors, snapshot); - } - } }); } @@ -295,7 +401,12 @@ export class Dev extends EventEmitter { const snapshot = snapshotCallerLocation(); return withAnnotatedStack(snapshot, async () => { await maybeWaitInteractive("patch " + file); - const wait = this.waitForHotReload(); + const isDev = this.nodeEnv === "development"; + await using _wait = isDev ? await this.batchChanges({ + errors: errors, + snapshot: snapshot, + }) : null; + const filename = this.join(file); const source = fs.readFileSync(filename, "utf8"); const contents = source.replace(find, replace); @@ -303,14 +414,6 @@ export class Dev extends EventEmitter { throw new Error(`Couldn't find and replace ${JSON.stringify(find)} in ${file}`); } await Bun.write(filename, typeof contents === "string" && shouldDedent ? dedent(contents) : contents); - await wait; - - if (errors !== null) { - errors ??= []; - for (const client of this.connectedClients) { - await client.expectErrorOverlay(errors, snapshot); - } - } }); } @@ -318,22 +421,60 @@ export class Dev extends EventEmitter { return path.join(this.rootDir, file); } - async waitForHotReload() { + waitForHotReload(wantsHmrEvent: boolean) { if (this.nodeEnv !== "development") return Promise.resolve(); - const err = this.output.waitForLine(/error/i).catch(() => {}); - const success = this.output.waitForLine(/bundled page|bundled route|reloaded/i, isCI ? 1000 : 250).catch(() => {}); - const ctrl = new AbortController(); - await Promise.race([ - // On failure, give a little time in case a partial write caused a - // bundling error, and a success came in. - err.then( - () => Bun.sleep(500), - () => {}, - ), - success, - EventEmitter.once(this, "redundant_watch", { signal: ctrl.signal }), - ]); - ctrl.abort(); + let dev = this; + return new Promise((resolve, reject) => { + let timer: NodeJS.Timer | null = null; + let clientWaits = 0; + let seenMainEvent = false; + function cleanupAndResolve() { + verboseSynchronization("Cleaning up and resolving"); + timer !== null && clearTimeout(timer); + dev.off("watch_synchronization", onEvent); + for (const dispose of disposes) { + dispose(); + } + if (fastBatches) resolve(); + else setTimeout(resolve, 250); + } + const disposes = new Set<() => void>(); + for (const client of dev.connectedClients) { + const socketEventHandler = () => { + verboseSynchronization("Client received event"); + clientWaits++; + if (seenMainEvent && clientWaits === dev.connectedClients.size) { + client.off("received-hmr-event", socketEventHandler); + cleanupAndResolve(); + } + }; + client.on("received-hmr-event", socketEventHandler); + disposes.add(() => { + client.off("received-hmr-event", socketEventHandler); + }); + } + async function onEvent(kind: WatchSynchronization) { + assert(kind !== WatchSynchronization.Started, "WatchSynchronization.Started should not be emitted"); + if (kind === WatchSynchronization.AnyBuildFinished) { + seenMainEvent = true; + cleanupAndResolve(); + } else if (kind === WatchSynchronization.AnyBuildFinishedWaitForWebSockets) { + verboseSynchronization("Need to wait for (" + clientWaits + "/" + dev.connectedClients.size + ") clients"); + seenMainEvent = true; + if (clientWaits === dev.connectedClients.size) { + cleanupAndResolve(); + } + } else if (kind === WatchSynchronization.ResultDidNotBundle) { + if (wantsHmrEvent) { + await Bun.sleep(500); + if (seenMainEvent) return; + console.warn("\x1b[33mWARN: Dev Server did not pick up any changed files. Consider wrapping this call in expectNoWebSocketActivity\x1b[35m"); + } + cleanupAndResolve(); + } + }; + dev.on("watch_synchronization", onEvent); + }); } async client( @@ -552,6 +693,7 @@ export class Client extends EventEmitter { suppressInteractivePrompt: boolean = false; expectingReload = false; hmr = false; + webSocketMessagesAllowed = true; constructor(url: string, options: { storeHotChunks?: boolean; hmr: boolean; expectErrors?: boolean }) { super(); @@ -684,19 +826,37 @@ export class Client extends EventEmitter { } expectMessage(...x: any) { + return this.#expectMessageImpl(true, x); + } + + expectMessageInAnyOrder(...x: any) { + return this.#expectMessageImpl(false, x); + } + + #expectMessageImpl(strictOrdering: boolean, x: any[]) { return withAnnotatedStack(snapshotCallerLocation(), async () => { if (this.exited) throw new Error("Client exited while waiting for message"); if (this.messages.length !== x.length) { + if (interactive) { + console.log("Waiting for messages (have", this.messages.length, "expected", x.length, ")"); + } + const dev = this; // Wait up to a threshold before giving up + function cleanup() { + dev.off("message", onMessage); + dev.off("exit", onExit); + } const resolver = Promise.withResolvers(); function onMessage(message: any) { - if (this.messages.length === x.length) resolver.resolve(); + process.nextTick(() => { + if (dev.messages.length === x.length) resolver.resolve(); + }); } function onExit() { resolver.resolve(); } - this.once("message", onMessage); - this.once("exit", onExit); + this.on("message", onMessage); + this.on("exit", onExit); let t: any = setTimeout( () => { t = null; @@ -706,11 +866,15 @@ export class Client extends EventEmitter { ); await resolver.promise; if (t) clearTimeout(t); - this.off("message", onMessage); + cleanup(); } if (this.exited) throw new Error("Client exited while waiting for message"); - const m = this.messages; + let m = this.messages; this.messages = []; + if (!strictOrdering) { + m = m.sort(); + x = x.sort(); + } expect(m).toEqual(x); }); } @@ -935,12 +1099,14 @@ export class Client extends EventEmitter { // Block WebSocket messages this.#proc.send({ type: "set-allow-websocket-messages", args: [false] }); + this.webSocketMessagesAllowed = false; try { await cb(); } finally { // Re-enable WebSocket messages this.#proc.send({ type: "set-allow-websocket-messages", args: [true] }); + this.webSocketMessagesAllowed = true; } }); } @@ -1275,7 +1441,7 @@ class OutputLineStream extends EventEmitter { export function indexHtmlScript(htmlFiles: string[]) { return [ - ...htmlFiles.map((file, i) => `import html${i} from "./${file}";`), + ...htmlFiles.map((file, i) => `import html${i} from ${JSON.stringify("./" + file.replaceAll(path.sep, '/'))};`), "export default {", " static: {", ...(htmlFiles.length === 1 @@ -1298,6 +1464,11 @@ export function indexHtmlScript(htmlFiles: string[]) { ].join("\n"); } +const skipTargets = [ + process.platform, + isCI ? 'ci' : null, +].filter(Boolean); + function testImpl( description: string, options: T, @@ -1383,7 +1554,7 @@ function testImpl( fs.writeFileSync( path.join(root, "harness_start.ts"), dedent` - import appConfig from "${path.join(mainDir, "bun.app.ts")}"; + import appConfig from ${JSON.stringify(path.join(mainDir, "bun.app.ts"))}; import { fullGC } from "bun:jsc"; const routes = appConfig.static ?? (appConfig.routes ??= {}); @@ -1509,17 +1680,16 @@ function testImpl( : "PROD" }:${basename}-${count}: ${description}`; try { - // TODO: resolve ci flakiness. - if (isCI && isWindows) { - return jest.test.skip(name, run); + if (options.skip && options.skip.some(x => skipTargets.includes(x))) { + jest.test.todo(name, run); + return options; } - jest.test( name, run, interactive ? interactive_timeout - : (options.timeoutMultiplier ?? 1) * (isWindows ? 10_000 : 5_000) * (Bun.version.includes("debug") ? 3 : 1), + : (options.timeoutMultiplier ?? 1) * (isWindows ? 15_000 : 10_000) * (Bun.version.includes("debug") ? 2 : 1), ); return options; } catch { diff --git a/test/bake/client-fixture.mjs b/test/bake/client-fixture.mjs index 290a024b34..ce32a350e4 100644 --- a/test/bake/client-fixture.mjs +++ b/test/bake/client-fixture.mjs @@ -15,6 +15,7 @@ url = new URL(url, "http://localhost:3000"); const storeHotChunks = args.includes("--store-hot-chunks"); const expectErrors = args.includes("--expect-errors"); +const verboseWebSockets = args.includes("--verbose-web-sockets"); // Create a new window instance let window; @@ -23,6 +24,7 @@ let expectingReload = false; let webSockets = []; let pendingReload = null; let pendingReloadTimer = null; +let updatingTimer = null; function reset() { for (const ws of webSockets) { @@ -41,6 +43,7 @@ function reset() { warn: () => {}, info: () => {}, assert: () => {}, + trace: () => {}, }; } } @@ -54,6 +57,21 @@ function createWindow(windowUrl) { height: 768, }); + let hasReadyEventListener = false; + window["bun do not use this outside of internal testing or else i'll cry"] = ({ onEvent }) => { + onEvent("bun:afterUpdate", () => { + setTimeout(() => { + process.send({ type: "received-hmr-event", args: [] }); + }, 50); + }); + hasReadyEventListener = true; + onEvent("bun:ready", () => { + setTimeout(() => { + process.send({ type: "received-hmr-event", args: [] }); + }, 50); + }); + }; + window.fetch = async function (url, options) { if (typeof url === "string") { url = new URL(url, windowUrl).href; @@ -68,31 +86,25 @@ function createWindow(windowUrl) { super(url, protocols, options); webSockets.push(this); this.addEventListener("message", event => { - if (!allowWebSocketMessages) { - const data = new Uint8Array(event.data); - console.error( - "[E] WebSocket message received while messages are not allowed. Event type", - JSON.stringify(String.fromCharCode(data[0])), - ); - let hexDump = ""; - for (let i = 0; i < data.length; i += 16) { - // Print offset - hexDump += "\x1b[2m" + i.toString(16).padStart(4, "0") + "\x1b[0m "; - // Print hex values - const chunk = data.slice(i, i + 16); - const hexValues = Array.from(chunk) - .map(b => b.toString(16).padStart(2, "0")) - .join(" "); - hexDump += hexValues.padEnd(48, " "); - // Print ASCII - hexDump += "\x1b[2m| \x1b[0m"; - for (const byte of chunk) { - hexDump += byte >= 32 && byte <= 126 ? String.fromCharCode(byte) : "\x1b[2m.\x1b[0m"; - } - hexDump += "\n"; + const data = new Uint8Array(event.data); + if (data[0] === "e".charCodeAt(0)) { + if (updatingTimer) { + clearTimeout(updatingTimer); } - console.error(hexDump); + updatingTimer = setTimeout(() => { + process.send({ type: "received-hmr-event", args: [] }); + updatingTimer = null; + }, 250); + } + if (!allowWebSocketMessages) { + const allowedTypes = ["n", "r"]; + if (allowedTypes.includes(String.fromCharCode(data[0]))) { + return; + } + dumpWebSocketMessage("[E] WebSocket message received while messages are not allowed", data); process.exit(exitCodeMap.websocketMessagesAreBanned); + } else { + verboseWebSockets && dumpWebSocketMessage("[I] WebSocket", data); } }); } @@ -106,7 +118,7 @@ function createWindow(windowUrl) { const originalConsole = window.console; window.console = { log: (...args) => { - process?.send({ type: "message", args: args }); + process.send({ type: "message", args: args }); }, error: (...args) => { console.error("[E]", ...args); @@ -120,6 +132,14 @@ function createWindow(windowUrl) { originalConsole.warn(...args); }, info: (...args) => { + if (args[0]?.startsWith("[Bun] Hot-module-reloading socket connected")) { + // Wait for all CSS assets to be fully loaded before emitting the event + if (!hasReadyEventListener) { + setTimeout(() => { + process.send({ type: "received-hmr-event", args: [] }); + }, 50); + } + } if (args[0]?.startsWith("[WS] receive message")) return; if (args[0]?.startsWith("Updated modules:")) return; console.info("[I]", ...args); @@ -130,9 +150,14 @@ function createWindow(windowUrl) { console.trace(...args); process.exit(exitCodeMap.assertionFailed); }, + trace: console.trace, }; window.location.reload = async () => { + if (updatingTimer) { + clearTimeout(updatingTimer); + } + console.info("[I] location.reload()"); reset(); if (expectingReload) { // Permission already granted, proceed with reload @@ -166,6 +191,28 @@ function createWindow(windowUrl) { } } +function dumpWebSocketMessage(message, data) { + console.error(`${message}. Event type`, JSON.stringify(String.fromCharCode(data[0]))); + let hexDump = ""; + for (let i = 0; i < data.length; i += 16) { + // Print offset + hexDump += "\x1b[2m" + i.toString(16).padStart(4, "0") + "\x1b[0m "; + // Print hex values + const chunk = data.slice(i, i + 16); + const hexValues = Array.from(chunk) + .map(b => b.toString(16).padStart(2, "0")) + .join(" "); + hexDump += hexValues.padEnd(48, " "); + // Print ASCII + hexDump += "\x1b[2m| \x1b[0m"; + for (const byte of chunk) { + hexDump += byte >= 32 && byte <= 126 ? String.fromCharCode(byte) : "\x1b[2m.\x1b[0m"; + } + hexDump += "\n"; + } + console.error(hexDump); +} + async function handleReload() { expectingReload = false; pendingReload = null; diff --git a/test/bake/dev/bundle.test.ts b/test/bake/dev/bundle.test.ts index d7ac4e0d87..89c87d5d42 100644 --- a/test/bake/dev/bundle.test.ts +++ b/test/bake/dev/bundle.test.ts @@ -195,6 +195,18 @@ devTest("default export same-scope handling", { await dev.writeNoChanges("fixture7.ts"); const chunk = await c.getMostRecentHmrChunk(); expect(chunk).toMatch(/default:\s*function/); + + // Since fixture7.ts is not marked as accepting, it will bubble the update + // to `index.ts`, re-evaluate it and some of the dependencies. + c.expectMessage( + "TWO", + "FOUR", + "FIVE", + "SEVEN", + "EIGHT", + "NINE", + "ELEVEN", + ); }, }); devTest("directory cache bust case #17576", { @@ -231,6 +243,9 @@ devTest("directory cache bust case #17576", { }, }); devTest("deleting imported file shows error then recovers", { + skip: [ + 'win32', // unlinkSync is having weird behavior + ], files: { "index.html": emptyHtmlFile({ styles: [], @@ -326,3 +341,50 @@ devTest("import.meta.main", { await c.expectMessage(false); }, }); +devTest("commonjs forms", { + files: { + "index.html": emptyHtmlFile({ + styles: [], + scripts: ["index.ts"], + }), + "index.ts": ` + import cjs from "./cjs.js"; + console.log(cjs); + `, + "cjs.js": ` + module.exports.field = {}; + `, + }, + async test(dev) { + await using c = await dev.client("/"); + await c.expectMessage({ field: {} }); + await c.expectReload(async () => { + await dev.write("cjs.js", `exports.field = "1";`); + }); + await c.expectMessage({ field: "1" }); + await c.expectReload(async () => { + await dev.write("cjs.js", `let theExports = exports; theExports.field = "2";`); + }); + await c.expectMessage({ field: "2" }); + await c.expectReload(async () => { + await dev.write("cjs.js", `let theModule = module; theModule.exports.field = "3";`); + }); + await c.expectMessage({ field: "3" }); + await c.expectReload(async () => { + await dev.write("cjs.js", `let { exports } = module; exports.field = "4";`); + }); + await c.expectMessage({ field: "4" }); + await c.expectReload(async () => { + await dev.write("cjs.js", `var { exports } = module; exports.field = "4.5";`); + }); + await c.expectMessage({ field: "4.5" }); + await c.expectReload(async () => { + await dev.write("cjs.js", `let theExports = module.exports; theExports.field = "5";`); + }); + await c.expectMessage({ field: "5" }); + await c.expectReload(async () => { + await dev.write("cjs.js", `require; eval("module.exports.field = '6'");`); + }); + await c.expectMessage({ field: "6" }); + }, +}); \ No newline at end of file diff --git a/test/bake/dev/css.test.ts b/test/bake/dev/css.test.ts index 4a83f00874..9a32d08fdb 100644 --- a/test/bake/dev/css.test.ts +++ b/test/bake/dev/css.test.ts @@ -477,10 +477,8 @@ devTest("css import before create project relative", { }, ); await c.expectNoWebSocketActivity(async () => { - await dev.write("assets/bun.png", imageFixtures.bun, { - errors: ['style/styles.css:2:21: error: Could not resolve: "/assets/bun.png"'], - }); - await dev.delete("assets/bun.png", { wait: false }); + await dev.write("assets/bun.png", imageFixtures.bun, { errors: null }); + await dev.delete("assets/bun.png", { errors: null }); }); await dev.fetch("/").expect.not.toContain("HELLO"); await dev.write( diff --git a/test/bake/dev/ecosystem.test.ts b/test/bake/dev/ecosystem.test.ts index fb5ad3feba..e93a39ff6d 100644 --- a/test/bake/dev/ecosystem.test.ts +++ b/test/bake/dev/ecosystem.test.ts @@ -23,20 +23,32 @@ devTest("svelte component islands example", { await using c = await dev.client("/"); expect(await c.elemText("button")).toBe("Clicked 5 times"); - await c.click("button"); - await Bun.sleep(500); // TODO: de-flake event ordering. - expect(await c.elemText("button")).toBe("Clicked 6 times"); + const result = await c.js` + document.querySelector("button").click(); + await new Promise(resolve => setTimeout(resolve, 10)); + return document.querySelector("button").textContent; + `; + expect(result).toBe("Clicked 6 times"); - await dev.patch("pages/index.svelte", { - find: "non-interactive", - replace: "awesome", + await c.expectReload(async () => { + await dev.patch("pages/index.svelte", { + find: "non-interactive", + replace: "awesome", + }); }); + await dev.patch("pages/_Counter.svelte", { + find: "interactive island", + replace: "magical", + }); + + expect(await c.elemText("#counter_text")).toInclude("magical"); + const html2 = await dev.fetch("/").text(); if (html2.includes("Bun__renderFallbackError")) throw new Error("failed"); // Expect SSR expect(html2).toContain(`

This is my svelte server component (awesome)

Bun v${Bun.version}

`); - expect(html2).toContain(`>This is a client component (interactive island)

`); + expect(html2).toContain(`>This is a client component (magical)

`); }, }); diff --git a/test/bake/dev/esm.test.ts b/test/bake/dev/esm.test.ts index b74eb841b4..05f637ba43 100644 --- a/test/bake/dev/esm.test.ts +++ b/test/bake/dev/esm.test.ts @@ -22,10 +22,12 @@ const liveBindingTest = devTest("live bindings with `var`", { await dev.fetch("/").equals("State: 1"); await dev.fetch("/").equals("State: 2"); await dev.fetch("/").equals("State: 3"); + console.log("patching"); await dev.patch("routes/index.ts", { find: "State", replace: "Value", }); + console.log("patching"); await dev.fetch("/").equals("Value: 4"); await dev.fetch("/").equals("Value: 5"); await dev.write( @@ -302,3 +304,58 @@ devTest("cannot require a module with top level await", { }); }, }); +devTest("function that is assigned to should become a live binding", { + files: { + "index.html": emptyHtmlFile({ + scripts: ["index.ts"], + }), + "index.ts": ` + // 1. basic test + import { live, change } from "./live.js"; + { + if (live() !== 1) throw new Error("live() should be 1"); + change(); + if (live() !== 2) throw new Error("live() should be 2"); + } + + // 2. integration test with @babel/runtime + import inheritsLoose from "./inheritsLoose.js"; + { + function A() {} + function B() {} + inheritsLoose(B, A); + } + + console.log('PASS'); + `, + "live.js": ` + export function live() { + return 1; + } + export function change() { + live = function() { + return 2; + } + } + `, + "inheritsLoose.js": ` + import setPrototypeOf from "./setPrototypeOf.js"; + function _inheritsLoose(t, o) { + t.prototype = Object.create(o.prototype), t.prototype.constructor = t, setPrototypeOf(t, o); + } + export { _inheritsLoose as default }; + `, + "setPrototypeOf.js": ` + function _setPrototypeOf(t, e) { + return _setPrototypeOf = Object.setPrototypeOf ? Object.setPrototypeOf.bind() : function (t, e) { + return t.__proto__ = e, t; + }, _setPrototypeOf(t, e); + } + export { _setPrototypeOf as default }; + `, + }, + async test(dev) { + await using c = await dev.client(); + await c.expectMessage("PASS"); + }, +}); diff --git a/test/bake/dev/hot.test.ts b/test/bake/dev/hot.test.ts index e9a6325fd7..6e83b13960 100644 --- a/test/bake/dev/hot.test.ts +++ b/test/bake/dev/hot.test.ts @@ -105,6 +105,7 @@ devTest("import.meta.hot.accept patches imports", { }, }); devTest("import.meta.hot.accept specifier", { + timeoutMultiplier: 3, files: { "index.html": emptyHtmlFile({ scripts: ["a.ts"], @@ -292,22 +293,23 @@ devTest("import.meta.hot.accept multiple modules", { await c.expectMessage("Name updated: Bob"); // Test updating both files - await Promise.all([ - dev.write( + { + await using batch = await dev.batchChanges(); + await dev.write( "counter.ts", ` export const count = 3; `, - ), - dev.write( + ); + await dev.write( "name.ts", ` export const name = "Charlie"; `, - ), - ]); + ); + } - await c.expectMessage("Counter updated: 3", "Name updated: Charlie"); + await c.expectMessageInAnyOrder("Counter updated: 3", "Name updated: Charlie"); }, }); devTest("import.meta.hot.data persistence", { diff --git a/test/bake/dev/react-spa.test.ts b/test/bake/dev/react-spa.test.ts index 2a3f37012e..8944361666 100644 --- a/test/bake/dev/react-spa.test.ts +++ b/test/bake/dev/react-spa.test.ts @@ -52,7 +52,7 @@ const reactAndRefreshStub = { exports.register = function(fn, name) { if (typeof name !== "string") throw new Error("name must be a string"); if (typeof fn !== "function") throw new Error("fn must be a function"); - if (components.has(name)) throw new Error("Component already registered: " + name + ". Read its hash from test harness first"); + if (components.has(name)) console.warn("WARNING: Component already registered: " + name + ". Read its hash from test harness first"); const entry = functionToComponent.get(fn) ?? { fn, calls: 0, hash: undefined, name: undefined, customHooks: undefined }; entry.name = name; components.set(name, entry); diff --git a/test/bake/dev/sourcemap.test.ts b/test/bake/dev/sourcemap.test.ts index 972e7d5833..7bbf3875d7 100644 --- a/test/bake/dev/sourcemap.test.ts +++ b/test/bake/dev/sourcemap.test.ts @@ -93,6 +93,10 @@ async function extractSourceMap(dev: Dev, scriptSource: string) { throw new Error("Source map URL not found in " + scriptSource); } const sourceMap = await dev.fetch(sourceMapUrl[1]).text(); + if (!sourceMap.startsWith('{')) { + throw new Error("Source map is not valid JSON: " + sourceMap); + } + console.log(sourceMap); return new Promise((resolve, reject) => { try { SourceMapConsumer.with(sourceMap, null, async (consumer: any) => { diff --git a/test/bake/fixtures/svelte-component-islands/pages/_Counter.svelte b/test/bake/fixtures/svelte-component-islands/pages/_Counter.svelte index 7c4a618edb..5fdb4d7062 100644 --- a/test/bake/fixtures/svelte-component-islands/pages/_Counter.svelte +++ b/test/bake/fixtures/svelte-component-islands/pages/_Counter.svelte @@ -8,7 +8,7 @@
-

This is a client component (interactive island)

+

This is a client component (interactive island)

diff --git a/test/bundler/bundler_drop.test.ts b/test/bundler/bundler_drop.test.ts index a50bda9f58..ee1ceffe7e 100644 --- a/test/bundler/bundler_drop.test.ts +++ b/test/bundler/bundler_drop.test.ts @@ -106,4 +106,12 @@ describe("bundler", () => { run: { stdout: "true" }, drop: ["Bun"], }); + itBundled("drop/IdentifierCall", { + files: { + "/a.js": `ASSERT("hello");`, + }, + run: { stdout: "" }, + drop: ["ASSERT"], + backend: "api", + }); });