From b692c5db60345205dc77533694de83dd9f8f81b0 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Wed, 24 Sep 2025 22:25:56 +0800
Subject: [PATCH 1/5] nonblocking dynamic imports

Allows dynamic imports to be loading asynchronously. I know reddit isnt the
best example, since it doesn't fully load, but this reduced the load time from
~7.2s to ~4.8s.
---
 src/browser/ScriptManager.zig             | 119 +++++++++-
 src/browser/mimalloc.zig                  |   4 +-
 src/browser/page.zig                      |   9 +-
 src/runtime/js.zig                        | 270 +++++++++++++++++-----
 src/tests/html/script/dynamic_import.html |  17 +-
 5 files changed, 342 insertions(+), 77 deletions(-)

diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig
index 0c2e66d6..12f21ca0 100644
--- a/src/browser/ScriptManager.zig
+++ b/src/browser/ScriptManager.zig
@@ -67,6 +67,7 @@ client: *Http.Client,
 allocator: Allocator,
 buffer_pool: BufferPool,
 script_pool: std.heap.MemoryPool(PendingScript),
+async_module_pool: std.heap.MemoryPool(AsyncModule),
 
 const OrderList = std.DoublyLinkedList;
 
@@ -85,6 +86,7 @@ pub fn init(browser: *Browser, page: *Page) ScriptManager {
         .static_scripts_done = false,
         .buffer_pool = BufferPool.init(allocator, 5),
         .script_pool = std.heap.MemoryPool(PendingScript).init(allocator),
+        .async_module_pool = std.heap.MemoryPool(AsyncModule).init(allocator),
     };
 }
 
