Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion around ESM exports support #2258

Closed
mstoykov opened this issue Nov 22, 2021 · 3 comments
Closed

Discussion around ESM exports support #2258

mstoykov opened this issue Nov 22, 2021 · 3 comments
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 js-compat question

Comments

@mstoykov
Copy link
Contributor

mstoykov commented Nov 22, 2021

tl;dr: there are problems with us supporting named and default exports and the way require works, and most importantly how ESM works. Please read the whole thing either way as the details are important especially if you don't understand how ESM and CommonJS differ and how babel transpiles.

Background:

Previous to #2108 and consequently #2234 the way for a module to export anything that can be used by a script was that common.Bind will be called on the registered module struct and w/e it returns will be returned by the k6 require implementation "as is".

common.Bind on its own just gets all public fields and methods of the struct provided and makes a map[string]interface{} which wraps methods in such a way that if they require context.Context as the first argument they will get a value there + other stuff that I won't reiterate again.

With the changes to using modules.Module, I also added a way for the module to specify more explicitly what it exports. And as part of that, I made it more ESM (Ecmascript modules) like where you can export a default and a not default exports which are named Named 😉.

Problem:

Goja does not have support for ESM, the whole import/export syntax that is currently used by k6 is actually transpiled to require by babel. This also happens to be one of the last 4 things babel is used for the other three being the exponent operator(**), classes and super(). But while the others might be used fairly sparse (by most users) the import/export syntax is more or less the most used feature that is still supported by babel and arguably the most used ever even before most ES6 support was missing from goja.
Just as an example ... look at any example or documentation and you will see that there is definitely a use of import and export. The only places we advise against using them is when we are talking about --compatibility-mode=base.

Babel currently transpiles all of this to what is known as commonJS which is more or less what nodejs used (and still supports). Nodejs also did not support ESM until 3 years ago (citation needed, might be 4) and there the situation is complicated as technically commonJS and ESM while letting you "import"/"export" and in general reuse functionality from (usually) different files/modules, they do work as differently from one another as possible IMO.

