-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This follows the EPS an allows the node CLI to have ESM as an entry point. `node ./example.mjs`. A newer V8 is needed for `import()` so that is not included. `import.meta` is still in specification stage so that also is not included. PR-URL: #14369 Author: Bradley Farias <[email protected]> Author: Guy Bedford <[email protected]> Author: Jan Krems <[email protected]> Author: Timothy Gu <[email protected]> Author: MichaΓ«l Zasso <[email protected]> Author: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
# ECMAScript Modules | ||
|
||
<!--introduced_in=v9.x.x--> | ||
|
||
> Stability: 1 - Experimental | ||
<!--name=esm--> | ||
|
||
Node contains support for ES Modules based upon the [the Node EP for ES Modules][]. | ||
|
||
Not all features of the EP are complete and will be landing as both VM support and implementation is ready. Error messages are still being polished. | ||
|
||
## Enabling | ||
|
||
<!-- type=misc --> | ||
|
||
The `--experimental-modules` flag can be used to enable features for loading ESM modules. | ||
|
||
Once this has been set, files ending with `.mjs` will be able to be loaded as ES Modules. | ||
|
||
```sh | ||
node --experimental-modules my-app.mjs | ||
``` | ||
|
||
## Features | ||
|
||
<!-- type=misc --> | ||
|
||
### Supported | ||
|
||
Only the CLI argument for the main entry point to the program can be an entry point into an ESM graph. In the future `import()` can be used to create entry points into ESM graphs at run time. | ||
|
||
### Unsupported | ||
|
||
| Feature | Reason | | ||
| --- | --- | | ||
| `require('./foo.mjs')` | ES Modules have differing resolution and timing, use language standard `import()` | | ||
| `import()` | pending newer V8 release used in Node.js | | ||
| `import.meta` | pending V8 implementation | | ||
| Loader Hooks | pending Node.js EP creation/consensus | | ||
|
||
## Notable differences between `import` and `require` | ||
|
||
### No NODE_PATH | ||
|
||
`NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks if this behavior is desired. | ||
|
||
### No `require.extensions` | ||
|
||
`require.extensions` is not used by `import`. The expectation is that loader hooks can provide this workflow in the future. | ||
|
||
### No `require.cache` | ||
|
||
`require.cache` is not used by `import`. It has a separate cache. | ||
|
||
### URL based paths | ||
|
||
ESM are resolved and cached based upon [URL](url.spec.whatwg.org) semantics. This means that files containing special characters such as `#` and `?` need to be escaped. | ||
|
||
Modules will be loaded multiple times if the `import` specifier used to resolve them have a different query or fragment. | ||
|
||
```js | ||
import './foo?query=1'; // loads ./foo with query of "?query=1" | ||
import './foo?query=2'; // loads ./foo with query of "?query=2" | ||
``` | ||
|
||
For now, only modules using the `file:` protocol can be loaded. | ||
|
||
## Interop with existing modules | ||
|
||
All CommonJS, JSON, and C++ modules can be used with `import`. | ||
|
||
Modules loaded this way will only be loaded once, even if their query or fragment string differs between `import` statements. | ||
|
||
When loaded via `import` these modules will provide a single `default` export representing the value of `module.exports` at the time they finished evaluating. | ||
|
||
```js | ||
import fs from 'fs'; | ||
fs.readFile('./foo.txt', (err, body) => { | ||
if (err) { | ||
console.error(err); | ||
} else { | ||
console.log(body); | ||
} | ||
}); | ||
``` | ||
|
||
[the Node EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
'use strict'; | ||
|
||
const { URL } = require('url'); | ||
const { getURLFromFilePath } = require('internal/url'); | ||
|
||
const { | ||
getNamespaceOfModuleWrap | ||
} = require('internal/loader/ModuleWrap'); | ||
|
||
const ModuleMap = require('internal/loader/ModuleMap'); | ||
const ModuleJob = require('internal/loader/ModuleJob'); | ||
const resolveRequestUrl = require('internal/loader/resolveRequestUrl'); | ||
const errors = require('internal/errors'); | ||
|
||
function getBase() { | ||
try { | ||
return getURLFromFilePath(`${process.cwd()}/`); | ||
} catch (e) { | ||
e.stack; | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
bmeck
Author
Member
|
||
// If the current working directory no longer exists. | ||
if (e.code === 'ENOENT') { | ||
return undefined; | ||
} | ||
throw e; | ||
} | ||
} | ||
|
||
class Loader { | ||
constructor(base = getBase()) { | ||
this.moduleMap = new ModuleMap(); | ||
if (typeof base !== 'undefined' && base instanceof URL !== true) { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'base', 'URL'); | ||
} | ||
this.base = base; | ||
} | ||
|
||
async resolve(specifier) { | ||
const request = resolveRequestUrl(this.base, specifier); | ||
This comment has been minimized.
Sorry, something went wrong.
richardbutler
|
||
if (request.url.protocol !== 'file:') { | ||
throw new errors.Error('ERR_INVALID_PROTOCOL', | ||
request.url.protocol, 'file:'); | ||
} | ||
return request.url; | ||
} | ||
|
||
async getModuleJob(dependentJob, specifier) { | ||
if (!this.moduleMap.has(dependentJob.url)) { | ||
throw new errors.Error('ERR_MISSING_MODULE', dependentJob.url); | ||
} | ||
const request = await resolveRequestUrl(dependentJob.url, specifier); | ||
const url = `${request.url}`; | ||
if (this.moduleMap.has(url)) { | ||
return this.moduleMap.get(url); | ||
} | ||
const dependencyJob = new ModuleJob(this, request); | ||
this.moduleMap.set(url, dependencyJob); | ||
return dependencyJob; | ||
} | ||
|
||
async import(specifier) { | ||
const request = await resolveRequestUrl(this.base, specifier); | ||
const url = `${request.url}`; | ||
let job; | ||
if (this.moduleMap.has(url)) { | ||
job = this.moduleMap.get(url); | ||
} else { | ||
job = new ModuleJob(this, request); | ||
this.moduleMap.set(url, job); | ||
} | ||
const module = await job.run(); | ||
return getNamespaceOfModuleWrap(module); | ||
} | ||
} | ||
Object.setPrototypeOf(Loader.prototype, null); | ||
module.exports = Loader; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
'use strict'; | ||
|
||
const { SafeSet, SafePromise } = require('internal/safe_globals'); | ||
const resolvedPromise = SafePromise.resolve(); | ||
const resolvedArrayPromise = SafePromise.resolve([]); | ||
const { ModuleWrap } = require('internal/loader/ModuleWrap'); | ||
|
||
const NOOP = () => { /* No-op */ }; | ||
class ModuleJob { | ||
/** | ||
* @param {module: ModuleWrap?, compiled: Promise} moduleProvider | ||
*/ | ||
constructor(loader, moduleProvider, url) { | ||
this.url = `${moduleProvider.url}`; | ||
this.moduleProvider = moduleProvider; | ||
this.loader = loader; | ||
this.error = null; | ||
this.hadError = false; | ||
|
||
if (moduleProvider instanceof ModuleWrap !== true) { | ||
// linked == promise for dependency jobs, with module populated, | ||
// module wrapper linked | ||
this.modulePromise = this.moduleProvider.createModule(); | ||
this.module = undefined; | ||
const linked = async () => { | ||
const dependencyJobs = []; | ||
this.module = await this.modulePromise; | ||
this.module.link(async (dependencySpecifier) => { | ||
const dependencyJobPromise = | ||
this.loader.getModuleJob(this, dependencySpecifier); | ||
dependencyJobs.push(dependencyJobPromise); | ||
const dependencyJob = await dependencyJobPromise; | ||
return dependencyJob.modulePromise; | ||
}); | ||
return SafePromise.all(dependencyJobs); | ||
}; | ||
this.linked = linked(); | ||
|
||
// instantiated == deep dependency jobs wrappers instantiated, | ||
//module wrapper instantiated | ||
this.instantiated = undefined; | ||
} else { | ||
const getModuleProvider = async () => moduleProvider; | ||
this.modulePromise = getModuleProvider(); | ||
this.moduleProvider = { finish: NOOP }; | ||
this.module = moduleProvider; | ||
this.linked = resolvedArrayPromise; | ||
this.instantiated = this.modulePromise; | ||
} | ||
} | ||
|
||
instantiate() { | ||
if (this.instantiated) { | ||
return this.instantiated; | ||
} | ||
return this.instantiated = new Promise(async (resolve, reject) => { | ||
const jobsInGraph = new SafeSet(); | ||
let jobsReadyToInstantiate = 0; | ||
// (this must be sync for counter to work) | ||
const queueJob = (moduleJob) => { | ||
if (jobsInGraph.has(moduleJob)) { | ||
return; | ||
} | ||
jobsInGraph.add(moduleJob); | ||
moduleJob.linked.then((dependencyJobs) => { | ||
for (const dependencyJob of dependencyJobs) { | ||
queueJob(dependencyJob); | ||
} | ||
checkComplete(); | ||
}, (e) => { | ||
if (!this.hadError) { | ||
this.error = e; | ||
this.hadError = true; | ||
} | ||
checkComplete(); | ||
}); | ||
}; | ||
const checkComplete = () => { | ||
if (++jobsReadyToInstantiate === jobsInGraph.size) { | ||
// I believe we only throw once the whole tree is finished loading? | ||
// or should the error bail early, leaving entire tree to still load? | ||
if (this.hadError) { | ||
reject(this.error); | ||
} else { | ||
try { | ||
this.module.instantiate(); | ||
for (const dependencyJob of jobsInGraph) { | ||
dependencyJob.instantiated = resolvedPromise; | ||
} | ||
resolve(this.module); | ||
} catch (e) { | ||
e.stack; | ||
reject(e); | ||
} | ||
} | ||
} | ||
}; | ||
queueJob(this); | ||
}); | ||
} | ||
|
||
async run() { | ||
const module = await this.instantiate(); | ||
try { | ||
module.evaluate(); | ||
} catch (e) { | ||
e.stack; | ||
this.hadError = true; | ||
this.error = e; | ||
throw e; | ||
} | ||
return module; | ||
} | ||
} | ||
Object.setPrototypeOf(ModuleJob.prototype, null); | ||
module.exports = ModuleJob; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
'use strict'; | ||
|
||
const ModuleJob = require('internal/loader/ModuleJob'); | ||
const { SafeMap } = require('internal/safe_globals'); | ||
const debug = require('util').debuglog('esm'); | ||
const errors = require('internal/errors'); | ||
|
||
// Tracks the state of the loader-level module cache | ||
class ModuleMap extends SafeMap { | ||
get(url) { | ||
if (typeof url !== 'string') { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'url', 'string'); | ||
} | ||
return super.get(url); | ||
} | ||
set(url, job) { | ||
if (typeof url !== 'string') { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'url', 'string'); | ||
} | ||
if (job instanceof ModuleJob !== true) { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'job', 'ModuleJob'); | ||
} | ||
debug(`Storing ${url} in ModuleMap`); | ||
return super.set(url, job); | ||
} | ||
has(url) { | ||
if (typeof url !== 'string') { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'url', 'string'); | ||
} | ||
return super.has(url); | ||
} | ||
} | ||
module.exports = ModuleMap; |
11 comments
on commit c8a389e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally not worth worrying about at this point, but FYI the commit message contains lines longer than 72 chars. (If we want to start allowing lines longer than that, we should update CONTRIBUTING.md. I suspect a fair number of people will want to keep the char length limit at 72, though.)
Again, not a big deal, but something to be aware of in the future.
The CLI tool core-validate-commit
is useful for linting for these sorts of things. Can be installed with npm -g core-validate-commit
or run just once with npx core-validate-commit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: OMG this landed! π π― π₯ π± β€οΈ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before Node folks read my comment, I want to say absolutely phenomenal work, this is a lot of fun to mess with locally for-the-first-time-ever. Feels historic and awesome!
I'm also not sure if this is the proper place to raise this issue, but while experimenting with this commit, I attempted to load my ES-compatible module: https://github.com/tbranyen/diffhtml/packages/diffhtml which ships with a dist/es
folder that contains ES modules. These modules all have file extensions with .js
. I followed the instructions in this commit to create an mjs
entry-point script and attempt to load this module to test it out.
Example code:
Ξ» cat ./index.mjs
import { innerHTML } from 'diffhtml/dist/es/index';
console.log(innerHTML);
Example execution:
./node --experimental-modules index.mjs
I get the following:
SyntaxError: The requested module does not provide an export named 'innerHTML'
This is strange, since if I look in this file I see:
Ξ» cat node_modules/diffhtml/dist/es/index.js
...
export { VERSION, addTransitionState, removeTransitionState, release, createTree, use, outerHTML, innerHTML, html, Internals };
I see it right there! So then I thought, well since import { readFile } from 'fs';
doesn't work, maybe I need to simple import the entire module as-is, so I try that (note: the reason this doesn't work, appears to be that fs module is still Node CJS):
Ξ» cat ./index.mjs
import diff from 'diffhtml/dist/es/index';
console.log(diff.innerHTML);
Notice the import
signature change. Now when I execute the same ./node
command as shown before, I get:
/home/tbranyen/git/diffhtml/packages/diffhtml/dist/es/index.js:1
(function (exports, require, module, __filename, __dirname) { import createTree from './tree/create';
^^^^^^
SyntaxError: Unexpected token import
WOAH, wholly unexpected and strange to see the Node CJS wrapper in there... Shouldn't Node know absolutely that I'm importing an ES module? Why did it assume it was an ES module with one signature, but a Node module with the other?
While you may be able to support a whole lot more than before, I'm wondering if this is going to cause a tidal wave of issues being opened as these semantics are quite complex to remember compared to the simple Node CJS implementation.
Maybe an additional note should be made that while you can get the import
semantics with .mjs
this needs to be true for all required modules as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't Node know absolutely that I'm importing an ES module?
No, because import
supports CJS modules as well. The only way it's able to tell is from the file extensions.
To make it clear: import
statements only works in .mjs
, period, no matter how it's loaded -- from another import
statement, as the main module, or in the future from import()
expressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two diff errors:
SyntaxError: The requested module does not provide an export named 'innerHTML'
and
(function (exports, require, module, __filename, __dirname) { import createTree from './tree/create';
seems kind of odd. I'd expect both to fail with a similar error.
For reference the @std/esm loader will treat both imports with the same error:
/projects/diffhtml/packages/diffhtml/dist/es/index.js:1
import createTree from './tree/create';
^
SyntaxError: 'import' and 'export' may only be used in ES modules
at /projects/diffhtml/packages/diffhtml/index.mjs
at Object.<anonymous> (/projects/diffhtml/packages/diffhtml/index.mjs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TimothyGu that can't be true, since I gave an example where the extension had nothing to do with the way the module was "interpreted". You'll notice how import { innerHTML }
parsed it as an ESM file even though it's .js
and says that there is no innerHTML
exported. Or maybe it was parsed as CJS, but that can't be true, because we saw how CJS gets a wrapper function.
@jdalton yup, very confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulled [email protected]
off npm:
import { innerHTML } from 'diffhtml/dist/es/index';
Resolves to diffhtml/dist/es/index.js
, which is CommonJS (it has .js
as an extension). CommonJS only provides a default
export. Export bindings are hooked up prior to parsing the module currently, but we could swap the ordering if we are ok with a slightly larger overhaul to duplicate require
mechanisms to have better errors when passed between.
import diff from 'diffhtml/dist/es/index';
Still resolves to diffhtml/dist/es/index.js
, which is CommonJS (it has .js
as an extension). export
statements are not valid in CommonJS so this fails on parsing.
@TimothyGu these don't look like bugs except wanting a better error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your insight, but I'd appreciate it if you don't try to bring unrelated modules into discussions of bugs in Node.js core.
Considering it's implementing Node's ESM rules (with the review of bmeck), helping find underspecified areas / scenarios, informing users of Node's ESM rules, and placating others who don't care for the direction, I think it's OK to mention esp. when it's in the context of the exact behavior being discussed.
these don't look like bugs except wanting a better error message.
Kinda splitting hairs on the label. Swapping the order will a more consistent experience and be less WTF for users. π
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdalton it swaps timing of when fs read occurs, so it is a visible change. A better error message doesn't need to swap the order if we can interpret the thrown error manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it swaps timing of when fs read occurs, so it is a visible change. A better error message doesn't need to swap the order if we can interpret the thrown error manually.
Rock! If it can be detected other ways that's cool too.
Space nit. The line is ...
%s.Legacy
should be ...%s. Legacy