Modules support

[mstoykov]

This is a WIP PR implementing ECMAScript Modules (ESM) in goja.

The PR does add every ESM feature as of ES2022 including:

• basic import/export syntax.
• module namespace import * as ns from "somewhere".
• dynamic import.
• async module evaluation, even though the source text module do not support top level await. This definitely needs more testing as it was added fairly recently and required ... a bit of refactoring.

The WIP part of this PR comes from the fact that there are many open implementation specifics that I am not certain about and at least for some of them I would need your input, @dop251.

(For not @dop251, while I will appreciate any input, if you just want to try it out - this might be a bit hard as there is no documentation. Also, as mentioned below a lot of the APIs will change. But if you really want look at modules_integration_test.go as that likely is the best place for a small(ish) but fully working example)

Why open an unfinished PR

I have worked on this on and off since I guess December, but mostly in the last 3 months. Unforunately while I did manage to implement most stuff I just kept getting more and more questions that likely I should've asked earlier. Also, I am getting more and more other work that also needs to get done. Because of that I decided to try to get some feedback on the current implementation in hopes that will make things easier for both of us.

I will try to ask most of the questions inline/incode, this hopefully will help with threading, but we can also move any discussion in another place if you want to. For some of the questions I have - I also have ideas, which I will probably go with when I have the time even if you haven't come back to me. Also, all my links will be to my branch, because links to code in big PRs just plain don't work, and you would likely want to open them locally either way.

Again I will continue working on this, but hopefully with some help the time it takes me will be smaller. It likely also will make this easier for you to review in the end ;)

Tthere are a bunch of:

• commented fmt.Print* lines that will be removed in any final PR.
• TODOs coments which will also be removed as I go through them.
• big functions that will likely be broken in smaller ones.

You can probably ignore those unless you want something to stay or want to given some specific feedback there.

I have tried to not change anything in goja that I don't need, but I still needed to touch stuff which aren't directly about modules. Hopefully all of are fine.

Really sorry for the formatting changes in some places. I have tried to bring them to a minimal as to not just move code around. I should've probably disabled gofumpt for this entire project and that would've helped more 🤷‍♂. I can probably try to remove them one final time after everything else is done.

This whole PR should be squashed in the end. Some commits are kind of useful if you need to figure out what I have tried, but most of it is likely very outdated and will definitely not be useful once we are done with this feature.

I guess first:

Are you interested in goja having ESM support in general? And do you see any fundamental issues with the approach I have taken and have not asked questions for?
It's quite a big change, I will fully understand if you don't want it or, do not have time to code review it and help me right now or until I am actually of the opinion I am done.

Background on why I am working on this and a particular problem I still need to fix in k6

I specifically wanted to work on this as in k6 we do have this, I guess unfortunate, requirement that import "commonjsModule" needs to work as well as require("ESMmodule"). Cycles of both of them aren't that necessary to be supported (although I guess it will be nice if I can make them work 🤷‍♂).

