-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Roadmap for stabilization of vm modules #37648
Comments
That last one might be a bug/race condition in Jest's implementation still. I haven't found the time to dig into it properly since opening up the issue, unfortunately. It feels like it's related to some module caching or context GC thing, but that's pure speculation. I have some hope it's related to whatever needs fixing for #36351 (again, speculating) |
definitely blocked on v8:9968 |
The above linked v8 issue has been closed as fixed now (:tada:) - any idea of when we'll know if it's enough for Node's use cases? |
The commit fixing the above V8 issue is in V8 9.4. I'll probably open a PR upgrading to that version next week. |
it is not fixed quite yet, check v8:10284 |
Ah ok, thanks for the link! Seems there's quite a bit of activity recently, so hopefully it's not too far away 🙂 |
Just wanted to say that Jest is blocked on those tasks being completed in order to properly support ESM. It would be amazing if this could be prioritized! |
Vitest is also blocked on those tasks too by the looks of vitest-dev/vitest#795 (comment). It seems to segfault on |
I got another segfault in Vitest: vitest-dev/vitest#317 |
|
vite also would have a use case for VM modules, if they were made stable. |
Any update on this? As mentioned earlier, using imports in jest test files needs the experimental flag
|
Pull requests are welcome! |
cmon.. it's been years. This feature would help w/ so much in the JS ecosystem |
Pull requests are still welcome. |
After 2.5 years, one of the issues in the OP can finally be checked off 😀 |
@joyeecheung can the first item be ticked off since #33439 is closed? |
Not sure if there's an issue for it, but the cache misses in |
This PR updates `execa` to `^8.0.1`. Since `execa@>=6.0.0` is ESM-only and `jest` only has experimental ESM support (jestjs/jest#10976), this required switching from `ts-jest` to `babel-jest`. To minimize dependency transpilation, the ESM packages that are necessary to transpile are enumerated in `jest.config.js`. This version of `execa` includes [automatic escaping of shell arguments](https://github.com/sindresorhus/execa/tree/v8.0.1#execafile-arguments-options), which was the entire point of #112, #113, and this PR. The state of ESM support in the Node.js ecosystem is absolutely horrible, and I would not recommend further migrations for the time being. We should continue to dual-release our packages and avoid ESM-only dependencies until the ecosystem has matured. For details see the above `jest` issue and nodejs/node#37648.
Just to be clear, is this what is blocking VSCode extensions from using ES modules? |
@nodejs/vm |
2 of the linked issues in the OP have been closed - can probably tick them off yeah. As for the remaining 2, #35714 is arguably a change that can be made as semver minor? And #33216 is hard to say whether is a bug in Node or in Jest, so probably shouldn't block stabilization. Having |
When using jest this code gets transpiled to `require` and the file URL syntax of the import is not supported and breaks the testing environment. This change only uses file URL import syntax on windows machines. This fixes our tests and is still supported on linux and mac operating systems. I think windows is definitely in the minority here and we should not optimize for it. Furthermore, I believe using a UNC path would be a better solution outright but I cannot test because I don't have a window machine at hand. Why not just use jest's `--experimental-vm-modules` flag? Glad you asked. Because of this [jest bug](jestjs/jest#11434 (comment)) which is caused by this [node bug](nodejs/node#37648) which is ultimately caused by this [v8 bug](https://issues.chromium.org/issues/40784051)
Updated |
Thoughts on making vm modules opt-out rather than opt-in for Node 23? |
What do you mean? Can you elaborate? |
@piranna I would wager @SimenB means this: https://nodejs.org/api/vm.html#class-vmmodule I.e. to not enforce |
Yeah 👍 Last sentence of #37648 (comment)
|
According to @SimenB, here are the issues that should probably be taken care of before we unflag and mark vm modules as stable:
vm.Script
'simportModuleDynamically
should get a reference to thecontext
used to run it (#35714)import()
instead of callingimportModuleDynamically
(#35889)/cc @devsnek
The text was updated successfully, but these errors were encountered: