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

module: Allow runMain to be ESM #14369

Closed
wants to merge 1 commit into
base: master
from
Jump to file or symbol
Failed to load files and symbols.
+1,578 −40
Diff settings

Always

Just for now

View
@@ -10,6 +10,11 @@ env:
parserOptions:
ecmaVersion: 2017
overrides:
- files: ["doc/api/esm.md", "*.mjs"]
parserOptions:
sourceType: module
rules:
# Possible Errors
# http://eslint.org/docs/rules/#possible-errors
View
@@ -150,7 +150,7 @@ coverage-build: all
"$(CURDIR)/testing/coverage/gcovr-patches.diff"); fi
if [ -d lib_ ]; then $(RM) -r lib; mv lib_ lib; fi
mv lib lib_
$(NODE) ./node_modules/.bin/nyc instrument lib_/ lib/
$(NODE) ./node_modules/.bin/nyc instrument --extension .js --extension .mjs lib_/ lib/

This comment has been minimized.

@bcoe

bcoe Aug 31, 2017

Member

I don't think you'd need to add the additional .mjs extension until you have some .mjs files in lib_, is this just preemptive with the hope that some modules will gradually convert their syntax over?

@bcoe

bcoe Aug 31, 2017

Member

I don't think you'd need to add the additional .mjs extension until you have some .mjs files in lib_, is this just preemptive with the hope that some modules will gradually convert their syntax over?

This comment has been minimized.

@bmeck

bmeck Aug 31, 2017

Member

preemptive

@bmeck

bmeck Aug 31, 2017

Member

preemptive

This comment has been minimized.

@jasnell

jasnell Aug 31, 2017

Member

will we need similar changes in vcbuild.bat? (I honestly don't know because I've actively tried to avoid doing anything with vcbuild.bat ;-) ... ping @refack )

@jasnell

jasnell Aug 31, 2017

Member

will we need similar changes in vcbuild.bat? (I honestly don't know because I've actively tried to avoid doing anything with vcbuild.bat ;-) ... ping @refack )

This comment has been minimized.

@addaleax

addaleax Sep 1, 2017

Member

We don’t have any Windows coverage tooling right now, so: no, but maybe the linter bits?

@addaleax

addaleax Sep 1, 2017

Member

We don’t have any Windows coverage tooling right now, so: no, but maybe the linter bits?

This comment has been minimized.

@refack

refack Sep 7, 2017

Member

Since even the linter bit is preemptive, IMHO this should not be a blocker.
I'll open a new PR to update the linter bits in vcbuild.

@refack

refack Sep 7, 2017

Member

Since even the linter bit is preemptive, IMHO this should not be a blocker.
I'll open a new PR to update the linter bits in vcbuild.

$(MAKE)
coverage-test: coverage-build
@@ -886,7 +886,7 @@ JSLINT_TARGETS = benchmark doc lib test tools
jslint:
@echo "Running JS linter..."
$(NODE) tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js,.md \
$(NODE) tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js,.mjs,.md \
$(JSLINT_TARGETS)
jslint-ci:
View
@@ -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';

This comment has been minimized.

@targos

targos Sep 7, 2017

Member

@not-an-aardvark is it possible to setup eslint's sourceType: module only for this file?

@targos

targos Sep 7, 2017

Member

@not-an-aardvark is it possible to setup eslint's sourceType: module only for this file?

This comment has been minimized.

@targos

targos Sep 7, 2017

Member

nvm I found a way

@targos

targos Sep 7, 2017

Member

nvm I found a way

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
@@ -109,6 +109,13 @@
'DeprecationWarning', 'DEP0062', startup, true);
}
if (process.binding('config').experimentalModules) {
process.emitWarning(
'The ESM module loader is experimental.',
'ExperimentalWarning', undefined);
}
// There are various modes that Node can run in. The most common two
// are running from a script and running the REPL - but there are a few
// others like the debugger or running --eval arguments. Here we decide
View
@@ -229,6 +229,9 @@ E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe');
E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks');
E('ERR_METHOD_NOT_IMPLEMENTED', 'The %s method is not implemented');
E('ERR_MISSING_ARGS', missingArgs);
E('ERR_MISSING_MODULE', 'Cannot find module %s');

This comment has been minimized.

@Fishrock123

Fishrock123 Aug 29, 2017

Member

Is this error also reusable for the CJS loader? Should it be more specific to ESM if not?

@Fishrock123

Fishrock123 Aug 29, 2017

Member