Due to k6 currently using babel internally to transpile ESM to CommonJS, users have been using both and because transpiling takes time we have historically made helper modules as CommonJS :(. So this all leads to the fact that some interoperability is heavily required.

Luckily I haven't really done anything to the code design to make it possible 🤷‍♂it just kind of works on the most basic level as shown in the k6 PoC PR

That k6 PR apart from lacking a ton of tests, mostly to test cycles and interoperability, has only 2 known bugs/problems - so probably a lot more 😢.

Both open and require should be relative to the script/module that is the current "root" of execution, instead they are currently (in that PR) relative to the file that is the main script/module - the argument to k6 run this/file/here.js.

This is one of those really hard to explain with just words problems. But I basically need activeScriptOrModule, but instead of returning the module we are currently "in" (as in the code/text place), I need the one that is the root of the execution.

And yes I (now) know that require is supposed to be relative to the file it's written in just like import()... but that is literally not how it has been since k6 was made and even if we change that ... open still exists. I have opened an issue to figure out if k6 would not want to change this.

You do currently do this in goja_nodejs here, but I am not sure this works with source maps (it seems to, I guess I am wrong where the "rename" happens 🤷‍♂) but in order to get the "root" module/script I will need to go through the whole stack ... which might be big :(. I kind of feel like that is an okay thing for goja to provide. What do you think?

(Note: currently k6 can know which the currently executing script file as it is the only thing that is executing stuff, once goja starts evaluating cyclic modules this will no longer work)

Edit: I just posted all of the comments inline and noticed, while copying the messages I had prepared, that the order of the code in the PR as always does not in any way reflect how the code actually flows 🤦

I would recommend looking at something like modules_integration_test.go to see how the code is used and then following the code from there. I would argue also that the most important comments and questions are in modules.go and compiler.go.

Also thanks again for working on goja 🙇

[mstoykov]

Just wanted to ping you @dop251.

I have removed the not-quite-yield hack I had and have made eval working as far as I can see.

Also top-level-await is also working.

AFAIK any modules features currently is working or .,.. I don't know about it 😅.

The code definitely needs way more work. But I want to touch bases on what direction the current base should be moved into.

On my "short" TODO list are:

1. realign the current implementation more with the current specification. While the code works, the specification changed twice over the time this has been worked on, so definitely some things are now called differently or are slightly differently explained. One of those changes helped me a lot as it specified more clearly how a thing should work, I expect other such will be also helpful. I just haven't had time to go through the code from one end to the other to check everything.
2. I do think the current exportIndirect and co. are not great - they were what I managed to do over a year ago, understanding way less than I do now. Unfortunately even now I would argue I do not have a much better way to do it that probably won't be even more performance degrading. Arguably for only source modules we can directly reference the stash of another module. Any ideas or pointers will be great.
3. The API that is exposed to the end user evolved ... uh ... naturally aka it is pretty crappy. Forget that it doesn't have any documentation (which also is a problem) it is way worse then before as it now have even more promises everywhere. Considerable amount of the work was to try to keep the error path to always resolve with an actual error instead of maybe an error maybe something else. The plan here is that I probably should be wrapping more of an object with function fields which will be way easier to work with then a bunch of interfaces. But more importantly won't require the user to implement multiple interfaces where most of the methods return false/nil or something similar.
4. Every inline TODO

Any feedback is welcome, I am writing this now, as I will be away for a bit leaving you with some time and also managed to get everything in the working but pretty ugly state.

There is a newer k6 PoC PR that hacks around some stuff and can be used as an example (on top of the integration tests) of how this should be used until I write documentation.

p.s. to everybody else - sorry for not coming back sooner, but making this work seemed more important than discussing the final API. Hopefully after a few rounds with @dop251 we will get to your points as well.

[dop251]

Thanks for working on this. Unfortunately I'm rather time constrained at the moment, and it's a big PR to review, but I'll get to it as soon as I can. It would be nice to close this final gap so that ES6 is fully supported.

[gabereiser]

Its been a couple months now, what's the status of this?

[godLei6]

when can mr?

[zakiRefai]

I am also wondering about when this PR will merge

[mstoykov]

After some internal discussion within the k6 team we have decided upon forking goja.

This is in part to get this PR going, but also as the JS engine is integral part of k6, us being able to make changes to it in a timely manner is paramount.

Some of those changes though might not be beneficial to goja or not stable enough.

As such this PR is now merged in our fork Sobek and due to the potential of us changing the API, I have removed backwards compatibility on the API from the README. It is very likely in the future the API will be completely redone, hopefully in a way that is easier on the user.

The k6 team does intend to keep Sobek and goja as close as possible code wise and to this end we will be back porting any future changes to Sobek and merging changes from goja in Sobek. Including this PR or at least its functionality when it is considered ready. Or if it ends up being included in goja in another way we will try to integrate those changes in Sobek.

Thank you for all the work done on this project @dop251 🙇.