Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Remove --experimental-vm-modules flag #507

Closed
Zolmeister opened this issue Apr 23, 2020 · 8 comments
Closed

Remove --experimental-vm-modules flag #507

Zolmeister opened this issue Apr 23, 2020 · 8 comments

Comments

@Zolmeister
Copy link

This proposal is to remove the --experimental-vm-modules cli flag.
vm.Module is required for tools that deal with transpilation, e.g. Coffeescript (jashkenas/coffeescript#5018 (comment)) in order to correctly support ESM modules.

@bmeck
Copy link
Member

bmeck commented Apr 23, 2020

Can we get clarification on why the desire to use vm instead of loaders?

@SimenB
Copy link
Member

SimenB commented Apr 23, 2020

Jest needs the VM modules, and is currently shipping code that use the flagged APIs. We use it to sandbox tests. (I have no comment about coffeescript usage of those apis, but in case you asked generally)

@bmeck
Copy link
Member

bmeck commented Apr 23, 2020

@SimenB what kind of sandboxing are you doing? I'm assuming it is in a different context? And can you explain what the loader APIs lack that you might need that vm.Module provides

@SimenB
Copy link
Member

SimenB commented Apr 23, 2020

Different contexts, yes. We either create one for a node environment or use the one JSDOM creates to emulate a browser environment. I don't think the loader API allows any custom vm.Context? We also need full control of both CJS and ESM resolution, loading and caching, so any ESM exclusive API is a non-starter.

The currently provided vm APIs works great for our use cases, from my experimentation and testing.

@jkrems
Copy link
Contributor

jkrems commented Apr 23, 2020

I personally would prefer if we had APIs that expose more targeted hooks per-context (loader-like). Last time I tried to implement a working non-trivial system using the existing vm APIs, I had to pretty much copy all of ModuleJob + ModuleLoader from core and clean up the code. But I'm not sure if anybody would sign up for that work, so it doesn't seem fair to hold back people who need a solution rather sooner than later.

@SimenB
Copy link
Member

SimenB commented Apr 23, 2020

I'm personally fine requiring people to use an experimental flag in Jest until you're happy with the implementation in Node fwiw, so no rush unflagging on my part. I just chimed in with a use case (backed by shipping code) for the current API 🙂 The implementation in jest is most definitely also experimental, so having that flag saves us from adding our own

@GeoffreyBooth
Copy link
Member

I've said this before, I think that vm.Module needs to provide a one-liner way to achieve the most common use case, which is evaluating string ESM code with default values for linking and context. From looking at the boilerplate example, that doesn’t seem possible at the moment. It doesn't seem hard to get from here to there, though, so I would want to keep the flag until we have that.

@Zolmeister
Copy link
Author

@bmeck I had not seen https://nodejs.org/api/esm.html#esm_transpiler_loader
After playing with it (and patching a few dependencies), it looks like it sufficiently addresses my use case.
Thank you.
Closing, but feel free to reopen for @SimenB

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants