From 740cdaba3d030dab39ca16a178c680f1236816e9 Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Sun, 27 Jul 2025 18:31:01 -0700 Subject: [PATCH] - fix catch-all routes not working in dev server - fix crash - fix require bug --- src/bake/DevServer.zig | 2 +- src/bake/hmr-module.ts | 5 ++ src/bun.js/api/server/RequestContext.zig | 21 ++++---- src/bun.js/bindings/BunString.cpp | 37 ++++++++++++++ src/bun.js/bindings/JSValue.zig | 7 +++ src/bun.js/bindings/ZigStackFrame.zig | 3 +- src/js_printer.zig | 3 ++ test/bake/dev/ssg-pages-router.test.ts | 63 ++++++++++++++++++++++++ 8 files changed, 128 insertions(+), 13 deletions(-) diff --git a/src/bake/DevServer.zig b/src/bake/DevServer.zig index 9664eb8858..798471de00 100644 --- a/src/bake/DevServer.zig +++ b/src/bake/DevServer.zig @@ -1232,7 +1232,7 @@ fn onFrameworkRequestWithBundle( const value_str = bun.String.cloneUTF8(param.value); defer value_str.deref(); - obj.put(global, key_str, value_str.toJS(global)); + _ = try obj.putBunStringOneOrArray(global, &key_str, value_str.toJS(global)); } break :blk obj; } else JSValue.null; diff --git a/src/bake/hmr-module.ts b/src/bake/hmr-module.ts index 8c8b666eb3..1e3d6e511c 100644 --- a/src/bake/hmr-module.ts +++ b/src/bake/hmr-module.ts @@ -115,6 +115,11 @@ export class HMRModule { : null; } + // Module Ids are pre-resolved by the bundler + requireResolve(id: Id): Id { + return id; + } + require(id: Id) { try { const mod = loadModuleSync(id, true, this); diff --git a/src/bun.js/api/server/RequestContext.zig b/src/bun.js/api/server/RequestContext.zig index 17bccf6d09..98172cad92 100644 --- a/src/bun.js/api/server/RequestContext.zig +++ b/src/bun.js/api/server/RequestContext.zig @@ -337,6 +337,7 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, pub fn renderDefaultError( this: *RequestContext, + arena_allocator: std.mem.Allocator, log: *logger.Log, err: anyerror, exceptions: []Api.JsException, @@ -351,12 +352,10 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, } } - const allocator = this.allocator; - - const fallback_container = allocator.create(Api.FallbackMessageContainer) catch unreachable; - defer allocator.destroy(fallback_container); + const fallback_container = arena_allocator.create(Api.FallbackMessageContainer) catch unreachable; + defer arena_allocator.destroy(fallback_container); fallback_container.* = Api.FallbackMessageContainer{ - .message = std.fmt.allocPrint(allocator, comptime Output.prettyFmt(fmt, false), args) catch unreachable, + .message = std.fmt.allocPrint(arena_allocator, comptime Output.prettyFmt(fmt, false), args) catch unreachable, .router = null, .reason = .fetch_event_handler, .cwd = VirtualMachine.get().transpiler.fs.top_level_dir, @@ -364,18 +363,19 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, .code = @as(u16, @truncate(@intFromError(err))), .name = @errorName(err), .exceptions = exceptions, - .build = log.toAPI(allocator) catch unreachable, + .build = log.toAPI(arena_allocator) catch unreachable, }, }; if (comptime fmt.len > 0) Output.prettyErrorln(fmt, args); Output.flush(); - var bb = std.ArrayList(u8).init(allocator); + // Explicitly use `this.allocator` and *not* the arena + var bb = std.ArrayList(u8).init(this.allocator); const bb_writer = bb.writer(); Fallback.renderBackend( - allocator, + arena_allocator, fallback_container, @TypeOf(bb_writer), bb_writer, @@ -1943,7 +1943,7 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, var vm: *jsc.VirtualMachine = this.server.?.vm; const globalThis = this.server.?.globalThis; if (comptime debug_mode) { - var arena = std.heap.ArenaAllocator.init(bun.default_allocator); + var arena = std.heap.ArenaAllocator.init(this.allocator); defer arena.deinit(); const allocator = arena.allocator(); var exception_list: std.ArrayList(Api.JsException) = std.ArrayList(Api.JsException).init(allocator); @@ -1954,9 +1954,10 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, vm.onUnhandledRejectionExceptionList = prev_exception_list; this.renderDefaultError( + allocator, vm.log, error.ExceptionOcurred, - exception_list.toOwnedSlice() catch @panic("TODO"), + exception_list.items, "{s} - {} failed", .{ @as(string, @tagName(this.method)), this.ensurePathname() }, ); diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp index e35e456dc0..396eef817d 100644 --- a/src/bun.js/bindings/BunString.cpp +++ b/src/bun.js/bindings/BunString.cpp @@ -744,6 +744,43 @@ extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::Stri str->hash(); } +extern "C" JSC::EncodedJSValue JSC__JSValue__upsertBunStringArray( + JSC::EncodedJSValue encodedTarget, + JSC::JSGlobalObject* global, + const BunString* key, + JSC::EncodedJSValue encodedValue) +{ + auto scope = DECLARE_THROW_SCOPE(global->vm()); + JSC::JSObject* target = JSC::JSValue::decode(encodedTarget).getObject(); + RETURN_IF_EXCEPTION(scope, {}); + JSC::JSValue newValue = JSC::JSValue::decode(encodedValue); + auto& vm = global->vm(); + WTF::String str = key->tag == BunStringTag::Empty ? WTF::emptyString() : key->toWTFString(); + Identifier id = Identifier::fromString(vm, str); + auto existingValue = target->getIfPropertyExists(global, id); + RETURN_IF_EXCEPTION(scope, {}); + + if (!existingValue.isEmpty()) { + // If existing value is already an array, push to it + if (existingValue.isObject() && existingValue.getObject()->inherits()) { + JSC::JSArray* array = jsCast(existingValue.getObject()); + array->push(global, newValue); + } else { + // Create new array with both values + JSC::JSArray* array = JSC::constructEmptyArray(global, nullptr, 2); + array->putDirectIndex(global, 0, existingValue); + array->putDirectIndex(global, 1, newValue); + target->putDirect(vm, id, array, 0); + } + } else { + // No existing value, just put the new value directly + target->putDirect(vm, id, newValue, 0); + } + + RETURN_IF_EXCEPTION(scope, {}); + return JSC::JSValue::encode(JSC::jsUndefined()); +} + extern "C" void JSC__JSValue__putBunString( JSC::EncodedJSValue encodedTarget, JSC::JSGlobalObject* global, diff --git a/src/bun.js/bindings/JSValue.zig b/src/bun.js/bindings/JSValue.zig index c3d3ad518a..039270668f 100644 --- a/src/bun.js/bindings/JSValue.zig +++ b/src/bun.js/bindings/JSValue.zig @@ -317,6 +317,13 @@ pub const JSValue = enum(i64) { JSC__JSValue__putBunString(value, global, key, result); } + extern "c" fn JSC__JSValue__upsertBunStringArray(value: JSValue, global: *JSGlobalObject, key: *const bun.String, result: jsc.JSValue) JSValue; + + /// Put key/val pair into `obj`. If `key` is already present on the object, create an array for the values. + pub fn putBunStringOneOrArray(obj: JSValue, global: *JSGlobalObject, key: *const bun.String, value: jsc.JSValue) bun.JSError!JSValue { + return fromJSHostCall(global, @src(), JSC__JSValue__upsertBunStringArray, .{ obj, global, key, value }); + } + pub fn put(value: JSValue, global: *JSGlobalObject, key: anytype, result: jsc.JSValue) void { const Key = @TypeOf(key); if (comptime @typeInfo(Key) == .pointer) { diff --git a/src/bun.js/bindings/ZigStackFrame.zig b/src/bun.js/bindings/ZigStackFrame.zig index 0edf8479c8..8b9a153d13 100644 --- a/src/bun.js/bindings/ZigStackFrame.zig +++ b/src/bun.js/bindings/ZigStackFrame.zig @@ -19,8 +19,7 @@ pub const ZigStackFrame = extern struct { var frame: api.StackFrame = comptime std.mem.zeroes(api.StackFrame); if (!this.function_name.isEmpty()) { var slicer = this.function_name.toUTF8(allocator); - defer slicer.deinit(); - frame.function_name = try allocator.dupe(u8, slicer.slice()); + frame.function_name = (try slicer.cloneIfNeeded(allocator)).slice(); } if (!this.source_url.isEmpty()) { diff --git a/src/js_printer.zig b/src/js_printer.zig index 5d304f8def..738ca549ed 100644 --- a/src/js_printer.zig +++ b/src/js_printer.zig @@ -2330,6 +2330,9 @@ fn NewPrinter( if (p.options.require_ref) |require_ref| { p.printSymbol(require_ref); p.print(".resolve"); + } else if (p.options.module_type == .internal_bake_dev) { + p.printSymbol(p.options.hmr_ref); + p.print(".requireResolve"); } else { p.print("require.resolve"); } diff --git a/test/bake/dev/ssg-pages-router.test.ts b/test/bake/dev/ssg-pages-router.test.ts index ff592b4d68..f8b2fbe873 100644 --- a/test/bake/dev/ssg-pages-router.test.ts +++ b/test/bake/dev/ssg-pages-router.test.ts @@ -324,3 +324,66 @@ export { Markdoc as default };`, expect(await c1.elemText("h1")).toBe("Welcome to SSG"); }, }); + +devTest("SSG pages router - catch-all routes [...slug]", { + framework: "react", + files: { + "pages/[...slug].tsx": ` + const CatchAllPage: Bun.SSGPage = ({ params }) => { + return ( +
+

