Skip to content

Commit bbafb04

Browse files
karlseguinkarlseguin
authored
Merge pull request #798 from lightpanda-io/module_loading
Fix module loading
2 parents 9fc2fa5 + d8ec503 commit bbafb04

File tree

2 files changed

+115
-114
lines changed

2 files changed

+115
-114
lines changed

‎src/browser/page.zig‎

Lines changed: 29 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,6 @@ pub const Page = struct {
8787
// execute any JavaScript
8888
main_context: *Env.JsContext,
8989

90-
// List of modules currently fetched/loaded.
91-
module_map: std.StringHashMapUnmanaged([]const u8),
92-
93-
// current_script is the script currently evaluated by the page.
94-
// current_script could by fetch module to resolve module's url to fetch.
95-
current_script: ?*const Script = null,
96-
9790
// indicates intention to navigate to another page on the next loop execution.
9891
delayed_navigation: bool = false,
9992

@@ -119,7 +112,6 @@ pub const Page = struct {
119112
.notification = browser.notification,
120113
}),
121114
.main_context = undefined,
122-
.module_map = .empty,
123115
};
124116
self.main_context = try session.executor.createJsContext(&self.window, self, self, true);
125117

@@ -147,34 +139,9 @@ pub const Page = struct {
147139
try Dump.writeHTML(doc, out);
148140
}
149141

