Support for js module scripts

[utay]

It seems like Lightpanda doesn't currently support js module scripts:

browser/src/browser/browser.zig

Lines 603 to 606 in cb356ff

	fn isJS(stype: ?[]const u8) bool {
	if (stype == null or stype.?.len == 0) return true;
	if (std.mem.eql(u8, stype.?, "application/javascript")) return true;
	if (!std.mem.eql(u8, stype.?, "module")) return true;

I tried to invert this condition but then I got a syntax error at evaluation time so I guess it's here for a good reason:

info(browser): eval remote /assets/index-Bb233Ojc.js: SyntaxError: Cannot use import statement outside a module

I couldn't find an existing issue where this is tracked, curious what needs to be done / what is the blocker for js modules? Let me know if there’s a way I can help!

[krichprollsch]

Hello @utay,
Thank you for the issue 🙏 .

v8 exposes a different API to load modules. We started lightpanda-io/zig-js-runtime#262 to support them.
We can finalize this PR w/ @francisbouvier. But you can use it locally if you want with the required zig-v8 branche too (lightpanda-io/zig-v8-fork#31). Note: you don't have to re-compile v8, lightpanda-io/zig-v8-fork#31 changes only v8.zig file, so the zig build is enough to benefit the change.

I think isJS could return an enum instead of a bool to specify the kind of JS script.
Then in case of module we should be able to load it via the module func.

[utay]

Hi @krichprollsch, thanks for the quick response!

I tried that and hit unreachable in resolveModuleCallback; so I tried to return null instead and I get a fatal error:

# Fatal error in ../../../../v8/src/execution/isolate-inl.h, line 98
# Debug check failed: has_scheduled_exception().

Let me know if I can make it work somehow, otherwise I'll wait for this PR to get merged :)

[utay]

Actually with a simple module script it works (doesn't crash) but with my production script it crashes with this fatal error. Here's the script, I didn't dig in yet!

<script type="module" src="https://sorare.com/assets/index-Bb233Ojc.js"></script>

[krichprollsch]

I tried your module via #345 and I can reproduce the issue.
The guilty code is here: https://github.com/lightpanda-io/zig-js-runtime/pull/262/files#diff-95eb15b03076f8e1523137913f2b48b9129434ad0b39ba2c6b9f6636f5c5ece0R378

I have to implement the resolve module callback function which is called when a module imports another.

[utay]

@krichprollsch I made some progress, here's where I am:

• As you just mentioned, the fatal error happens when resolveModuleCallback returns null and the module import crashes
• I implemented resolveModuleCallback to resolve the import with an HTTP GET and return the instantiated module
• At this point LDP does no longer crash but the HTML output is unexpected (it's the same as if no JS had run)
• So I checked the return value of module and it's a promise; I checked the promise state/result:

debug: v8.Promise.State.kRejected TypeError: Cannot read properties of undefined (reading 'pushState')

pushState is used in https://sorare.com/assets/vendor-BdsYyh6F.js and it seems to be called on window.history. I'm not sure why it's undefined

EDIT: with a shell I realized that src/html/window.zig doesn't implement history so I guess this is why and it's outside the scope of modules

[krichprollsch]

well done @utay 👍

EDIT: with a shell I realized that src/html/window.zig doesn't implement history so I guess this is why and it's outside the scope of modules

You are right, history web api is not done for now. So I suppose the module loading works by itself.
Do you think you can share your changes in a PR?

[utay]

Yes, I started here: utay/zig-js-runtime@b32d0c1

Still need to:

• Find a way to retrieve the origin URL instead of hardcoding it
• Check how to handle errors given that the callback doesn't return an error, maybe return null is fine

[krichprollsch]

@utay I worked on the modules the past days and I implements resolveModuleCallback into #345 and lightpanda-io/zig-js-runtime#262

In difference with your change, the browser is responsible to fetch and compile the module.
zig-jsruntime calls the registred module loader function to retrieve the module itself.

I didn't handle the case when a module is loaded from a different url than the main page and has a deps with a relative url.

[krichprollsch]

To test lightpanda-io/zig-js-runtime#262 you will have to re-build v8 with make install-zig-jsruntime-dev.

[utay]

@krichprollsch It looks great, thank you!

Just curious, I was using Context.get|setEmbedderData and I see that you're using Isolate.get|setData, could you explain why the latter is more appropriate?

[krichprollsch]

We had a discussion about that point with @francisbouvier.
We choose Isolate.setData instead of Context.SetEmbedderData because resolveModuleCallback is called during the module's compilation. And the module compilation expects only an Isolate and not the Context.
So it makes more sense for us to keep the values required to compile and resolve modules at the isolate level.

I wanted to thank you for your PR, it helped me a lot to finalize the changes. 🙏