Is this error also reusable for the CJS loader? Should it be more specific to ESM if not?

This comment has been minimized.

@bmeck

bmeck Aug 30, 2017

Member

Could be, but was not planning to change CJS code right now.

@bmeck

bmeck Aug 30, 2017

Member

Could be, but was not planning to change CJS code right now.

This comment has been minimized.

@Fishrock123

Fishrock123 Aug 30, 2017

Member

Or, perhaps this error should be more specific - e.g. what happened that the module could not be found?

@Fishrock123

Fishrock123 Aug 30, 2017

Member

Or, perhaps this error should be more specific - e.g. what happened that the module could not be found?

This comment has been minimized.

@Fishrock123

Fishrock123 Aug 30, 2017

Member

(Assuming this happens for more than ENOENT - if not, ignore.)

@Fishrock123

Fishrock123 Aug 30, 2017

Member

(Assuming this happens for more than ENOENT - if not, ignore.)

This comment has been minimized.

@bmeck

bmeck Aug 30, 2017

Member

Since it generally involves crawling around the fs, using DEBUG is probably more sane. otherwise it would be a large list of things it searched.

@bmeck

bmeck Aug 30, 2017

Member

Since it generally involves crawling around the fs, using DEBUG is probably more sane. otherwise it would be a large list of things it searched.

E('ERR_MODULE_RESOLUTION_LEGACY', '%s not found by import in %s.' +
'Legacy behavior in require would have found it at %s');
E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times');
E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function');
E('ERR_NAPI_CONS_PROTOTYPE_OBJECT', 'Constructor.prototype must be an object');
@@ -237,6 +240,7 @@ E('ERR_NO_ICU', '%s is not supported on Node.js compiled without ICU');
E('ERR_NO_LONGER_SUPPORTED', '%s is no longer supported');
E('ERR_OUTOFMEMORY', 'Out of memory');
E('ERR_PARSE_HISTORY_DATA', 'Could not parse history data in %s');
E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s');
E('ERR_SERVER_ALREADY_LISTEN',
'Listen method has been called more than once without closing.');
E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound');
@@ -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;
// If the current working directory no longer exists.
if (e.code === 'ENOENT') {

This comment has been minimized.

@Fishrock123

Fishrock123 Aug 29, 2017

Member

Should this also be the same for EACCESS? Not really sure, just curious.

@Fishrock123

Fishrock123 Aug 29, 2017

Member

Should this also be the same for EACCESS? Not really sure, just curious.

This comment has been minimized.

@bmeck

bmeck Aug 30, 2017

Member

I don't think so, since it would still exist.

matches:

if (e.code !== 'ENOENT') {

@bmeck

bmeck Aug 30, 2017

Member

I don't think so, since it would still exist.

matches:

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.

@miraage

miraage Oct 6, 2017

Missing await statement.

@miraage

miraage Oct 6, 2017

Missing await statement.

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;

This comment has been minimized.

@jkrems

jkrems Jul 31, 2017

Contributor

The duplication between import and getModuleJob seems a bit awkward. Is there any way we could make import reuse getModuleJob (or the common pieces via something else)?

@jkrems

jkrems Jul 31, 2017

Contributor

The duplication between import and getModuleJob seems a bit awkward. Is there any way we could make import reuse getModuleJob (or the common pieces via something else)?

This comment has been minimized.

@bmeck

bmeck Jul 31, 2017

Member

could make getModuleJob accept null as first argument and use this.base but that feels a bit wrong since you might have something without a base.

@bmeck

bmeck Jul 31, 2017

Member

could make getModuleJob accept null as first argument and use this.base but that feels a bit wrong since you might have something without a base.

This comment has been minimized.

@jkrems

jkrems Jul 31, 2017

Contributor

If it doesn't have a base, it shouldn't matter (unless I misunderstood what you meant by it). E.g. something that's already absolute shouldn't be hurt by any given base.

@jkrems

jkrems Jul 31, 2017

Contributor

If it doesn't have a base, it shouldn't matter (unless I misunderstood what you meant by it). E.g. something that's already absolute shouldn't be hurt by any given base.

This comment has been minimized.

@bmeck

bmeck Jul 31, 2017

Member

I need to think on if things always have absolute paths if they don't have a base. unlinked cwd came up for example.

@bmeck

bmeck Jul 31, 2017

Member

I need to think on if things always have absolute paths if they don't have a base. unlinked cwd came up for example.

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;
@@ -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;
@@ -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;
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.