I will (for this particular issue) concentrate on what require and import do differently from the user perspective and not go deep (as I also don't understand it well enough) on the actual implementation requirements for both:
First of all require is a function call and it just returns a ... thing. What it returns is what is exported and it is basically the value of module.exports (exports is just an "alias" to module.exports) from the file/module that was imported.

five.js:

modules.exports = 5

main.js:

5 == require("./five.js", 5);

should work.
I would like to emphasize that there isn't any special syntax here or any kind of special treatment of anything here (modules.exports and exports are a bit magical to facilitate cyclical imports and they need to be defined by the require implementation, but ) for what require does ... it's just a function call like all others. It can in reality even be implemented in pure JS using something to read/load the js and eval.

ESM is completely different thing. ESM in of itself is a syntax. What (and how) it does what it does can not be implemented in pure JS and in actuality the parsing of import .... from "./five.js" requires the parsing of "./five.js" in ESM. I will not go in details here though.

Additionally, in ESM there is such a thing as a "default" export (and consequently import).

import something from "somewhere";

and

import { something } from "somewhere";

have completely different meanings:
the first imports the default export of "somewhere" and names it "something"
the second imports "something" from "somewhere" where that is how it's named in the module that is imported. Not a property of the default of the export named "something".

How does babel know what to do?

If you've read so far you now wonder what

import http, { post } from 'k6/http';
import { sleep } from 'k6';

export default function() {
        http.get("https://test.k6.io")
        sleep(1);        
        post("https://test.k6.io")
}

will be transpiled to by babel and how will it behave. Well using the currently used babel and just printing what it returns we get(comments mine):

'use strict';

Object.defineProperty(exports, "__esModule", { // this is explained at the end of this file
        value: true
});

exports.default = function () { // this our default function
        _http2.default.get("https://test.k6.io"); // notice how we get the `default` and then call `get` on it ?
        (0, _k.sleep)(1); // this uses the comma operator to AFAIK make `this` be the `globalThis` for the function call
        (0, _http.post)("https://test.k6.io"); // notice that her `default` is not used and that it used `_http` instead of `_http2`
};

var _http = require('k6/http'); // here just a call to require

var _http2 = _interopRequireDefault(_http); // oh interesting we define _http2 as the return from this function call

var _k = require('k6'); // here just a call to require

// as we can see this will return its argument if it's defined and does have a property `__esModule` which is true, otherwise it will return a new object with property `default` that is equal to the argument. This same property is set earlier for this code so that babel signals to itself(mostly) that this is an ESM and then when it loads it can through this function find this out. 
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

As you can see babel will add some stuff in order to be able to distinguish between ESM through the use of __esModule to figure out that default is actually a thing and if there is ESM module that does not have a default export this will blow up. But in the case where it isn't an ESM it will just be the default to the whole object.

That last thing is basically what has been happening with k6 internal modules up until the latest changes. Now there is actually code to create a module.exports for the internal modules in a similar (same way) as babel does for its own uses.

If you look carefully though you will notice that in cases where either there is no default export or there is only a default export it will just return the default export or the (so-called) named ones as an object and no __esModule.

This was a compromise/hack we made when first adding this code as otherwise if you just have (let's say) named exports and someone does

import coolthing from "k6/something"
coolthing.coolName;

this will blow up as there is no default export so coolthin will be undefined. (Technically this is supposed to be parse error in ECMAScript, but obviously babel transpiling this can't know this at the time).

Also if there is only a default export the code:

var coolthing = require("k6/something")
coolthing.coolName

will blow up as the correct name is coolthing.default.coolName, coolthing is the whole modules.Export object after all.

It was actually the second one(supporting require) for which we made the change at that time but as noted this basically only works as long as modules don't have both default and named exports. So the moment we start to have differing Named and default exports this will "break"

I did back then and ... I do argue now again that most users will not write require by hand they will use babel to do it. Given that having require work in some more user-friendly way is IMO less important than having import/export work correctly.

Decision to be made:

Okay after all of this there are some decisions that need to be made:

  1. Do we actually want to have internal modules that can have named and default exports?
    Nodejs basically decided that they won't have different ones and their default export is basically all their named ones so the above distinction isn't even there.
    We can always do something similar "by hand" or keep the current hack.

  2. If we decide to not support it for our modules, do we also drop support for extensions? (if we decide to keep it, I see no reason extension to not have it :))

  3. Should we break some of the stuff in case we want to keep it? Or only for new stuff are we going to make this distinction.

Example code:

import http, {Client, get} from "k6";

let client = new http.Client(); // this used the default export - while I would probably do this in go, I feel like this isn't very javascripty
// or 
let client = new Client(); // this uses a named one - a lot of sense 

export default function() {
   client.get(); // uses  
   http.get(); // default export - a lot of sense to me 
   // or 
   get(); // the named export - this makes little sense to me

}

The general idea of the default export is that it exports some kind of shortcut or the only way to do something IIRC. Most k6 modules IMO don't have this very clear distinction, apart from http for which http.get seems really nice especially if/when we will have some kind of Client.

Another important thing is that default exports are not namespaces.

import * as http from "k6/http";

imports all exports as http from the module, import http from "k6/http" imports the default export. Again this is the same for nodejs and currently for k6, but is not actually how ESM is designed to be used ;).

@mstoykov mstoykov added evaluation needed proposal needs to be validated or tested before fully implementing it in k6 js-compat question labels Nov 22, 2021
@mstoykov
Copy link
Contributor Author

Also if we decide that we support them, no matter what require does now it probably will still need to do this same thing even when import/export is actually supported by goja because:

  1. we need to not break compatibility with old scripts (especially if we decide to break it around this)
  2. this workaround so require works in a way that babel can make ESM work will still be needed as users can user babel to transpile their code and then run this transpiled code so k6 can't know if the original had import and was transpiled or was written by hand. While source maps might help in some cases they might not have been generated or available :)

@na--
Copy link
Member

na-- commented Dec 3, 2021

It took a few starts, but I think I understand everything mentioned here and the code in

k6/js/initcontext.go

Lines 176 to 197 in 5c14dae

func toESModuleExports(exp modules.Exports) interface{} {
if exp.Named == nil {
return exp.Default
}
if exp.Default == nil {
return exp.Named
}
result := make(map[string]interface{}, len(exp.Named)+2)
for k, v := range exp.Named {
result[k] = v
}
// Maybe check that those weren't set
result["default"] = exp.Default
// this so babel works with the `default` when it transpiles from ESM to commonjs.
// This should probably be removed once we have support for ESM directly. So that require doesn't get support for
// that while ESM has.
result["__esModule"] = true
return result
}

I am not sure what the end result should look like though, and I guess it partially depends on how native import/export is going to be implemented in goja anyway (:crossed_fingers:). But your point #2258 (comment) seems quite sensible and I don't see a reason to not continue having whatever hacks for require() are necessary to keep user-transpiled scripts working, even after we remove babel ourselves 🤷‍♂️ If needs be, we could do a minor breaking change and remove require() support in --compatibility-mode=extended and leave it available only in base 🤷‍♂️

And we can change anything else in how we expose our built-in modules to make them compliant with both the ES module spec and with our publicly documented JS APIs in https://k6.io/docs/javascript-api/

@mstoykov
Copy link
Contributor Author

mstoykov commented Jul 8, 2024

All of this have been implemented and you can have both default and named exports.

With the addition of native ESM we will likely need to redesign part of that, but it is currently working and has been since #2108

@mstoykov mstoykov closed this as completed Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 js-compat question
Projects
None yet
Development

No branches or pull requests

2 participants