diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 119dbf23..6330ea1f 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -322,7 +322,25 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local if (cacheable) { gop = try self.module_cache.getOrPut(arena, url); if (gop.found_existing) { - if (gop.value_ptr.module != null) { + if (gop.value_ptr.module) |cache_mod| { + if (gop.value_ptr.module_promise == null) { + // This an usual case, but it can happen if a module is + // first asynchronously requested and then synchronously + // requested as a child of some root import. In that case, + // the module may not be instantiated yet (so we have to + // do that). It might not be evaluated yet. So we have + // to do that too. Evaluation is particularly important + // as it sets up our cache entry's module_promise. + // It appears that v8 handles potential double-instantiated + // and double-evaluated modules safely. The 2nd instantiation + // is a no-op, and the second evaluation returns the same + // promise. + const mod = local.toLocal(cache_mod); + if (mod.getStatus() == .kUninstantiated and try mod.instantiate(resolveModuleCallback) == false) { + return error.ModuleInstantiationError; + } + return self.evaluateModule(want_result, mod, url, true); + } return if (comptime want_result) gop.value_ptr.* else {}; } } else { @@ -352,6 +370,10 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local return error.ModuleInstantiationError; } + return self.evaluateModule(want_result, mod, owned_url, cacheable); +} + +fn evaluateModule(self: *Context, comptime want_result: bool, mod: js.Module, url: []const u8, cacheable: bool) !(if (want_result) ModuleEntry else void) { const evaluated = mod.evaluate() catch { if (comptime IS_DEBUG) { std.debug.assert(mod.getStatus() == .kErrored); @@ -365,7 +387,7 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local }; log.warn(.js, "evaluate module", .{ .message = message, - .specifier = owned_url, + .specifier = url, }); return error.EvaluationError; }; @@ -374,24 +396,15 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local // Must be a promise that gets returned here. lp.assert(evaluated.isPromise(), "Context.module non-promise", .{}); - if (comptime !want_result) { - // avoid creating a bunch of persisted objects if it isn't - // cacheable and the caller doesn't care about results. - // This is pretty common, i.e. every