feat: experimental ES Modules support #9772
Conversation
SimenB
requested review from
cpojer,
scotthovestadt,
rickhanlonii,
DiZy,
thymikee and
jeysal
SimenB
force-pushed
the
esm-experiments
branch
4 times, most recently
from
f82910e
to
d3608c5
Compare
SimenB
force-pushed
the
esm-experiments
branch
3 times, most recently
from
26037fb
to
08fd412
Compare
Codecov Report
@@ Coverage Diff @@
## master #9772 +/- ##
==========================================
- Coverage 64.88% 64.51% -0.38%
==========================================
Files 289 290 +1
Lines 12229 12318 +89
Branches 3030 3044 +14
==========================================
+ Hits 7935 7947 +12
- Misses 3654 3731 +77
Partials 640 640
Continue to review full report at Codecov.
|
SimenB
force-pushed
the
esm-experiments
branch
4 times, most recently
from
1172f4d
to
31e567b
Compare
| return false; | ||
| } | ||
|
|
||
| // this isn't correct - we might wanna load any file as a module (using synthetic module) |
There was a problem hiding this comment.
What's the issue with falling back to pkg.packageJson.type === 'module'; for non-JS as well?
There was a problem hiding this comment.
mostly because it's not how node would do it - it expects explicit loaders. This is wrong, but I think less wrong. This whole thing is gonna be implemented in resolve at some point regardless
packages/jest-runtime/src/index.ts
Outdated
| initializeImportMeta(meta: JestImportMeta) { | ||
| meta.url = pathToFileURL(modulePath).href; | ||
| // @ts-ignore TODO: fill this | ||
| meta.jest = {}; |
There was a problem hiding this comment.
Do we want to provide Jest in this way at all, or do we want to rely on import from '@jest/globals' for ESM instead?
There was a problem hiding this comment.
Not sure. The @jest/globals thing is a thing I came of with this morning. I think I like importing more than adding to meta (it's easier to type (no messing with the global ImportMeta interface) which usually means it's easier to consume as well), so maybe just drop it?
| }); | ||
| }, | ||
| // should identifier be `node://${moduleName}`? | ||
| {context, identifier: moduleName}, |
There was a problem hiding this comment.
Normally stack traces will show e.g. fs.js as the identifier. Have you tried a fs.readFileSync('/does/not/exist') to see what shows up with this logic?
There was a problem hiding this comment.
Doesn't seem to show up at all
$ node --experimental-vm-modules file.mjs
(node:93098) ExperimentalWarning: The ESM module loader is experimental.
Error: ENOENT: no such file or directory, open '/does/not/exist'
at Object.openSync (fs.js:462:3)
at Module.readFileSync (fs.js:364:35)
at myTestingModule.js:3:6
at SourceTextModule.evaluate (internal/vm/module.js:221:32)
at file:///Users/simen/repos/jest/file.mjs:35:22 {
errno: -2,
syscall: 'open',
code: 'ENOENT',
path: '/does/not/exist'
}
(node:93098) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any timeTest:
import {SourceTextModule, SyntheticModule} from 'vm';
import {createRequire} from 'module';
const require = createRequire(import.meta.url);
const fs = require('fs');
const required = fs;
const module = new SourceTextModule(
`
import * as fs from 'fs';
fs.readFileSync('/does/not/exist');
`,
{identifier: 'myTestingModule.js'},
);
module
.link(async () => {
const m = new SyntheticModule(
['default', ...Object.keys(required)],
function () {
this.setExport('default', required);
Object.entries(required).forEach(([key, value]) => {
this.setExport(key, value);
});
},
{identifier: 'fs halla'},
);
await m.link(() => {});
await m.evaluate();
return m;
})
.then(() => module.evaluate())
.catch(error => {
process.exitCode = 1;
console.error(error);
});| // unstable as it should be replaced by https://github.com/nodejs/modules/issues/393, and we don't want people to use it | ||
| unstable_shouldLoadAsEsm = Resolver.unstable_shouldLoadAsEsm; | ||
|
|
||
| private async loadEsmModule( |
There was a problem hiding this comment.
Another limitation you did not mention seems to be that it does not use any of the logic we have for normal modules regarding mocks, isolation, etc., correct? If so, we could maybe leave a comment saying this needs to be rewritten completely to share code with requireModule
There was a problem hiding this comment.
mocks are not implemented no, but isolation is. THe runtime itself is per test, so isolation should be correct. will need to implement mocks, though
There was a problem hiding this comment.
Sorry that was unclear, I meant isolateModules
There was a problem hiding this comment.
aha, gotcha. isolateModules doesn't work with ESM regardless since it either has to be static or async. We probably need to do something else clever
Co-Authored-By: Tim Seckinger <seckinger.tim@gmail.com>
|
I'll document this when it's more stable |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This adds very basic support for running tests as native ES modules. 2 main limitations:
jestobject onimport.metais an empty object. (meaning none ofjest.*functions are available)Another limitation is that the APIs we need only exist on node 13, and they're flagged as experimental
Like V8 test coverage, it only supports the node env, but using JSDOM 16 should also work, although I haven't bothered to verify.
Ref #9430.
Test plan
Integration test added. More of a smoketest than anything else.