@@ -92,6 +94,7 @@ pub fn deinit(self: *ScriptManager) void {
     self.reset();
     self.buffer_pool.deinit();
     self.script_pool.deinit();
+    self.async_module_pool.deinit();
 }
 
 pub fn reset(self: *ScriptManager) void {
@@ -256,7 +259,7 @@ pub fn addFromElement(self: *ScriptManager, element: *parser.Element) !void {
 // Unlike external modules which can only ever be executed after releasing an
 // http handle, these are executed without there necessarily being a free handle.
 // Thus, Http/Client.zig maintains a dedicated handle for these calls.
-pub fn blockingGet(self: *ScriptManager, url: [:0]const u8) !BlockingResult {
+pub fn blockingGet(self: *ScriptManager, url: [:0]const u8) !GetResult {
     std.debug.assert(self.is_blocking == false);
 
     self.is_blocking = true;
@@ -302,6 +305,34 @@ pub fn blockingGet(self: *ScriptManager, url: [:0]const u8) !BlockingResult {
     }
 }
 
+pub fn getAsyncModule(self: *ScriptManager, url: [:0]const u8, cb: AsyncModule.Callback, cb_data: *anyopaque) !void {
+    const async = try self.async_module_pool.create();
+    errdefer self.async_module_pool.destroy(async);
+
+    async.* = .{
+        .cb = cb,
+        .manager = self,
+        .cb_data = cb_data,
+    };
+
+    var headers = try self.client.newHeaders();
+    try self.page.requestCookie(.{}).headersForRequest(self.page.arena, url, &headers);
+
+    try self.client.request(.{
+        .url = url,
+        .method = .GET,
+        .headers = headers,
+        .cookie_jar = self.page.cookie_jar,
+        .ctx = async,
+        .resource_type = .script,
+        .start_callback = if (log.enabled(.http, .debug)) AsyncModule.startCallback else null,
+        .header_callback = AsyncModule.headerCallback,
+        .data_callback = AsyncModule.dataCallback,
+        .done_callback = AsyncModule.doneCallback,
+        .error_callback = AsyncModule.errorCallback,
+    });
+}
+
 pub fn staticScriptsDone(self: *ScriptManager) void {
     std.debug.assert(self.static_scripts_done == false);
     self.static_scripts_done = true;
@@ -594,7 +625,7 @@ const Script = struct {
                 .javascript => _ = js_context.eval(content, url) catch break :blk false,
                 .module => {
                     // We don't care about waiting for the evaluation here.
-                    _ = js_context.module(content, url, cacheable) catch break :blk false;
+                    js_context.module(false, content, url, cacheable) catch break :blk false;
                 },
             }
             break :blk true;
@@ -760,7 +791,7 @@ const Blocking = struct {
     const State = union(enum) {
         running: void,
         err: anyerror,
-        done: BlockingResult,
+        done: GetResult,
     };
 
     fn startCallback(transfer: *Http.Transfer) !void {
@@ -814,19 +845,93 @@ const Blocking = struct {
     fn errorCallback(ctx: *anyopaque, err: anyerror) void {
         var self: *Blocking = @ptrCast(@alignCast(ctx));
         self.state = .{ .err = err };
-        self.buffer_pool.release(self.buffer);
+        if (self.buffer.items.len > 0) {
+            self.buffer_pool.release(self.buffer);
+        }
     }
 };
 
-pub const BlockingResult = struct {
+pub const AsyncModule = struct {
+    cb: Callback,
+    cb_data: *anyopaque,
+    manager: *ScriptManager,
+    buffer: std.ArrayListUnmanaged(u8) = .{},
+
+    pub const Callback = *const fn (ptr: *anyopaque, result: anyerror!GetResult) void;
+
+    fn startCallback(transfer: *Http.Transfer) !void {
+        log.debug(.http, "script fetch start", .{ .req = transfer, .async = true });
+    }
+
+    fn headerCallback(transfer: *Http.Transfer) !void {
+        const header = &transfer.response_header.?;
+        log.debug(.http, "script header", .{
+            .req = transfer,
+            .async = true,
+            .status = header.status,
+            .content_type = header.contentType(),
+        });
+
+        if (header.status != 200) {
+            return error.InvalidStatusCode;
+        }
+
+        var self: *AsyncModule = @ptrCast(@alignCast(transfer.ctx));
+        self.buffer = self.manager.buffer_pool.get();
+    }
+
+    fn dataCallback(transfer: *Http.Transfer, data: []const u8) !void {
+        // too verbose
+        // log.debug(.http, "script data chunk", .{
+        //     .req = transfer,
+        //     .blocking = true,
+        // });
+
+        var self: *AsyncModule = @ptrCast(@alignCast(transfer.ctx));
+        self.buffer.appendSlice(self.manager.allocator, data) catch |err| {
+            log.err(.http, "SM.dataCallback", .{
+                .err = err,
+                .len = data.len,
+                .ascyn = true,
+                .transfer = transfer,
+            });
+            return err;
+        };
+    }
+
+    fn doneCallback(ctx: *anyopaque) !void {
+        var self: *AsyncModule = @ptrCast(@alignCast(ctx));
+        defer self.manager.async_module_pool.destroy(self);
+        self.cb(self.cb_data, .{
+            .buffer = self.buffer,
+            .buffer_pool = &self.manager.buffer_pool,
+        });
+    }
+
+    fn errorCallback(ctx: *anyopaque, err: anyerror) void {
+        var self: *AsyncModule = @ptrCast(@alignCast(ctx));
+
+        if (err != error.Abort) {
+            self.cb(self.cb_data, err);
+        }
+
+        if (self.buffer.items.len > 0) {
+            self.manager.buffer_pool.release(self.buffer);
+        }
+
+        self.manager.async_module_pool.destroy(self);
+    }
+};
+
+pub const GetResult = struct {
     buffer: std.ArrayListUnmanaged(u8),
     buffer_pool: *BufferPool,
 
-    pub fn deinit(self: *BlockingResult) void {
+    pub fn deinit(self: *GetResult) void {
         self.buffer_pool.release(self.buffer);
     }
 
-    pub fn src(self: *const BlockingResult) []const u8 {
+    pub fn src(self: *const GetResult) []const u8 {
         return self.buffer.items;
     }
 };
diff --git a/src/browser/mimalloc.zig b/src/browser/mimalloc.zig
index 4e2dea28..d396a642 100644
--- a/src/browser/mimalloc.zig
+++ b/src/browser/mimalloc.zig
@@ -62,8 +62,8 @@ pub fn getRSS() i64 {
     const data = writer.written();
     const index = std.mem.indexOf(u8, data, "rss: ") orelse return -1;
     const sep = std.mem.indexOfScalarPos(u8, data, index + 5, ' ') orelse return -2;
-    const value = std.fmt.parseFloat(f64, data[index+5..sep]) catch return -3;
-    const unit = data[sep+1..];
+    const value = std.fmt.parseFloat(f64, data[index + 5 .. sep]) catch return -3;
+    const unit = data[sep + 1 ..];
     if (std.mem.startsWith(u8, unit, "KiB,")) {
         return @as(i64, @intFromFloat(value)) * 1024;
     }
diff --git a/src/browser/page.zig b/src/browser/page.zig
index 87b42534..99a402d3 100644
--- a/src/browser/page.zig
+++ b/src/browser/page.zig
@@ -255,9 +255,14 @@ pub const Page = struct {
         try Node.prepend(head, &[_]Node.NodeOrText{.{ .node = parser.elementToNode(base) }});
     }
 
-    pub fn fetchModuleSource(ctx: *anyopaque, src: [:0]const u8) !ScriptManager.BlockingResult {
+    pub fn fetchModuleSource(ctx: *anyopaque, url: [:0]const u8) !ScriptManager.GetResult {
         const self: *Page = @ptrCast(@alignCast(ctx));
-        return self.script_manager.blockingGet(src);
+        return self.script_manager.blockingGet(url);
+    }
+
+    pub fn fetchAsyncModuleSource(ctx: *anyopaque, url: [:0]const u8, cb: ScriptManager.AsyncModule.Callback, cb_data: *anyopaque) !void {
+        const self: *Page = @ptrCast(@alignCast(ctx));
+        return self.script_manager.getAsyncModule(url, cb, cb_data);
     }
 
     pub fn wait(self: *Page, wait_ms: i32) Session.WaitResult {
diff --git a/src/runtime/js.zig b/src/runtime/js.zig
index 977c96be..ae3f0cfa 100644
--- a/src/runtime/js.zig
+++ b/src/runtime/js.zig
@@ -22,6 +22,7 @@ const v8 = @import("v8");
 
 const log = @import("../log.zig");
 const SubType = @import("subtype.zig").SubType;
+const ScriptManager = @import("../browser/ScriptManager.zig");
 
 const Allocator = std.mem.Allocator;
 const ArenaAllocator = std.heap.ArenaAllocator;
@@ -540,6 +541,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     .module_loader = .{
                         .ptr = safe_module_loader,
                         .func = ModuleLoader.fetchModuleSource,
+                        .async = ModuleLoader.fetchAsyncModuleSource,
                     },
                     .global_callback = global_callback,
                 };
@@ -707,16 +709,26 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
             const ModuleLoader = struct {
                 ptr: *anyopaque,
-                func: *const fn (ptr: *anyopaque, url: [:0]const u8) anyerror!BlockingResult,
-
-                // Don't like having to reach into ../browser/ here. But can't think
-                // of a good way to fix this.
-                const BlockingResult = @import("../browser/ScriptManager.zig").BlockingResult;
+                func: *const fn (ptr: *anyopaque, url: [:0]const u8) anyerror!ScriptManager.GetResult,
+                async: *const fn (ptr: *anyopaque, url: [:0]const u8, cb: ScriptManager.AsyncModule.Callback, cb_state: *anyopaque) anyerror!void,
             };
 
             const ModuleEntry = struct {
-                module: PersistentModule,
-                promise: PersistentPromise,
+                // Can be null if we're asynchrously loading the module, in
+                // which case resolver_promise cannot be null.
+                module: ?PersistentModule = null,
+
+                // The promise of the evaluting module. The resolved value is
+                // meaningless to us, but the resolver promise needs to chain
+                // to this, since we need to know when it's complete.
+                module_promise: ?PersistentPromise = null,
+
+                // The promise for the resolver which is loading the module.
+                // (AKA, the first time we try to load it). This resolver will
+                // chain to the module_promise  and, when it's done evaluating
+                // will resolve its namespace. Any other attempt to load the
+                // module willchain to this.
+                resolver_promise: ?PersistentPromise = null,
             };
 
             // no init, started with executor.createJsContext()
@@ -751,8 +763,15 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 {
                     var it = self.module_cache.valueIterator();
                     while (it.next()) |entry| {
-                        entry.module.deinit();
-                        entry.promise.deinit();
+                        if (entry.module) |*mod| {
+                            mod.deinit();
+                        }
+                        if (entry.module_promise) |*p| {
+                            p.deinit();
+                        }
+                        if (entry.resolver_promise) |*p| {
+                            p.deinit();
+                        }
                     }
                 }
 
@@ -813,13 +832,16 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 return self.createValue(value);
             }
 
-            // compile and eval a JS module
-            // It returns null if the module is already compiled and in the cache.
-            // It returns a v8.Promise if the module must be evaluated.
-            pub fn module(self: *JsContext, src: []const u8, url: []const u8, cacheable: bool) !ModuleEntry {
+            pub fn module(self: *JsContext, comptime want_result: bool, src: []const u8, url: []const u8, cacheable: bool) !(if (want_result) ModuleEntry else void) {
                 if (cacheable) {
                     if (self.module_cache.get(url)) |entry| {
-                        return entry;
+                        // The dynamic import will creaet an entry withouth the
+                        // module to prevent multiple calls from asynchronously
+                        // loading the same module. If we're here, without the
+                        // module, then it's time to load it.
+                        if (entry.module != null) {
+                            return if (comptime want_result) entry else {};
+                        }
                     }
                 }
                 errdefer _ = self.module_cache.remove(url);
@@ -842,16 +864,44 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 // Must be a promise that gets returned here.
                 std.debug.assert(evaluated.isPromise());
 
-                const entry = ModuleEntry{
-                    .module = PersistentModule.init(self.isolate, m),
-                    .promise = PersistentPromise.init(self.isolate, .{.handle = evaluated.handle}),
-                };
-                if (cacheable) {
-                    try self.module_cache.putNoClobber(arena, owned_url, entry);
-                    try self.module_identifier.put(arena, m.getIdentityHash(), owned_url);
+                if (comptime !want_result) {
+                    // avoid creating a bunch of persisted objects if it isn't
+                    // cachachable and the caller doesn't care about results.
+                    // This is pretty common, i.e. every <script type=module>
+                    // within the html page.
+                    if (!cacheable) {
+                        return;
+                    }
                 }
 
-                return entry;
+                // anyone who cares about the result, should also want it to
+                // be cached
+                std.debug.assert(cacheable);
+
+                const persisted_module = PersistentModule.init(self.isolate, m);
+                const persisted_promise = PersistentPromise.init(self.isolate, .{ .handle = evaluated.handle });
+
+                var gop = try self.module_cache.getOrPut(arena, owned_url);
+                if (gop.found_existing) {
+                    // only way for us to have found an existing entry, is if
+                    // we're asynchronously loading this module
+                    std.debug.assert(gop.value_ptr.module == null);
+                    std.debug.assert(gop.value_ptr.module_promise == null);
+                    std.debug.assert(gop.value_ptr.resolver_promise != null);
+
+                    // keep the resolver promise, it's doing the heavy lifting
+                    // and any other async loads will be chained to it.
+                    gop.value_ptr.module = persisted_module;
+                    gop.value_ptr.module_promise = persisted_promise;
+                } else {
+                    gop.value_ptr.* = ModuleEntry{
+                        .module = persisted_module,
+                        .module_promise = persisted_promise,
+                        .resolver_promise = null,
+                    };
+                }
+                try self.module_identifier.put(arena, m.getIdentityHash(), owned_url);
+                return if (comptime want_result) gop.value_ptr.* else {};
             }
 
             pub fn newArray(self: *JsContext, len: u32) JsObject {
@@ -1590,7 +1640,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 try_catch.init(self);
                 defer try_catch.deinit();
 
-                const entry = self.module(fetch_result.src(), normalized_specifier, true) catch |err| {
+                const entry = self.module(true, fetch_result.src(), normalized_specifier, true) catch |err| {
                     log.warn(.js, "compile resolved module", .{
                         .specifier = specifier,
                         .stack = try_catch.stack(self.call_arena) catch null,
@@ -1600,7 +1650,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     });
                     return null;
                 };
-                return entry.module.handle;
+                // entry.module is always set when returning from self.module()
+                return entry.module.?.handle;
             }
 
             pub fn dynamicModuleCallback(
@@ -1618,22 +1669,22 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 const isolate = self.isolate;
 
                 const resource = jsStringToZig(self.call_arena, .{ .handle = resource_name.? }, isolate) catch |err| {
-                    log.err(.app, "OOM", .{.err = err, .src =  "dynamicModuleCallback1"});
+                    log.err(.app, "OOM", .{ .err = err, .src = "dynamicModuleCallback1" });
                     return @constCast(self.rejectPromise("Out of memory").handle);
                 };
 
                 const specifier = jsStringToZig(self.call_arena, .{ .handle = v8_specifier.? }, isolate) catch |err| {
-                    log.err(.app, "OOM", .{.err = err, .src =  "dynamicModuleCallback2"});
+                    log.err(.app, "OOM", .{ .err = err, .src = "dynamicModuleCallback2" });
                     return @constCast(self.rejectPromise("Out of memory").handle);
                 };
 
                 const normalized_specifier = @import("../url.zig").stitch(
-                    self.call_arena,
+                    self.context_arena, // might need to survive until the module is loaded
                     specifier,
                     resource,
                     .{ .alloc = .if_needed, .null_terminated = true },
                 ) catch |err| {
-                    log.err(.app, "OOM", .{.err = err, .src =  "dynamicModuleCallback3"});
+                    log.err(.app, "OOM", .{ .err = err, .src = "dynamicModuleCallback3" });
                     return @constCast(self.rejectPromise("Out of memory").handle);
                 };
 
@@ -1646,74 +1697,165 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 return @constCast(promise.handle);
             }
 
-            fn _dynamicModuleCallback(self: *JsContext, specifier: [:0]const u8) !v8.Promise {
-                const isolate = self.isolate;
-                const ctx = self.v8_context;
+            // Will get passed to ScriptManager and then passed back to us when
+            // the src of the module is loaded
+            const DynamicModuleResolveState = struct {
+                js_context: *JsContext,
+                specifier: [:0]const u8,
+                resolver: v8.PromiseResolver,
+            };
 
-                const entry = self.module_cache.get(specifier) orelse blk: {
+            fn _dynamicModuleCallback(self: *JsContext, specifier: [:0]const u8) !v8.Promise {
+                const gop = try self.module_cache.getOrPut(self.context_arena, specifier);
+                if (gop.found_existing and gop.value_ptr.resolver_promise != null) {
+                    // This is easy, there's already something responsible
+                    // for loading the module. Maybe it's still loading, maybe
+                    // it's complete. Whatever, we can just return that promise.
+                    return gop.value_ptr.resolver_promise.?.castToPromise();
+                }
+
+                const isolate = self.isolate;
+
+                const persistent_resolver = v8.Persistent(v8.PromiseResolver).init(isolate, v8.PromiseResolver.init(self.v8_context));
+                try self.persisted_promise_resolvers.append(self.context_arena, persistent_resolver);
+                var resolver = persistent_resolver.castToPromiseResolver();
+
+                const state = try self.context_arena.create(DynamicModuleResolveState);
+                state.* = .{
+                    .js_context = self,
+                    .resolver = resolver,
+                    .specifier = specifier,
+                };
+
+                const promise = resolver.getPromise();
+
+                if (!gop.found_existing) {
+                    // this module hasn't been seen before. This is the most
+                    // complicated path.
+
+                    // First, we'll setup a bare entry into our cache. This will
+                    // prevent anyone one else from trying to asychronously load
+                    // it. Instead, they can just return our promise.
+                    gop.value_ptr.* = ModuleEntry{
+                        .module = null,
+                        .module_promise = null,
+                        .resolver_promise = PersistentPromise.init(self.isolate, .{ .handle = promise.handle }),
+                    };
+
+                    // Next, we need to actually load it.
                     const module_loader = self.module_loader;
-                    var fetch_result = try module_loader.func(module_loader.ptr, specifier);
+                    module_loader.async(module_loader.ptr, specifier, dynamicModuleSourceCallback, state) catch |err| {
+                        const error_msg = v8.String.initUtf8(isolate, @errorName(err));
+                        _ = resolver.reject(self.v8_context, error_msg.toValue());
+                    };
+
+                    // For now, we're done. but this will be continued in
+                    // `dynamicModuleSourceCallback`, once the source for the
+                    // moduel is loaded.
+                    return promise;
+                }
+
+                // So we have a module, but no async resolver. This can only
+                // happen if the module was first synchronously loaded (Does that
+                // ever even happen?!) You'd think we cann just return the module
+                // but no, we need to resolve the module namespace, and the
+                // module could still be loading!
+                // We need to do part of what the first case is going to do in
+                // `dynamicModuleSourceCallback`, but we can skip some steps
+                // since the module is alrady loaded,
+
+                // like before, we want to set this up so that if anything else
+                // tries to load this module, it can just return our promise
+                // since we're going to be doing all the work.
+                gop.value_ptr.resolver_promise = PersistentPromise.init(self.isolate, .{ .handle = promise.handle });
+
+                // But we can skip direclty to `resolveDynamicModule` which is
+                // what the above callback will eventually do.
+                self.resolveDynamicModule(state, gop.value_ptr.*);
+                return promise;
+            }
+
+            fn dynamicModuleSourceCallback(ctx: *anyopaque, fetch_result_: anyerror!ScriptManager.GetResult) void {
+                const state: *DynamicModuleResolveState = @ptrCast(@alignCast(ctx));
+                var self = state.js_context;
+
+                var fetch_result = fetch_result_ catch |err| {
+                    const error_msg = v8.String.initUtf8(self.isolate, @errorName(err));
+                    _ = state.resolver.reject(self.v8_context, error_msg.toValue());
+                    return;
+                };
+
+                const module_entry = blk: {
                     defer fetch_result.deinit();
 
                     var try_catch: TryCatch = undefined;
                     try_catch.init(self);
                     defer try_catch.deinit();
 
-                    break :blk self.module(fetch_result.src(), specifier, true) catch {
+                    break :blk self.module(true, fetch_result.src(), state.specifier, true) catch {
                         const ex = try_catch.exception(self.call_arena) catch |err| @errorName(err) orelse "unknown error";
                         log.err(.js, "module compilation failed", .{
-                            .specifier = specifier,
+                            .specifier = state.specifier,
                             .exception = ex,
                             .stack = try_catch.stack(self.call_arena) catch null,
                             .line = try_catch.sourceLineNumber() orelse 0,
                         });
-                        return self.rejectPromise(ex);
+                        const error_msg = v8.String.initUtf8(self.isolate, ex);
+                        _ = state.resolver.reject(self.v8_context, error_msg.toValue());
+                        return;
                     };
                 };
 
-                const EvaluationData = struct {
-                    module: v8.Module,
-                    resolver: v8.PromiseResolver,
-                };
-                const resolver = v8.Persistent(v8.PromiseResolver).init(isolate, v8.PromiseResolver.init(self.v8_context));
-                try self.persisted_promise_resolvers.append(self.context_arena, resolver);
+                self.resolveDynamicModule(state, module_entry);
+            }
 
-                const ev_data = try self.context_arena.create(EvaluationData);
-                ev_data.* = .{
-                    .module = entry.module.castToModule(),
-                    .resolver = resolver.castToPromiseResolver(),
-                };
-                const external = v8.External.init(isolate, @ptrCast(ev_data));
+            fn resolveDynamicModule(self: *JsContext, state: *DynamicModuleResolveState, module_entry: ModuleEntry) void {
+                const ctx = self.v8_context;
+                const external = v8.External.init(self.isolate, @ptrCast(state));
+
+                // we can only be here if the module has been evaluated and if
+                // we have a resolve loading this asynchronously.
+                std.debug.assert(module_entry.module != null);
+                std.debug.assert(module_entry.module_promise != null);
+                std.debug.assert(module_entry.resolver_promise != null);
+
+                // We've gotten the source for the module and are evaluating it.
+                // You might thing we're done, but the module evaluation is
+                // itself asynchronous. We need to chain to the module's own
+                // promise. When the module is evalutated, it resolves to the
+                // last value of the module. But, for module loading, we need to
+                // resolve to the module's namespace.
 
                 const then_callback = v8.Function.initWithData(ctx, struct {
                     pub fn callback(raw_info: ?*const v8.c.FunctionCallbackInfo) callconv(.c) void {
                         const info = v8.FunctionCallbackInfo.initFromV8(raw_info);
-                        const iso = info.getIsolate();
+                        const isolate = info.getIsolate();
 
-                        const data: *EvaluationData = @ptrCast(@alignCast(info.getExternalValue()));
-                        _ = data.resolver.resolve(iso.getCurrentContext(), data.module.getModuleNamespace());
+                        const s: *DynamicModuleResolveState = @ptrCast(@alignCast(info.getExternalValue()));
+                        const js_context = s.js_context;
+                        const me = js_context.module_cache.getPtr(s.specifier).?;
+                        const namespace = me.module.?.castToModule().getModuleNamespace();
+                        _ = s.resolver.resolve(isolate.getCurrentContext(), namespace);
                     }
                 }.callback, external);
 
                 const catch_callback = v8.Function.initWithData(ctx, struct {
                     pub fn callback(raw_info: ?*const v8.c.FunctionCallbackInfo) callconv(.c) void {
                         const info = v8.FunctionCallbackInfo.initFromV8(raw_info);
-                        const iso = info.getIsolate();
-
-                        const data: *EvaluationData = @ptrCast(@alignCast(info.getExternalValue()));
-                        _ = data.resolver.reject(iso.getCurrentContext(), info.getData());
+                        const isolate = info.getIsolate();
+                        const data: *DynamicModuleResolveState = @ptrCast(@alignCast(info.getExternalValue()));
+                        _ = data.resolver.reject(isolate.getCurrentContext(), info.getData());
                     }
                 }.callback, external);
 
-                _ = entry.promise.castToPromise().thenAndCatch(ctx, then_callback, catch_callback) catch |err| {
+                _ = module_entry.module_promise.?.castToPromise().thenAndCatch(ctx, then_callback, catch_callback) catch |err| {
                     log.err(.js, "module evaluation is promise", .{
                         .err = err,
-                        .specifier = specifier,
+                        .specifier = state.specifier,
                     });
-                    return self.rejectPromise("Failed to evaluate promise");
+                    const error_msg = v8.String.initUtf8(self.isolate, "Failed to evaluate promise");
+                    _ = state.resolver.reject(ctx, error_msg.toValue());
                 };
-
-                return ev_data.resolver.getPromise();
             }
 
             // Reverses the mapZigInstanceToJs, making sure that our TaggedAnyOpaque
@@ -3997,11 +4139,11 @@ const NoopInspector = struct {
 };
 
 const ErrorModuleLoader = struct {
-    // Don't like having to reach into ../browser/ here. But can't think
-    // of a good way to fix this.
-    const BlockingResult = @import("../browser/ScriptManager.zig").BlockingResult;
+    pub fn fetchModuleSource(_: *anyopaque, _: [:0]const u8) !ScriptManager.GetResult {
+        return error.NoModuleLoadConfigured;
+    }
 
-    pub fn fetchModuleSource(_: *anyopaque, _: [:0]const u8) !BlockingResult {
+    pub fn fetchAsyncModuleSource(_: *anyopaque, _: [:0]const u8, _: ScriptManager.AsyncModule.Callback, _: *anyopaque) !void {
         return error.NoModuleLoadConfigured;
     }
 };
diff --git a/src/tests/html/script/dynamic_import.html b/src/tests/html/script/dynamic_import.html
index aece7d1c..ddaa19a2 100644
--- a/src/tests/html/script/dynamic_import.html
+++ b/src/tests/html/script/dynamic_import.html
@@ -5,15 +5,28 @@
 <script id=dynamic_import type=module>
   const promise1 = new Promise((resolve) => {
     Promise.all([
+      import('./import.js'),
+      import('./import.js'),
+      import('./import.js'),
+      import('./import.js'),
       import('./import.js'),
       import('./import.js'),
       import('./import2.js'),
+      import('./import.js'),
+      import('./import.js'),
     ]).then(resolve);
   });
 
   testing.async(promise1, (res) => {
+    testing.expectEqual(9, res.length);
     testing.expectEqual('hello', res[0].greeting);
     testing.expectEqual('hello', res[1].greeting);
-    testing.expectEqual('world', res[2].greeting);
-  })
+    testing.expectEqual('hello', res[2].greeting);
+    testing.expectEqual('hello', res[3].greeting);
+    testing.expectEqual('hello', res[4].greeting);
+    testing.expectEqual('hello', res[5].greeting);
+    testing.expectEqual('world', res[6].greeting);
+    testing.expectEqual('hello', res[7].greeting);
+    testing.expectEqual('hello', res[8].greeting);
+  });
 </script>

From 252fd78473ff9f00a407a629d142156c8b45947a Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Wed, 24 Sep 2025 22:44:46 +0800
Subject: [PATCH 2/5] remove duplicate put, add more assertions

---
 src/runtime/js.zig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/runtime/js.zig b/src/runtime/js.zig
index ae3f0cfa..41345260 100644
--- a/src/runtime/js.zig
+++ b/src/runtime/js.zig
@@ -850,6 +850,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
                 const arena = self.context_arena;
                 const owned_url = try arena.dupe(u8, url);
+
                 try self.module_identifier.putNoClobber(arena, m.getIdentityHash(), owned_url);
                 errdefer _ = self.module_identifier.remove(m.getIdentityHash());
 
@@ -900,7 +901,6 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                         .resolver_promise = null,
                     };
                 }
-                try self.module_identifier.put(arena, m.getIdentityHash(), owned_url);
                 return if (comptime want_result) gop.value_ptr.* else {};
             }
 
@@ -1763,6 +1763,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 // We need to do part of what the first case is going to do in
                 // `dynamicModuleSourceCallback`, but we can skip some steps
                 // since the module is alrady loaded,
+                std.debug.assert(gop.value_ptr.module != null);
+                std.debug.assert(gop.value_ptr.module_promise != null);
 
                 // like before, we want to set this up so that if anything else
                 // tries to load this module, it can just return our promise
@@ -1818,6 +1820,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 std.debug.assert(module_entry.module != null);
                 std.debug.assert(module_entry.module_promise != null);
                 std.debug.assert(module_entry.resolver_promise != null);
+                std.debug.assert(self.module_cache.contains(state.specifier));
 
                 // We've gotten the source for the module and are evaluating it.
                 // You might thing we're done, but the module evaluation is

From 6bf2ff91680f089f1227eb09c0c4fdb9d23c1903 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Thu, 25 Sep 2025 13:39:02 +0800
Subject: [PATCH 3/5] Protect against context changing during module
 resolution.

---
 src/runtime/js.zig | 48 +++++++++++++++++++++++++++++++++++-----------
 src/testing.zig    |  4 ++++
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/src/runtime/js.zig b/src/runtime/js.zig
index 41345260..aebfbda6 100644
--- a/src/runtime/js.zig
+++ b/src/runtime/js.zig
@@ -177,6 +177,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
         meta_lookup: [Types.len]TypeMeta,
 
+        context_id: usize,
+
         const Self = @This();
 
         const TYPE_LOOKUP = TypeLookup{};
@@ -222,6 +224,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
             errdefer allocator.destroy(env);
 
             env.* = .{
+                .context_id = 0,
                 .platform = platform,
                 .isolate = isolate,
                 .templates = undefined,
@@ -528,9 +531,12 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                         return error.ConsoleDeleteError;
                     }
                 }
+                const context_id = env.context_id;
+                env.context_id = context_id + 1;
 
                 self.js_context = JsContext{
                     .state = state,
+                    .id = context_id,
                     .isolate = isolate,
                     .v8_context = v8_context,
                     .templates = &env.templates,
@@ -634,6 +640,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
         // Loosely maps to a Browser Page.
         pub const JsContext = struct {
+            id: usize,
             state: State,
             isolate: v8.Isolate,
             // This context is a persistent object. The persistent needs to be recovered and reset.
@@ -1700,6 +1707,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
             // Will get passed to ScriptManager and then passed back to us when
             // the src of the module is loaded
             const DynamicModuleResolveState = struct {
+                // this is what we're trying to get from the module and resolve
+                // on our promise
+                namespace: ?v8.Value,
+                context_id: usize,
                 js_context: *JsContext,
                 specifier: [:0]const u8,
                 resolver: v8.PromiseResolver,
@@ -1725,6 +1736,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     .js_context = self,
                     .resolver = resolver,
                     .specifier = specifier,
+                    .context_id = self.id,
+                    .namespace = null,
                 };
 
                 const promise = resolver.getPromise();
@@ -1817,10 +1830,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
                 // we can only be here if the module has been evaluated and if
                 // we have a resolve loading this asynchronously.
-                std.debug.assert(module_entry.module != null);
                 std.debug.assert(module_entry.module_promise != null);
                 std.debug.assert(module_entry.resolver_promise != null);
                 std.debug.assert(self.module_cache.contains(state.specifier));
+                state.namespace = module_entry.module.?.castToModule().getModuleNamespace();
 
                 // We've gotten the source for the module and are evaluating it.
                 // You might thing we're done, but the module evaluation is
@@ -1831,23 +1844,36 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
                 const then_callback = v8.Function.initWithData(ctx, struct {
                     pub fn callback(raw_info: ?*const v8.c.FunctionCallbackInfo) callconv(.c) void {
-                        const info = v8.FunctionCallbackInfo.initFromV8(raw_info);
-                        const isolate = info.getIsolate();
+                        var info = v8.FunctionCallbackInfo.initFromV8(raw_info);
+                        var caller = Caller(JsContext, State).init(info);
+                        defer caller.deinit();
 
                         const s: *DynamicModuleResolveState = @ptrCast(@alignCast(info.getExternalValue()));
-                        const js_context = s.js_context;
-                        const me = js_context.module_cache.getPtr(s.specifier).?;
-                        const namespace = me.module.?.castToModule().getModuleNamespace();
-                        _ = s.resolver.resolve(isolate.getCurrentContext(), namespace);
+
+                        if (s.context_id != caller.js_context.id) {
+                            // The microtask is tied to the isolate, not the context
+                            // it can be resolved while another context is active
+                            // (Which seems crazy to me). If that happens, then
+                            // another page was loaded and we MUST ignore thise
+                            // (most of the fields in state are not valid)
+                            return;
+                        }
+
+                        _ = s.resolver.resolve(caller.js_context.v8_context, s.namespace.?);
                     }
                 }.callback, external);
 
                 const catch_callback = v8.Function.initWithData(ctx, struct {
                     pub fn callback(raw_info: ?*const v8.c.FunctionCallbackInfo) callconv(.c) void {
-                        const info = v8.FunctionCallbackInfo.initFromV8(raw_info);
-                        const isolate = info.getIsolate();
-                        const data: *DynamicModuleResolveState = @ptrCast(@alignCast(info.getExternalValue()));
-                        _ = data.resolver.reject(isolate.getCurrentContext(), info.getData());
+                        var info = v8.FunctionCallbackInfo.initFromV8(raw_info);
+                        var caller = Caller(JsContext, State).init(info);
+                        defer caller.deinit();
+
+                        const s: *DynamicModuleResolveState = @ptrCast(@alignCast(info.getExternalValue()));
+                        if (s.context_id != caller.js_context.id) {
+                            return;
+                        }
+                        _ = s.resolver.reject(caller.js_context.v8_context, info.getData());
                     }
                 }.callback, external);
 
diff --git a/src/testing.zig b/src/testing.zig
index 5abdd024..17ded5e1 100644
--- a/src/testing.zig
+++ b/src/testing.zig
@@ -403,6 +403,10 @@ pub fn htmlRunner(file: []const u8) !void {
     const url = try std.fmt.allocPrint(arena_allocator, "http://localhost:9582/src/tests/{s}", .{file});
     try page.navigate(url, .{});
     _ = page.wait(2000);
+    // page exits more aggressively in tests. We want to make sure this is called
+    // at lease once.
+    page.session.browser.runMicrotasks();
+    page.session.browser.runMessageLoop();
 
     @import("root").js_runner_duration += std.time.Instant.since(try std.time.Instant.now(), start);
 

From eed10dd1bbb6e98131c68d671b13a4ca3b9d6113 Mon Sep 17 00:00:00 2001
From: Karl Seguin <karlseguin@users.noreply.github.com>
Date: Fri, 26 Sep 2025 10:37:31 +0800
Subject: [PATCH 4/5] Apply suggestions from code review

fix typos

Co-authored-by: Pierre Tachoire <pierre@lightpanda.io>
---
 src/runtime/js.zig | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/runtime/js.zig b/src/runtime/js.zig
index aebfbda6..e66173a2 100644
--- a/src/runtime/js.zig
+++ b/src/runtime/js.zig
@@ -725,7 +725,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 // which case resolver_promise cannot be null.
                 module: ?PersistentModule = null,
 
-                // The promise of the evaluting module. The resolved value is
+                // The promise of the evaluating module. The resolved value is
                 // meaningless to us, but the resolver promise needs to chain
                 // to this, since we need to know when it's complete.
                 module_promise: ?PersistentPromise = null,
@@ -842,7 +842,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
             pub fn module(self: *JsContext, comptime want_result: bool, src: []const u8, url: []const u8, cacheable: bool) !(if (want_result) ModuleEntry else void) {
                 if (cacheable) {
                     if (self.module_cache.get(url)) |entry| {
-                        // The dynamic import will creaet an entry withouth the
+                        // The dynamic import will create an entry without the
                         // module to prevent multiple calls from asynchronously
                         // loading the same module. If we're here, without the
                         // module, then it's time to load it.
@@ -874,7 +874,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
                 if (comptime !want_result) {
                     // avoid creating a bunch of persisted objects if it isn't
-                    // cachachable and the caller doesn't care about results.
+                    // cacheable and the caller doesn't care about results.
                     // This is pretty common, i.e. every <script type=module>
                     // within the html page.
                     if (!cacheable) {
@@ -1836,9 +1836,9 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 state.namespace = module_entry.module.?.castToModule().getModuleNamespace();
 
                 // We've gotten the source for the module and are evaluating it.
-                // You might thing we're done, but the module evaluation is
+                // You might think we're done, but the module evaluation is
                 // itself asynchronous. We need to chain to the module's own
-                // promise. When the module is evalutated, it resolves to the
+                // promise. When the module is evaluated, it resolves to the
                 // last value of the module. But, for module loading, we need to
                 // resolve to the module's namespace.
 
@@ -1854,7 +1854,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                             // The microtask is tied to the isolate, not the context
                             // it can be resolved while another context is active
                             // (Which seems crazy to me). If that happens, then
-                            // another page was loaded and we MUST ignore thise
+                            // another page was loaded and we MUST ignore this
                             // (most of the fields in state are not valid)
                             return;
                         }

From 2aa4b036735c8419cd565e6553680a7a6f912154 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Fri, 26 Sep 2025 15:34:32 +0800
Subject: [PATCH 5/5] try to cleanup persisted references

---
 src/runtime/js.zig | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/runtime/js.zig b/src/runtime/js.zig
index e66173a2..e09e89e5 100644
--- a/src/runtime/js.zig
+++ b/src/runtime/js.zig
@@ -1707,16 +1707,17 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
             // Will get passed to ScriptManager and then passed back to us when
             // the src of the module is loaded
             const DynamicModuleResolveState = struct {
-                // this is what we're trying to get from the module and resolve
-                // on our promise
-                namespace: ?v8.Value,
+                // The module that we're resolving (we'll actually resolve its
+                // namespace)
+                module: ?v8.Module,
                 context_id: usize,
                 js_context: *JsContext,
                 specifier: [:0]const u8,
-                resolver: v8.PromiseResolver,
+                resolver: v8.Persistent(v8.PromiseResolver),
             };
 
             fn _dynamicModuleCallback(self: *JsContext, specifier: [:0]const u8) !v8.Promise {
+                const isolate = self.isolate;
                 const gop = try self.module_cache.getOrPut(self.context_arena, specifier);
                 if (gop.found_existing and gop.value_ptr.resolver_promise != null) {
                     // This is easy, there's already something responsible
@@ -1725,22 +1726,21 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     return gop.value_ptr.resolver_promise.?.castToPromise();
                 }
 
-                const isolate = self.isolate;
-
                 const persistent_resolver = v8.Persistent(v8.PromiseResolver).init(isolate, v8.PromiseResolver.init(self.v8_context));
                 try self.persisted_promise_resolvers.append(self.context_arena, persistent_resolver);
                 var resolver = persistent_resolver.castToPromiseResolver();
 
                 const state = try self.context_arena.create(DynamicModuleResolveState);
                 state.* = .{
+                    .module = null,
                     .js_context = self,
-                    .resolver = resolver,
                     .specifier = specifier,
                     .context_id = self.id,
-                    .namespace = null,
+                    .resolver = persistent_resolver,
                 };
 
-                const promise = resolver.getPromise();
+                const persisted_promise = PersistentPromise.init(self.isolate, resolver.getPromise());
+                const promise = persisted_promise.castToPromise();
 
                 if (!gop.found_existing) {
                     // this module hasn't been seen before. This is the most
@@ -1752,7 +1752,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     gop.value_ptr.* = ModuleEntry{
                         .module = null,
                         .module_promise = null,
-                        .resolver_promise = PersistentPromise.init(self.isolate, .{ .handle = promise.handle }),
+                        .resolver_promise = persisted_promise,
                     };
 
                     // Next, we need to actually load it.
@@ -1782,7 +1782,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 // like before, we want to set this up so that if anything else
                 // tries to load this module, it can just return our promise
                 // since we're going to be doing all the work.
-                gop.value_ptr.resolver_promise = PersistentPromise.init(self.isolate, .{ .handle = promise.handle });
+                gop.value_ptr.resolver_promise = persisted_promise;
 
                 // But we can skip direclty to `resolveDynamicModule` which is
                 // what the above callback will eventually do.
@@ -1796,7 +1796,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
                 var fetch_result = fetch_result_ catch |err| {
                     const error_msg = v8.String.initUtf8(self.isolate, @errorName(err));
-                    _ = state.resolver.reject(self.v8_context, error_msg.toValue());
+                    _ = state.resolver.castToPromiseResolver().reject(self.v8_context, error_msg.toValue());
                     return;
                 };
 
@@ -1816,7 +1816,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                             .line = try_catch.sourceLineNumber() orelse 0,
                         });
                         const error_msg = v8.String.initUtf8(self.isolate, ex);
-                        _ = state.resolver.reject(self.v8_context, error_msg.toValue());
+                        _ = state.resolver.castToPromiseResolver().reject(self.v8_context, error_msg.toValue());
                         return;
                     };
                 };
@@ -1826,6 +1826,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
             fn resolveDynamicModule(self: *JsContext, state: *DynamicModuleResolveState, module_entry: ModuleEntry) void {
                 const ctx = self.v8_context;
+                const isolate = self.isolate;
                 const external = v8.External.init(self.isolate, @ptrCast(state));
 
                 // we can only be here if the module has been evaluated and if
@@ -1833,7 +1834,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 std.debug.assert(module_entry.module_promise != null);
                 std.debug.assert(module_entry.resolver_promise != null);
                 std.debug.assert(self.module_cache.contains(state.specifier));
-                state.namespace = module_entry.module.?.castToModule().getModuleNamespace();
+                state.module = module_entry.module.?.castToModule();
 
                 // We've gotten the source for the module and are evaluating it.
                 // You might think we're done, but the module evaluation is
@@ -1859,7 +1860,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                             return;
                         }
 
-                        _ = s.resolver.resolve(caller.js_context.v8_context, s.namespace.?);
+                        const namespace = s.module.?.getModuleNamespace();
+                        _ = s.resolver.castToPromiseResolver().resolve(caller.js_context.v8_context, namespace);
                     }
                 }.callback, external);
 
@@ -1873,7 +1875,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                         if (s.context_id != caller.js_context.id) {
                             return;
                         }
-                        _ = s.resolver.reject(caller.js_context.v8_context, info.getData());
+                        _ = s.resolver.castToPromiseResolver().reject(caller.js_context.v8_context, info.getData());
                     }
                 }.callback, external);
 
@@ -1882,8 +1884,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                         .err = err,
                         .specifier = state.specifier,
                     });
-                    const error_msg = v8.String.initUtf8(self.isolate, "Failed to evaluate promise");
-                    _ = state.resolver.reject(ctx, error_msg.toValue());
+                    const error_msg = v8.String.initUtf8(isolate, "Failed to evaluate promise");
+                    _ = state.resolver.castToPromiseResolver().reject(ctx, error_msg.toValue());
                 };
             }