js: make certain only one copy of the exports is used per each module

[mstoykov]

This is inline with ESM and works for source code modules, but not for modules written in go as teh code was never made to work with it.

This also lets (as the test suggests) change what k6/http.get is across the VU just as it should be possible.

[mstoykov]

I will try to disassemble Bundle, BundleInstance and InitContext in some way that I can have only 1 place that we cache the exports and can get them from because this is getting pretty bad :(

[imiric]

Besides my confusion about the duplicate caches below, I'm confused and surprised this is allowed behavior for ESM. I thought this sort of monkeypatching was reserved to Ruby and Python, and I'm not sure it's even possible there 😄

While it's convenient, there are many things that could go wrong if we allow modules to change global properties of other modules at runtime 😅

I did a quick test with Node v16, and this behavior doesn't seem possible. Replicating your test I get:

TypeError: Cannot assign to read only property 'group' of object '[object Module]'

Which seems like the sane thing to do.

Can you link us to the relevant part of the ES spec that mentions this should be allowed?

[mstoykov]

Besides my confusion about the duplicate caches below, I'm confused and surprised this is allowed behavior for ESM. I thought this sort of monkeypatching was reserved to Ruby and Python, and I'm not sure it's even possible there smile

Given how this is more or less how the whole language is designed from day one - to be able to extend properties of objects, I would argue it is the most natural thing 🤣

While it's convenient, there are many things that could go wrong if we allow modules to change global properties of other modules at runtime sweat_smile

This particular change doesn't change if that is possible just makes go modules inline with js modules ;)

You can see how node (specifically) both allows this and has a way to restore the original state - a functionality that I don't think we need at this point.

I did a quick test with Node v16, and this behavior doesn't seem possible. Replicating your test I get:

TypeError: Cannot assign to read only property 'group' of object '[object Module]'

Which seems like the sane thing to do.

Yeah, unfortunately hurrying to write the test for both js modules and go ones I did use the one syntax that prevents this by specification. And of course babel does not prevent it 🤦. Good catch 👍 🙇

Can you link us to the relevant part of the ES spec that mentions this should be allowed?

There really isn't a place where it says that it is forbidden so it is allowed. Default exports are just an object most of the time so unless something makes them immutable it won't matter.

The only part of the specification around ESM that does this is the one around module namespaces (the import * as soemthing from "somewhere" syntax I used way too eagerly), and there it is defined "deep" within the specification on the Set method

[imiric]

Given how this is more or less how the whole language is designed from day one - to be able to extend properties of objects, I would argue it is the most natural thing

Right, this is not unlike being able to change the prototype of an object globally 😄

The only part of the specification around ESM that does this ...

Thanks, though I don't know why I asked for links to the ES spec 😅 It's absolutely unreadable 🙈

[oleiade]

Looking good (albeit weird, and rubyish 💎 , I agree, pun intended)! 🎉 👍🏻
The Javascript voices are impenetrable 🙏🏻