Catch-all Route

+

{JSON.stringify(params)}

+
    + {params.slug && Array.isArray(params.slug) ? ( + params.slug.map((segment, index) => ( +
  • {segment}
    • + )) + ) : ( +
  • No slug array
    • + )} +
+
+ ); + }; + + export default CatchAllPage; + + export const getStaticPaths: Bun.GetStaticPaths = async () => { + return { + paths: [ + { params: { slug: ["docs"] } }, + { params: { slug: ["docs", "getting-started"] } }, + { params: { slug: ["docs", "api", "reference"] } }, + { params: { slug: ["blog", "2024", "january", "new-features"] } }, + ], + }; + }; + `, + }, + async test(dev) { + // Test single segment + await using c1 = await dev.client("/docs"); + expect(await c1.elemText("h1")).toBe("Catch-all Route"); + expect(await c1.elemText("#params")).toBe('{"slug":"docs"}'); + expect(await c1.elemsText("li")).toEqual(["No slug array"]); + + // Test two segments + await using c2 = await dev.client("/docs/getting-started"); + expect(await c2.elemText("h1")).toBe("Catch-all Route"); + expect(await c2.elemText("#params")).toBe('{"slug":["docs","getting-started"]}'); + expect(await c2.elemsText("li")).toEqual(["docs", "getting-started"]); + + // Test three segments + await using c3 = await dev.client("/docs/api/reference"); + expect(await c3.elemText("h1")).toBe("Catch-all Route"); + expect(await c3.elemText("#params")).toBe('{"slug":["docs","api","reference"]}'); + expect(await c3.elemsText("li")).toEqual(["docs", "api", "reference"]); + + // Test four segments + await using c4 = await dev.client("/blog/2024/january/new-features"); + expect(await c4.elemText("h1")).toBe("Catch-all Route"); + expect(await c4.elemText("#params")).toBe('{"slug":["blog","2024","january","new-features"]}'); + expect(await c4.elemsText("li")).toEqual(["blog", "2024", "january", "new-features"]); + }, +});