150-
pub fn fetchModuleSource(ctx: *anyopaque, specifier: []const u8) !?[]const u8 {
142+
pub fn fetchModuleSource(ctx: *anyopaque, src: []const u8) !?[]const u8 {
151143
const self: *Page = @ptrCast(@alignCast(ctx));
152-
const base = if (self.current_script) |s| s.src else null;
153-
154-
const src = blk: {
155-
if (base) |_base| {
156-
break :blk try URL.stitch(self.arena, specifier, _base, .{});
157-
} else break :blk specifier;
158-
};
159-
160-
if (self.module_map.get(src)) |module| {
161-
log.debug(.http, "fetching module", .{
162-
.src = src,
163-
.cached = true,
164-
});
165-
return module;
166-
}
167-
168-
log.debug(.http, "fetching module", .{
169-
.src = src,
170-
.base = base,
171-
.cached = false,
172-
.specifier = specifier,
173-
});
174-
175-
const module = try self.fetchData(specifier, base);
176-
if (module) |_module| try self.module_map.putNoClobber(self.arena, src, _module);
177-
return module;
144+
return self.fetchData("module", src);
178145
}
179146

180147
pub fn wait(self: *Page) !void {
@@ -473,26 +440,20 @@ pub const Page = struct {
473440
log.err(.browser, "clear document script", .{ .err = err });
474441
};
475442

476-
var script_source: ?[]const u8 = null;
477-
defer self.current_script = null;
478-
if (script.src) |src| {
479-
self.current_script = script;
480-
481-
// https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
482-
script_source = (try self.fetchData(src, null)) orelse {
483-
// TODO If el's result is null, then fire an event named error at
484-
// el, and return
485-
return;
486-
};
487-
} else {
443+
const src = script.src orelse {
488444
// source is inline
489445
// TODO handle charset attribute
490-
script_source = try parser.nodeTextContent(parser.elementToNode(script.element));
491-
}
446+
const script_source = try parser.nodeTextContent(parser.elementToNode(script.element)) orelse return;
447+
return script.eval(self, script_source);
448+
};
492449

493-
if (script_source) |ss| {
494-
try script.eval(self, ss);
495-
}
450+
// https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
451+
const script_source = (try self.fetchData("script", src)) orelse {
452+
// TODO If el's result is null, then fire an event named error at
453+
// el, and return
454+
return;
455+
};
456+
return script.eval(self, script_source);
496457

497458
// TODO If el's from an external file is true, then fire an event
498459
// named load at el.
@@ -502,34 +463,32 @@ pub const Page = struct {
502463
// It resolves src using the page's uri.
503464
// If a base path is given, src is resolved according to the base first.
504465
// the caller owns the returned string
505-
fn fetchData(self: *const Page, src: []const u8, base: ?[]const u8) !?[]const u8 {
466+
fn fetchData(
467+
self: *const Page,
468+
comptime reason: []const u8,
469+
src: []const u8,
470+
) !?[]const u8 {
506471
const arena = self.arena;
507472

508473
// Handle data URIs.
509474
if (try DataURI.parse(arena, src)) |data_uri| {
510475
return data_uri.data;
511476
}
512477

513-
var res_src = src;
514-
515-
// if a base path is given, we resolve src using base.
516-
if (base) |_base| {
517-
res_src = try URL.stitch(arena, src, _base, .{ .alloc = .if_needed });
518-
}
519-
520478
var origin_url = &self.url;
521-
const url = try origin_url.resolve(arena, res_src);
479+
const url = try origin_url.resolve(arena, src);
522480

523481
var status_code: u16 = 0;
524482
log.debug(.http, "fetching script", .{
525483
.url = url,
526484
.src = src,
527-
.base = base,
485+
.reason = reason,
528486
});
529487

530488
errdefer |err| log.err(.http, "fetch error", .{
531489
.err = err,
532490
.url = url,
491+
.reason = reason,
533492
.status = status_code,
534493
});
535494

@@ -563,6 +522,7 @@ pub const Page = struct {
563522

564523
log.info(.http, "fetch complete", .{
565524
.url = url,
525+
.reason = reason,
566526
.status = status_code,
567527
.content_length = arr.items.len,
568528
});
@@ -1025,25 +985,16 @@ const Script = struct {
1025985
try_catch.init(page.main_context);
1026986
defer try_catch.deinit();
1027987

1028-
const src = self.src orelse "inline";
988+
const src = self.src orelse page.url.raw;
1029989

1030990
log.debug(.browser, "executing script", .{ .src = src, .kind = self.kind });
1031991

1032-
_ = switch (self.kind) {
1033-
.javascript => page.main_context.exec(body, src),
1034-
.module => blk: {
1035-
switch (try page.main_context.module(body, src)) {
1036-
.value => |v| break :blk v,
1037-
.exception => |e| {
1038-
log.warn(.user_script, "eval module", .{
1039-
.src = src,
1040-
.err = try e.exception(page.arena),
1041-
});
1042-
return error.JsErr;
1043-
},
1044-
}
1045-
},
1046-
} catch {
992+
const result = switch (self.kind) {
993+
.javascript => page.main_context.eval(body, src),
994+
.module => page.main_context.module(body, src),
995+
};
996+
997+
result catch {
1047998
if (page.delayed_navigation) {
1048999
return error.Terminated;
10491000
}

‎src/runtime/js.zig‎

Lines changed: 86 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
515515
};
516516

517517
const PersistentObject = v8.Persistent(v8.Object);
518+
const PersistentModule = v8.Persistent(v8.Module);
518519
const PersistentFunction = v8.Persistent(v8.Function);
519520

520521
// Loosely maps to a Browser Page.
@@ -572,6 +573,16 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
572573
// Some Zig types have code to execute when the call scope ends
573574
call_scope_end_callbacks: std.ArrayListUnmanaged(CallScopeEndCallback) = .empty,
574575

576+
// Our module cache: normalized module specifier => module.
577+
module_cache: std.StringHashMapUnmanaged(PersistentModule) = .empty,
578+
579+
// Module => Path. The key is the module hashcode (module.getIdentityHash)
580+
// and the value is the full path to the module. We need to capture this
581+
// so that when we're asked to resolve a dependent module, and all we're
582+
// given is the specifier, we can form the full path. The full path is
583+
// necessary to lookup/store the dependent module in the module_cache.
584+
module_identifier: std.AutoHashMapUnmanaged(u32, []const u8) = .empty,
585+
575586
const ModuleLoader = struct {
576587
ptr: *anyopaque,
577588
func: *const fn (ptr: *anyopaque, specifier: []const u8) anyerror!?[]const u8,
@@ -605,6 +616,13 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
605616
}
606617
}
607618

619+
{
620+
var it = self.module_cache.valueIterator();
621+
while (it.next()) |p| {
622+
p.deinit();
623+
}
624+
}
625+
608626
for (self.callbacks.items) |*cb| {
609627
cb.deinit();
610628
}
@@ -646,6 +664,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
646664
}
647665

648666
// Executes the src
667+
pub fn eval(self: *JsContext, src: []const u8, name: ?[]const u8) !void {
668+
_ = try self.exec(src, name);
669+
}
670+
649671
pub fn exec(self: *JsContext, src: []const u8, name: ?[]const u8) !Value {
650672
const isolate = self.isolate;
651673
const v8_context = self.v8_context;
@@ -669,25 +691,31 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
669691

670692
// compile and eval a JS module
671693
// It doesn't wait for callbacks execution
672-
pub fn module(self: *JsContext, src: []const u8, name: []const u8) !union(enum) { value: Value, exception: Exception } {
673-
const v8_context = self.v8_context;
674-
const m = try compileModule(self.isolate, src, name);
694+
pub fn module(self: *JsContext, src: []const u8, url: []const u8) !void {
695+
const arena = self.context_arena;
675696

676-
// instantiate
677-
// resolveModuleCallback loads module's dependencies.
678-
const ok = m.instantiate(v8_context, resolveModuleCallback) catch {
679-
return error.ExecutionError;
680-
};
697+
const gop = try self.module_cache.getOrPut(arena, url);
698+
if (gop.found_existing) {
699+
return;
700+
}
701+
errdefer _ = self.module_cache.remove(url);
702+
703+
const m = try compileModule(self.isolate, src, url);
704+
705+
const owned_url = try arena.dupe(u8, url);
706+
try self.module_identifier.putNoClobber(arena, m.getIdentityHash(), owned_url);
707+
errdefer _ = self.module_identifier.remove(m.getIdentityHash());
708+
709+
gop.key_ptr.* = owned_url;
710+
gop.value_ptr.* = PersistentModule.init(self.isolate, m);
681711

682-
if (!ok) {
712+
// resolveModuleCallback loads module's dependencies.
713+
const v8_context = self.v8_context;
714+
if (try m.instantiate(v8_context, resolveModuleCallback) == false) {
683715
return error.ModuleInstantiationError;
684716
}
685717

686-
// evaluate
687-
const value = m.evaluate(v8_context) catch {
688-
return .{ .exception = self.createException(m.getException()) };
689-
};
690-
return .{ .value = self.createValue(value) };
718+
_ = try m.evaluate(v8_context);
691719
}
692720

693721
// Wrap a v8.Exception
@@ -1234,52 +1262,74 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
12341262
c_context: ?*const v8.C_Context,
12351263
c_specifier: ?*const v8.C_String,
12361264
import_attributes: ?*const v8.C_FixedArray,
1237-
referrer: ?*const v8.C_Module,
1265+
c_referrer: ?*const v8.C_Module,
12381266
) callconv(.C) ?*const v8.C_Module {
12391267
_ = import_attributes;
1240-
_ = referrer;
12411268

1242-
std.debug.assert(c_context != null);
12431269
const v8_context = v8.Context{ .handle = c_context.? };
1244-
12451270
const self: *JsContext = @ptrFromInt(v8_context.getEmbedderData(1).castTo(v8.BigInt).getUint64());
12461271

1247-
// build the specifier value.
1248-
const specifier = valueToString(
1249-
self.call_arena,
1250-
.{ .handle = c_specifier.? },
1251-
self.isolate,
1252-
v8_context,
1253-
) catch |e| {
1254-
log.err(.js, "resolve module specifier", .{ .err = e });
1272+
const specifier = jsStringToZig(self.call_arena, .{ .handle = c_specifier.? }, self.isolate) catch |err| {
1273+
log.err(.js, "resolve module", .{ .err = err });
12551274
return null;
12561275
};
1276+
const referrer = v8.Module{ .handle = c_referrer.? };
12571277

1258-
// not currently needed
1259-
// const referrer_module = if (referrer) |ref| v8.Module{ .handle = ref } else null;
1260-
const module_loader = self.module_loader;
1261-
const source = module_loader.func(module_loader.ptr, specifier) catch |err| {
1262-
log.err(.js, "resolve module fetch", .{
1278+
return self._resolveModuleCallback(referrer, specifier) catch |err| {
1279+
log.err(.js, "resolve module", .{
12631280
.err = err,
12641281
.specifier = specifier,
12651282
});
12661283
return null;
1267-
} orelse return null;
1284+
};
1285+
}
1286+
1287+
fn _resolveModuleCallback(
1288+
self: *JsContext,
1289+
referrer: v8.Module,
1290+
specifier: []const u8,
1291+
) !?*const v8.C_Module {
1292+
const referrer_path = self.module_identifier.get(referrer.getIdentityHash()) orelse {
1293+
// Shouldn't be possible.
1294+
return error.UnknownModuleReferrer;
1295+
};
1296+
1297+
const normalized_specifier = try @import("../url.zig").stitch(
1298+
self.call_arena,
1299+
specifier,
1300+
referrer_path,
1301+
.{ .alloc = .if_needed },
1302+
);
1303+
1304+
if (self.module_cache.get(normalized_specifier)) |pm| {
1305+
return pm.handle;
1306+
}
1307+
1308+
const module_loader = self.module_loader;
1309+
const source = try module_loader.func(module_loader.ptr, normalized_specifier) orelse return null;
12681310

12691311
var try_catch: TryCatch = undefined;
12701312
try_catch.init(self);
12711313
defer try_catch.deinit();
12721314

12731315
const m = compileModule(self.isolate, source, specifier) catch |err| {
1274-
log.err(.js, "resolve module compile", .{
1316+
log.warn(.js, "compile resolved module", .{
12751317
.specifier = specifier,
1276-
.stack = try_catch.stack(self.context_arena) catch null,
1277-
.src = try_catch.sourceLine(self.context_arena) catch "err",
1318+
.stack = try_catch.stack(self.call_arena) catch null,
1319+
.src = try_catch.sourceLine(self.call_arena) catch "err",
12781320
.line = try_catch.sourceLineNumber() orelse 0,
1279-
.exception = (try_catch.exception(self.context_arena) catch @errorName(err)) orelse @errorName(err),
1321+
.exception = (try_catch.exception(self.call_arena) catch @errorName(err)) orelse @errorName(err),
12801322
});
12811323
return null;
12821324
};
1325+
1326+
// We were hoping to find the module in our cache, and thus used
1327+
// the short-lived call_arena to create the normalized_specifier.
1328+
// But now this'll live for the lifetime of the context.
1329+
const arena = self.context_arena;
1330+
const owned_specifier = try arena.dupe(u8, normalized_specifier);
1331+
try self.module_cache.put(arena, owned_specifier, PersistentModule.init(self.isolate, m));
1332+
try self.module_identifier.putNoClobber(arena, m.getIdentityHash(), owned_specifier);
12831333
return m.handle;
12841334
}
12851335
};

0 commit comments

Comments
 (0)