From fb6f7e43d8817b8a4a4dfe751ac8563d8f34a7fc Mon Sep 17 00:00:00 2001 From: chloe caruso Date: Fri, 21 Feb 2025 20:08:21 -0800 Subject: [PATCH] Dev Server: improve react refresh and export default handling (#17538) --- src/bake/client/overlay.ts | 1 - src/bake/hmr-runtime-client.ts | 2 +- src/js_parser.zig | 415 ++++++++++++++++++++------------ test/bake/client-fixture.mjs | 1 + test/bake/dev-server-harness.ts | 151 +++++++++++- test/bake/dev/bundle.test.ts | 282 +++++++++++++++++++++- 6 files changed, 679 insertions(+), 173 deletions(-) diff --git a/src/bake/client/overlay.ts b/src/bake/client/overlay.ts index 82496cf71d..360d192b57 100644 --- a/src/bake/client/overlay.ts +++ b/src/bake/client/overlay.ts @@ -314,7 +314,6 @@ export async function onRuntimeError(err: any, fatal = false, async = false) { async, code, }); - console.log(code); } catch (e) { console.error("Failed to remap error", e); runtimeErrors.push({ diff --git a/src/bake/hmr-runtime-client.ts b/src/bake/hmr-runtime-client.ts index d4eba56bef..1e93373411 100644 --- a/src/bake/hmr-runtime-client.ts +++ b/src/bake/hmr-runtime-client.ts @@ -185,7 +185,7 @@ const ws = initWebSocket( } window.addEventListener("error", event => { - onRuntimeError(event.error, true, true); + onRuntimeError(event.error, true, false); }); window.addEventListener("unhandledrejection", event => { onRuntimeError(event.reason, true, true); diff --git a/src/js_parser.zig b/src/js_parser.zig index be36b1cef9..10da0ccd0d 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -19165,6 +19165,33 @@ fn NewParser_( data.default_name = createDefaultName(p, data.value.expr.loc) catch unreachable; } + if (p.options.features.react_fast_refresh and switch (data.value.expr.data) { + .e_arrow => true, + .e_call => |call| switch (call.target.data) { + .e_identifier => |id| id.ref == p.react_refresh.latest_signature_ref, + else => false, + }, + else => false, + }) { + // declare a temporary ref for this + const temp_id = p.generateTempRef("default_export"); + try p.current_scope.generated.push(p.allocator, temp_id); + + try stmts.append(Stmt.alloc(S.Local, .{ + .kind = .k_const, + .decls = try G.Decl.List.fromSlice(p.allocator, &.{ + .{ + .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp_id }, stmt.loc), + .value = data.value.expr, + }, + }), + }, stmt.loc)); + + data.value = .{ .expr = .initIdentifier(temp_id, stmt.loc) }; + + try p.emitReactRefreshRegister(stmts, "default", temp_id, .default); + } + if (p.options.features.server_components.wrapsExports()) { data.value.expr = p.wrapValueForServerComponentReference(data.value.expr, "default"); } @@ -19203,119 +19230,150 @@ fn NewParser_( } }, - .stmt => |s2| { - switch (s2.data) { - .s_function => |func| { - var name: string = ""; - if (func.func.name) |func_loc| { - name = p.loadNameFromRef(func_loc.ref.?); + .stmt => |s2| switch (s2.data) { + .s_function => |func| { + const name = if (func.func.name) |func_loc| + p.loadNameFromRef(func_loc.ref.?) + else name: { + func.func.name = data.default_name; + break :name js_ast.ClauseItem.default_alias; + }; + + var react_hook_data: ?ReactRefresh.HookContext = null; + const prev = p.react_refresh.hook_ctx_storage; + defer p.react_refresh.hook_ctx_storage = prev; + p.react_refresh.hook_ctx_storage = &react_hook_data; + + func.func = p.visitFunc(func.func, func.func.open_parens_loc); + + if (p.is_control_flow_dead) { + return; + } + + if (data.default_name.ref.?.isSourceContentsSlice()) { + data.default_name = createDefaultName(p, stmt.loc) catch unreachable; + } + + if (react_hook_data) |*hook| { + stmts.append(p.getReactRefreshHookSignalDecl(hook.signature_cb)) catch bun.outOfMemory(); + + data.value = .{ + .expr = p.getReactRefreshHookSignalInit(hook, p.newExpr( + E.Function{ .func = func.func }, + stmt.loc, + )), + }; + } + + if (mark_for_replace) { + const entry = p.options.features.replace_exports.getPtr("default").?; + if (entry.* == .replace) { + data.value = .{ .expr = entry.replace }; } else { - func.func.name = data.default_name; - name = js_ast.ClauseItem.default_alias; - } - - var react_hook_data: ?ReactRefresh.HookContext = null; - const prev = p.react_refresh.hook_ctx_storage; - defer p.react_refresh.hook_ctx_storage = prev; - p.react_refresh.hook_ctx_storage = &react_hook_data; - - func.func = p.visitFunc(func.func, func.func.open_parens_loc); - - if (react_hook_data) |*hook| { - stmts.append(p.getReactRefreshHookSignalDecl(hook.signature_cb)) catch bun.outOfMemory(); - - data.value = .{ - .expr = p.getReactRefreshHookSignalInit(hook, p.newExpr( - E.Function{ .func = func.func }, - stmt.loc, - )), - }; - } - - if (p.is_control_flow_dead) { + _ = p.injectReplacementExport(stmts, Ref.None, logger.Loc.Empty, entry); return; } + } - if (mark_for_replace) { - const entry = p.options.features.replace_exports.getPtr("default").?; - if (entry.* == .replace) { - data.value = .{ .expr = entry.replace }; - } else { - _ = p.injectReplacementExport(stmts, Ref.None, logger.Loc.Empty, entry); - return; - } - } - - if (data.default_name.ref.?.isSourceContentsSlice()) { - data.default_name = createDefaultName(p, stmt.loc) catch unreachable; - } - - if (p.options.features.react_fast_refresh) { - try p.handleReactRefreshRegister(stmts, name, data.default_name.ref.?, .default); + if (p.options.features.react_fast_refresh and + (ReactRefresh.isComponentishName(name) or bun.strings.eqlComptime(name, "default"))) + { + // If server components or react refresh had wrapped the value (convert to .expr) + // then a temporary variable must be emitted. + // + // > export default _s(function App() { ... }, "...") + // > $RefreshReg(App, "App.tsx:default") + // + // > const default_export = _s(function App() { ... }, "...") + // > export default default_export; + // > $RefreshReg(default_export, "App.tsx:default") + const ref = if (data.value == .expr) emit_temp_var: { + const temp_id = p.generateTempRef("default_export"); + try p.current_scope.generated.push(p.allocator, temp_id); + + stmts.append(Stmt.alloc(S.Local, .{ + .kind = .k_const, + .decls = try G.Decl.List.fromSlice(p.allocator, &.{ + .{ + .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp_id }, stmt.loc), + .value = data.value.expr, + }, + }), + }, stmt.loc)) catch bun.outOfMemory(); + + data.value = .{ .expr = .initIdentifier(temp_id, stmt.loc) }; + + break :emit_temp_var temp_id; + } else data.default_name.ref.?; + + if (p.options.features.server_components.wrapsExports()) { + data.value = .{ .expr = p.wrapValueForServerComponentReference(if (data.value == .expr) data.value.expr else p.newExpr(E.Function{ .func = func.func }, stmt.loc), "default") }; } + try stmts.append(stmt.*); + try p.emitReactRefreshRegister(stmts, name, ref, .default); + } else { if (p.options.features.server_components.wrapsExports()) { data.value = .{ .expr = p.wrapValueForServerComponentReference(p.newExpr(E.Function{ .func = func.func }, stmt.loc), "default") }; } - stmts.append(stmt.*) catch unreachable; + try stmts.append(stmt.*); + } - // if (func.func.name != null and func.func.name.?.ref != null) { - // stmts.append(p.keepStmtSymbolName(func.func.name.?.loc, func.func.name.?.ref.?, name)) catch unreachable; - // } - // prevent doubling export default function name + // if (func.func.name != null and func.func.name.?.ref != null) { + // stmts.append(p.keepStmtSymbolName(func.func.name.?.loc, func.func.name.?.ref.?, name)) catch unreachable; + // } + return; + }, + .s_class => |class| { + _ = p.visitClass(s2.loc, &class.class, data.default_name.ref.?); + + if (p.is_control_flow_dead) return; - }, - .s_class => |class| { - _ = p.visitClass(s2.loc, &class.class, data.default_name.ref.?); - if (p.is_control_flow_dead) - return; - - if (mark_for_replace) { - const entry = p.options.features.replace_exports.getPtr("default").?; - if (entry.* == .replace) { - data.value = .{ .expr = entry.replace }; - } else { - _ = p.injectReplacementExport(stmts, Ref.None, logger.Loc.Empty, entry); - return; - } - } - - if (data.default_name.ref.?.isSourceContentsSlice()) { - data.default_name = createDefaultName(p, stmt.loc) catch unreachable; - } - - // We only inject a name into classes when there is a decorator - if (class.class.has_decorators) { - if (class.class.class_name == null or - class.class.class_name.?.ref == null) - { - class.class.class_name = data.default_name; - } - } - - // This is to handle TS decorators, mostly. - var class_stmts = p.lowerClass(.{ .stmt = s2 }); - bun.assert(class_stmts[0].data == .s_class); - - if (class_stmts.len > 1) { - data.value.stmt = class_stmts[0]; - stmts.append(stmt.*) catch {}; - stmts.appendSlice(class_stmts[1..]) catch {}; + if (mark_for_replace) { + const entry = p.options.features.replace_exports.getPtr("default").?; + if (entry.* == .replace) { + data.value = .{ .expr = entry.replace }; } else { - data.value.stmt = class_stmts[0]; - stmts.append(stmt.*) catch {}; + _ = p.injectReplacementExport(stmts, Ref.None, logger.Loc.Empty, entry); + return; } + } - if (p.options.features.server_components.wrapsExports()) { - data.value = .{ .expr = p.wrapValueForServerComponentReference(p.newExpr(class.class, stmt.loc), "default") }; + if (data.default_name.ref.?.isSourceContentsSlice()) { + data.default_name = createDefaultName(p, stmt.loc) catch unreachable; + } + + // We only inject a name into classes when there is a decorator + if (class.class.has_decorators) { + if (class.class.class_name == null or + class.class.class_name.?.ref == null) + { + class.class.class_name = data.default_name; } + } - return; - }, - else => {}, - } + // This is to handle TS decorators, mostly. + var class_stmts = p.lowerClass(.{ .stmt = s2 }); + bun.assert(class_stmts[0].data == .s_class); + + if (class_stmts.len > 1) { + data.value.stmt = class_stmts[0]; + stmts.append(stmt.*) catch {}; + stmts.appendSlice(class_stmts[1..]) catch {}; + } else { + data.value.stmt = class_stmts[0]; + stmts.append(stmt.*) catch {}; + } + + if (p.options.features.server_components.wrapsExports()) { + data.value = .{ .expr = p.wrapValueForServerComponentReference(p.newExpr(class.class, stmt.loc), "default") }; + } + + return; + }, + else => {}, }, } @@ -19599,27 +19657,19 @@ fn NewParser_( data.kind = kind; try stmts.append(stmt.*); - if (data.is_export and p.options.features.server_components.wrapsExports()) { - for (data.decls.slice()) |*decl| try_annotate: { - const val = decl.value orelse break :try_annotate; - switch (val.data) { - .e_arrow, .e_function => {}, - else => break :try_annotate, - } - const id = switch (decl.binding.data) { - .b_identifier => |id| id.ref, - else => break :try_annotate, - }; - const original_name = p.symbols.items[id.innerIndex()].original_name; - decl.value = p.wrapValueForServerComponentReference(val, original_name); - } - } - if (p.options.features.react_fast_refresh and p.current_scope == p.module_scope) { for (data.decls.slice()) |decl| try_register: { const val = decl.value orelse break :try_register; switch (val.data) { + // Assigning a component to a local. .e_arrow, .e_function => {}, + + // A wrapped component. + .e_call => |call| switch (call.target.data) { + .e_identifier => |id| if (id.ref != p.react_refresh.latest_signature_ref) + break :try_register, + else => break :try_register, + }, else => break :try_register, } const id = switch (decl.binding.data) { @@ -19631,6 +19681,18 @@ fn NewParser_( } } + if (data.is_export and p.options.features.server_components.wrapsExports()) { + for (data.decls.slice()) |*decl| try_annotate: { + const val = decl.value orelse break :try_annotate; + const id = switch (decl.binding.data) { + .b_identifier => |id| id.ref, + else => break :try_annotate, + }; + const original_name = p.symbols.items[id.innerIndex()].original_name; + decl.value = p.wrapValueForServerComponentReference(val, original_name); + } + } + return; } pub fn s_expr(p: *P, stmts: *ListManaged(Stmt), stmt: *Stmt, data: *S.SExpr) !void { @@ -23156,35 +23218,44 @@ fn NewParser_( } }; - pub fn handleReactRefreshRegister(p: *P, stmts: *ListManaged(Stmt), original_name: []const u8, ref: Ref, export_kind: enum { named, default }) !void { + const ReactRefreshExportKind = enum { named, default }; + + pub fn handleReactRefreshRegister(p: *P, stmts: *ListManaged(Stmt), original_name: []const u8, ref: Ref, export_kind: ReactRefreshExportKind) !void { bun.assert(p.options.features.react_fast_refresh); bun.assert(p.current_scope == p.module_scope); if (ReactRefresh.isComponentishName(original_name)) { - // $RefreshReg$(component, "file.ts:Original Name") - const loc = logger.Loc.Empty; - try stmts.append(p.s(S.SExpr{ .value = p.newExpr(E.Call{ - .target = Expr.initIdentifier(p.react_refresh.register_ref, loc), - .args = try ExprNodeList.fromSlice(p.allocator, &.{ - Expr.initIdentifier(ref, loc), - p.newExpr(E.String{ - .data = try bun.strings.concat(p.allocator, &.{ - p.source.path.pretty, - ":", - switch (export_kind) { - .named => original_name, - .default => "default", - }, - }), - }, loc), - }), - }, loc) }, loc)); - - p.recordUsage(ref); - p.react_refresh.register_used = true; + try p.emitReactRefreshRegister(stmts, original_name, ref, export_kind); } } + pub fn emitReactRefreshRegister(p: *P, stmts: *ListManaged(Stmt), original_name: []const u8, ref: Ref, export_kind: ReactRefreshExportKind) !void { + bun.assert(p.options.features.react_fast_refresh); + bun.assert(p.current_scope == p.module_scope); + + // $RefreshReg$(component, "file.ts:Original Name") + const loc = logger.Loc.Empty; + try stmts.append(p.s(S.SExpr{ .value = p.newExpr(E.Call{ + .target = Expr.initIdentifier(p.react_refresh.register_ref, loc), + .args = try ExprNodeList.fromSlice(p.allocator, &.{ + Expr.initIdentifier(ref, loc), + p.newExpr(E.String{ + .data = try bun.strings.concat(p.allocator, &.{ + p.source.path.pretty, + ":", + switch (export_kind) { + .named => original_name, + .default => "default", + }, + }), + }, loc), + }), + }, loc) }, loc)); + + p.recordUsage(ref); + p.react_refresh.register_used = true; + } + pub fn wrapValueForServerComponentReference(p: *P, val: Expr, original_name: []const u8) Expr { bun.assert(p.options.features.server_components.wrapsExports()); bun.assert(p.current_scope == p.module_scope); @@ -23298,6 +23369,7 @@ fn NewParser_( pub fn getReactRefreshHookSignalDecl(p: *P, signal_cb_ref: Ref) Stmt { const loc = logger.Loc.Empty; + p.react_refresh.latest_signature_ref = signal_cb_ref; // var s_ = $RefreshSig$(); return p.s(S.Local{ .decls = G.Decl.List.fromSlice(p.allocator, &.{.{ .binding = p.b(B.Identifier{ .ref = signal_cb_ref }, loc), @@ -24000,6 +24072,12 @@ const ReactRefresh = struct { /// the start of the function, and then add the call to `_s(func, ...)`. hook_ctx_storage: ?*?HookContext = null, + /// This is the most recently generated `_s` call. This is used to compare + /// against seen calls to plain identifiers when in "export default" and in + /// "const Component =" to know if an expression had been wrapped in a hook + /// signature function. + latest_signature_ref: Ref = Ref.None, + pub const HookContext = struct { hasher: std.hash.Wyhash, signature_cb: Ref, @@ -24135,7 +24213,25 @@ pub const ConvertESMExportsForHmr = struct { } // Try to move the export default expression to the end. - if (st.canBeMoved()) { + // TODO: make a function + const can_be_moved_to_inner_scope = switch (st.value) { + .stmt => |s| switch (s.data) { + .s_class => |c| c.class.canBeMoved() and (if (c.class.class_name) |name| + p.symbols.items[name.ref.?.inner_index].use_count_estimate == 0 + else + true), + .s_function => |f| if (f.func.name) |name| + p.symbols.items[name.ref.?.inner_index].use_count_estimate == 0 + else + true, + else => unreachable, + }, + .expr => |e| switch (e.data) { + .e_identifier => true, + else => e.canBeMoved(), + }, + }; + if (can_be_moved_to_inner_scope) { try ctx.export_props.append(p.allocator, .{ .key = Expr.init(E.String, .{ .data = "default" }, stmt.loc), .value = st.value.toExpr(), @@ -24144,26 +24240,41 @@ pub const ConvertESMExportsForHmr = struct { return; } - // Otherwise, a new symbol is needed - const temp_id = p.generateTempRef("default_export"); - try ctx.last_part.declared_symbols.append(p.allocator, .{ .ref = temp_id, .is_top_level = true }); - try ctx.last_part.symbol_uses.putNoClobber(p.allocator, temp_id, .{ .count_estimate = 1 }); - try p.current_scope.generated.push(p.allocator, temp_id); + // Otherwise, an identifier must be exported + switch (st.value) { + .expr => { + const temp_id = p.generateTempRef("default_export"); + try ctx.last_part.declared_symbols.append(p.allocator, .{ .ref = temp_id, .is_top_level = true }); + try ctx.last_part.symbol_uses.putNoClobber(p.allocator, temp_id, .{ .count_estimate = 1 }); + try p.current_scope.generated.push(p.allocator, temp_id); - try ctx.export_props.append(p.allocator, .{ - .key = Expr.init(E.String, .{ .data = "default" }, stmt.loc), - .value = Expr.initIdentifier(temp_id, stmt.loc), - }); + try ctx.export_props.append(p.allocator, .{ + .key = Expr.init(E.String, .{ .data = "default" }, stmt.loc), + .value = Expr.initIdentifier(temp_id, stmt.loc), + }); - break :stmt Stmt.alloc(S.Local, .{ - .kind = .k_const, - .decls = try G.Decl.List.fromSlice(p.allocator, &.{ - .{ - .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp_id }, stmt.loc), - .value = st.value.toExpr(), - }, - }), - }, stmt.loc); + break :stmt Stmt.alloc(S.Local, .{ + .kind = .k_const, + .decls = try G.Decl.List.fromSlice(p.allocator, &.{ + .{ + .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp_id }, stmt.loc), + .value = st.value.toExpr(), + }, + }), + }, stmt.loc); + }, + .stmt => |s| { + try ctx.export_props.append(p.allocator, .{ + .key = Expr.init(E.String, .{ .data = "default" }, stmt.loc), + .value = Expr.initIdentifier(switch (s.data) { + .s_class => |class| class.class.class_name.?.ref.?, + .s_function => |func| func.func.name.?.ref.?, + else => unreachable, + }, stmt.loc), + }); + break :stmt s; + }, + } }, .s_class => |st| stmt: { // Strip the "export" keyword diff --git a/test/bake/client-fixture.mjs b/test/bake/client-fixture.mjs index 664a27625c..cd198a3c1a 100644 --- a/test/bake/client-fixture.mjs +++ b/test/bake/client-fixture.mjs @@ -87,6 +87,7 @@ function createWindow(windowUrl) { error: (...args) => { console.error("[E]", ...args); originalConsole.error(...args); + process.exit(4); }, warn: (...args) => { console.warn("[W]", ...args); diff --git a/test/bake/dev-server-harness.ts b/test/bake/dev-server-harness.ts index 478c379301..53c3ebb79c 100644 --- a/test/bake/dev-server-harness.ts +++ b/test/bake/dev-server-harness.ts @@ -1,4 +1,14 @@ /// +/* Dev server tests can be run with `bun test` or in interactive mode with `bun run test.ts "name filter"` + * + * Env vars: + * + * To run with an out-of-path node.js: + * export BUN_DEV_SERVER_CLIENT_EXECUTABLE="/Users/clo/.local/share/nvm/v22.13.1/bin/node" + * + * To write files to a stable location: + * export BUN_DEV_SERVER_TEST_TEMP="/Users/clo/scratch/dev" + */ import { Bake, BunFile, Subprocess } from "bun"; import fs, { readFileSync, realpathSync } from "node:fs"; import path from "node:path"; @@ -46,13 +56,73 @@ export const reactRefreshStub = { `, }; +/** To test react refresh's registration system */ export const reactAndRefreshStub = { "node_modules/react-refresh/runtime.js": ` - export const performReactRefresh = () => {}; - export const injectIntoGlobalHook = () => {}; + exports.performReactRefresh = () => {}; + exports.injectIntoGlobalHook = () => {}; + exports.register = require("bun-devserver-react-mock").register; + exports.createSignatureFunctionForTransform = require("bun-devserver-react-mock").createSignatureFunctionForTransform; + `, + "node_modules/react/index.js": ` + exports.useState = (y) => [y, x => {}]; + `, + "node_modules/bun-devserver-react-mock/index.js": ` + globalThis.components = new Map(); + globalThis.functionToComponent = new Map(); + exports.expectRegistered = function(fn, filename, exportId) { + const name = filename + ":" + exportId; + try { + if (!components.has(name)) { + for (const [k, v] of components) { + if (v.fn === fn) throw new Error("Component registered under name " + k + " instead of " + name); + } + throw new Error("Component not registered: " + name); + } + if (components.get(name).fn !== fn) throw new Error("Component registered with wrong name: " + name); + } catch (e) { + console.log(components); + throw e; + } + } + exports.hashFromFunction = function(fn) { + if (!keyFromFunction.has(fn)) throw new Error("Function not registered: " + fn); + return keyFromFunction.get(fn).hash; + } + 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"); + const entry = functionToComponent.get(fn) ?? { fn, calls: 0, hash: undefined }; + components.set(name, entry); + functionToComponent.set(fn, entry); + } + exports.createSignatureFunctionForTransform = function(fn) { + let entry = null; + return function(fn, hash) { + if (fn !== undefined) { + entry = functionToComponent.get(fn) ?? { fn, calls: 0, hash: undefined }; + functionToComponent.set(fn, entry); + entry.hash = hash; + entry.calls = 0; + return fn; + } else { + if (!entry) throw new Error("Function not registered"); + entry.calls++; + return entry.fn; + } + } + } `, "node_modules/react/jsx-dev-runtime.js": ` - export const jsxDEV = (tag, props, key) => {}; + export const $$typeof = Symbol.for("react.element"); + export const jsxDEV = (tag, props, key) => ({ + $$typeof, + props, + key, + ref: null, + type: tag, + }); `, }; @@ -156,17 +226,25 @@ export class Dev { baseUrl: string; panicked = false; connectedClients: Set = new Set(); + options: { files: Record }; // These properties are not owned by this class devProcess: Subprocess<"pipe", "pipe", "pipe">; output: OutputLineStream; - constructor(root: string, port: number, process: Subprocess<"pipe", "pipe", "pipe">, stream: OutputLineStream) { + constructor( + root: string, + port: number, + process: Subprocess<"pipe", "pipe", "pipe">, + stream: OutputLineStream, + options: DevServerTest, + ) { this.rootDir = realpathSync(root); this.port = port; this.baseUrl = `http://localhost:${port}`; this.devProcess = process; this.output = stream; + this.options = options as any; this.output.on("panic", () => { this.panicked = true; }); @@ -210,6 +288,19 @@ export class Dev { }); } + read(file: string): string { + return fs.readFileSync(path.join(this.rootDir, file), "utf8"); + } + + /** + * Writes the file back without any changes + * This is useful for triggering file watchers without modifying content + */ + async writeNoChanges(file: string): Promise { + const content = this.read(file); + await this.write(file, content, { dedent: false }); + } + patch( file: string, { @@ -286,8 +377,6 @@ export class Dev { } } -type StepFn = (dev: Dev) => Promise; - export interface Step { run: StepFn; caller: string; @@ -432,7 +521,7 @@ class StylePromise extends Promise> { } } -const node = process.env.DEV_SERVER_CLIENT_EXECUTABLE ?? Bun.which("node"); +const node = process.env.BUN_DEV_SERVER_CLIENT_EXECUTABLE ?? Bun.which("node"); expect(node, "test will fail if this is not node").not.toBe(process.execPath); const danglingProcesses = new Set(); @@ -822,6 +911,20 @@ export class Client extends EventEmitter { } }); } + + async reactRefreshComponentHash(file: string, name: string): Promise { + return withAnnotatedStack(snapshotCallerLocation(), async () => { + const component = await this.js` + const k = ${file} + ":" + ${name}; + const entry = globalThis.components.get(k); + if (!entry) throw new Error("Component not found: " + k); + globalThis.components.delete(k); + globalThis.functionToComponent.delete(entry.fn); + return entry.hash; + `; + return component; + }); + } } function expectProxy(text: Promise, chain: string[], expect: any): any { @@ -968,9 +1071,24 @@ async function withAnnotatedStack(stackLine: string, cb: () => Promise): P } } -const tempDir = fs.mkdtempSync( - path.join(process.platform === "darwin" && !process.env.CI ? "/tmp" : os.tmpdir(), "bun-dev-test-"), -); +const tempDir = + process.env.BUN_DEV_SERVER_TEST_TEMP || + fs.mkdtempSync(path.join(process.platform === "darwin" && !process.env.CI ? "/tmp" : os.tmpdir(), "bun-dev-test-")); + +// Ensure temp directory exists +if (!fs.existsSync(tempDir)) { + fs.mkdirSync(tempDir, { recursive: true }); +} + +function cleanTestDir(dir: string) { + if (!fs.existsSync(dir)) return; + const files = fs.readdirSync(dir); + for (const file of files) { + const filePath = path.join(dir, file); + fs.rmSync(filePath, { recursive: true, force: true }); + } +} + const devTestRoot = path.join(import.meta.dir, "dev").replaceAll("\\", "/"); const counts: Record = {}; @@ -1148,6 +1266,10 @@ export function devTest(description: string, options: T async function run() { const root = path.join(tempDir, basename + count); + + // Clean the test directory if it exists + cleanTestDir(root); + if ("files" in options) { const htmlFiles = Object.keys(options.files).filter(file => file.endsWith(".html")); await writeAll(root, options.files); @@ -1240,7 +1362,7 @@ export function devTest(description: string, options: T using stream = new OutputLineStream("dev", devProcess.stdout, devProcess.stderr); const port = parseInt((await stream.waitForLine(/localhost:(\d+)/))[1], 10); // @ts-expect-error - const dev = new Dev(root, port, devProcess, stream); + const dev = new Dev(root, port, devProcess, stream, options); await maybeWaitInteractive("start"); @@ -1283,12 +1405,15 @@ export function devTest(description: string, options: T return options; } catch { // not in bun test. allow interactive use - const arg = process.argv[2]; + let arg = process.argv.slice(2).join(" ").trim(); + if (arg.startsWith("-t")) { + arg = arg.slice(2).trim(); + } if (!arg) { const mainFile = Bun.$.escape(path.relative(process.cwd(), process.argv[1])); console.error("Options for running Dev Server tests:"); console.error(" - automated: bun test " + mainFile); - console.error(" - interactive: bun " + mainFile + " "); + console.error(" - interactive: bun " + mainFile + " [-t] "); process.exit(1); } if (name.includes(arg)) { diff --git a/test/bake/dev/bundle.test.ts b/test/bake/dev/bundle.test.ts index 83fe5b3525..9c88e536ce 100644 --- a/test/bake/dev/bundle.test.ts +++ b/test/bake/dev/bundle.test.ts @@ -1,5 +1,5 @@ // Bundle tests are tests concerning bundling bugs that only occur in DevServer. -import { dedent } from "bundler/expectBundled"; +import { expect } from "bun:test"; import { devTest, emptyHtmlFile, minimalFramework, reactAndRefreshStub, reactRefreshStub } from "../dev-server-harness"; devTest("import identifier doesnt get renamed", { @@ -88,7 +88,7 @@ devTest("importing a file before it is created", { `, }, async test(dev) { - const c = await dev.client("/", { + await using c = await dev.client("/", { errors: [`index.ts:1:21: error: Could not resolve: "./second"`], }); @@ -99,7 +99,8 @@ devTest("importing a file before it is created", { await c.expectMessage("value: 456"); }, }); -devTest("react refresh - default export function", { +// https://github.com/oven-sh/bun/issues/17447 +devTest("react refresh should register and track hook state", { framework: minimalFramework, files: { ...reactAndRefreshStub, @@ -108,14 +109,283 @@ devTest("react refresh - default export function", { scripts: ["index.tsx"], }), "index.tsx": ` - import { render } from 'bun-devserver-react-mock'; - render(); + import { expectRegistered } from 'bun-devserver-react-mock'; + import App from './App.tsx'; + expectRegistered(App, "App.tsx", "default"); `, "App.tsx": ` export default function App() { + let [a, b] = useState(1); return
Hello, world!
; } `, }, - async test(dev) {}, + async test(dev) { + await using c = await dev.client("/", {}); + const firstHash = await c.reactRefreshComponentHash("App.tsx", "default"); + expect(firstHash).toBeDefined(); + + // hash does not change when hooks stay same + await dev.write( + "App.tsx", + ` + export default function App() { + let [a, b] = useState(1); + return
Hello, world! {a}
; + } + `, + ); + const secondHash = await c.reactRefreshComponentHash("App.tsx", "default"); + expect(secondHash).toEqual(firstHash); + + // hash changes when hooks change + await dev.write( + "App.tsx", + ` + export default function App() { + let [a, b] = useState(2); + return
Hello, world! {a}
; + } + `, + ); + const thirdHash = await c.reactRefreshComponentHash("App.tsx", "default"); + expect(thirdHash).not.toEqual(firstHash); + }, +}); +devTest("react refresh cases", { + framework: minimalFramework, + files: { + ...reactAndRefreshStub, + "index.html": emptyHtmlFile({ + styles: [], + scripts: ["index.tsx"], + }), + "index.tsx": ` + import { expectRegistered } from 'bun-devserver-react-mock'; + + expectRegistered((await import("./default_unnamed")).default, "default_unnamed.tsx", "default"); + expectRegistered((await import("./default_named")).default, "default_named.tsx", "default"); + expectRegistered((await import("./default_arrow")).default, "default_arrow.tsx", "default"); + expectRegistered((await import("./local_var")).LocalVar, "local_var.tsx", "LocalVar"); + expectRegistered((await import("./local_const")).LocalConst, "local_const.tsx", "LocalConst"); + await import("./non_exported"); + + expectRegistered((await import("./default_unnamed_hooks")).default, "default_unnamed_hooks.tsx", "default"); + expectRegistered((await import("./default_named_hooks")).default, "default_named_hooks.tsx", "default"); + expectRegistered((await import("./default_arrow_hooks")).default, "default_arrow_hooks.tsx", "default"); + expectRegistered((await import("./local_var_hooks")).LocalVar, "local_var_hooks.tsx", "LocalVar"); + expectRegistered((await import("./local_const_hooks")).LocalConst, "local_const_hooks.tsx", "LocalConst"); + await import("./non_exported_hooks"); + `, + "default_unnamed.tsx": ` + export default function() { + return
; + } + `, + "default_named.tsx": ` + export default function Hello() { + return
; + } + `, + "default_arrow.tsx": ` + export default () => { + return
; + } + `, + "local_var.tsx": ` + export var LocalVar = () => { + return
; + } + `, + "local_const.tsx": ` + export const LocalConst = () => { + return
; + } + `, + "non_exported.tsx": ` + import { expectRegistered } from 'bun-devserver-react-mock'; + + function NonExportedFunc() { + return
; + } + + const NonExportedVar = () => { + return
; + } + + // Anonymous function with name + const NonExportedAnon = (function MyNamedAnon() { + return
; + }); + + // Anonymous function without name + const NonExportedAnonUnnamed = (function() { + return
; + }); + + expectRegistered(NonExportedFunc, "non_exported.tsx", "NonExportedFunc"); + expectRegistered(NonExportedVar, "non_exported.tsx", "NonExportedVar"); + expectRegistered(NonExportedAnon, "non_exported.tsx", "NonExportedAnon"); + expectRegistered(NonExportedAnonUnnamed, "non_exported.tsx", "NonExportedAnonUnnamed"); + `, + "default_unnamed_hooks.tsx": ` + export default function() { + const [count, setCount] = useState(0); + return
{count}
; + } + `, + "default_named_hooks.tsx": ` + export default function Hello() { + const [count, setCount] = useState(0); + return
{count}
; + } + `, + "default_arrow_hooks.tsx": ` + export default () => { + const [count, setCount] = useState(0); + return
{count}
; + } + `, + "local_var_hooks.tsx": ` + export var LocalVar = () => { + const [count, setCount] = useState(0); + return
{count}
; + } + `, + "local_const_hooks.tsx": ` + export const LocalConst = () => { + const [count, setCount] = useState(0); + return
{count}
; + } + `, + "non_exported_hooks.tsx": ` + import { expectRegistered } from 'bun-devserver-react-mock'; + + function NonExportedFunc() { + const [count, setCount] = useState(0); + return
{count}
; + } + + const NonExportedVar = () => { + const [count, setCount] = useState(0); + return
{count}
; + } + + // Anonymous function with name + const NonExportedAnon = (function MyNamedAnon() { + const [count, setCount] = useState(0); + return
{count}
; + }); + + // Anonymous function without name + const NonExportedAnonUnnamed = (function() { + const [count, setCount] = useState(0); + return
{count}
; + }); + + expectRegistered(NonExportedFunc, "non_exported_hooks.tsx", "NonExportedFunc"); + expectRegistered(NonExportedVar, "non_exported_hooks.tsx", "NonExportedVar"); + expectRegistered(NonExportedAnon, "non_exported_hooks.tsx", "NonExportedAnon"); + expectRegistered(NonExportedAnonUnnamed, "non_exported_hooks.tsx", "NonExportedAnonUnnamed"); + `, + }, + async test(dev) { + await using c = await dev.client("/"); + }, +}); +devTest("default export same-scope handling", { + files: { + ...reactRefreshStub, + "index.html": emptyHtmlFile({ + styles: [], + scripts: ["index.ts", "react-refresh/runtime"], + }), + "index.ts": ` + await import("./fixture1.ts"); + console.log((new ((await import("./fixture2.ts")).default)).a); + await import("./fixture3.ts"); + console.log((new ((await import("./fixture4.ts")).default)).result); + console.log((await import("./fixture5.ts")).default); + console.log((await import("./fixture6.ts")).default); + console.log((await import("./fixture7.ts")).default()); + console.log((await import("./fixture8.ts")).default()); + console.log((await import("./fixture9.ts")).default(false)); + `, + "fixture1.ts": ` + const sideEffect = () => "a"; + export default class A { + [sideEffect()] = "ONE"; + } + console.log(new A().a); + `, + "fixture2.ts": ` + const sideEffect = () => "a"; + export default class A { + [sideEffect()] = "TWO"; + } + `, + "fixture3.ts": ` + export default class A { + result = "THREE" + } + console.log(new A().result); + `, + "fixture4.ts": ` + export default class MOVE { + result = "FOUR" + } + `, + "fixture5.ts": ` + const default_export = "FIVE"; + export default default_export; + `, + "fixture6.ts": ` + const default_export = "S"; + function sideEffect() { + return default_export + "EVEN"; + } + export default sideEffect(); + console.log(default_export + "IX"); + `, + "fixture7.ts": ` + export default function() { return "EIGHT" }; + `, + "fixture8.ts": ` + export default function MOVE() { return "NINE" }; + `, + "fixture9.ts": ` + export default function named(flag = true) { return flag ? "TEN" : "ELEVEN" }; + console.log(named()); + `, + }, + async test(dev) { + await using c = await dev.client("/", { storeHotChunks: true }); + c.expectMessage( + // + "ONE", + "TWO", + "THREE", + "FOUR", + "FIVE", + "SIX", + "SEVEN", + "EIGHT", + "NINE", + "TEN", + "ELEVEN", + ); + + const filesExpectingMove = Object.entries(dev.options.files) + .filter(([, content]) => content.includes("MOVE")) + .map(([path]) => path); + for (const file of filesExpectingMove) { + await dev.writeNoChanges(file); + const chunk = await c.getMostRecentHmrChunk(); + expect(chunk).toMatch(/default:\s*(function|class)\s*MOVE/); + } + + await dev.writeNoChanges("fixture7.ts"); + const chunk = await c.getMostRecentHmrChunk(); + expect(chunk).toMatch(/default:\s*function/); + }, });