From 4e4add302a589f3c76049f189b682b4034607b88 Mon Sep 17 00:00:00 2001 From: guybedford Date: Tue, 28 Aug 2018 17:28:46 +0200 Subject: [PATCH 01/37] esm: remove .json support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs: https://github.com/nodejs/modules/pull/180 PR-URL: https://github.com/nodejs/ecmascript-modules/pull/6 Reviewed-By: Myles Borins Reviewed-By: John-David Dalton Reviewed-By: Michaël Zasso --- doc/api/esm.md | 3 +-- lib/internal/modules/esm/default_resolve.js | 1 - lib/internal/modules/esm/translators.js | 27 ++------------------- src/module_wrap.cc | 2 +- test/es-module/test-esm-json.mjs | 8 ------ test/fixtures/es-modules/json.json | 3 --- 6 files changed, 4 insertions(+), 40 deletions(-) delete mode 100644 test/es-module/test-esm-json.mjs delete mode 100644 test/fixtures/es-modules/json.json diff --git a/doc/api/esm.md b/doc/api/esm.md index e81a69c6ed..d5bc002e01 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -86,7 +86,7 @@ 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`. +CommonJS 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. @@ -176,7 +176,6 @@ module. This can be one of the following: | `'esm'` | Load a standard JavaScript module | | `'cjs'` | Load a node-style CommonJS module | | `'builtin'` | Load a node builtin CommonJS module | -| `'json'` | Load a JSON file | | `'addon'` | Load a [C++ Addon][addons] | | `'dynamic'` | Use a [dynamic instantiate hook][] | diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 33366f0069..4dfd08dea3 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -47,7 +47,6 @@ function search(target, base) { const extensionFormatMap = { '__proto__': null, '.mjs': 'esm', - '.json': 'json', '.node': 'addon', '.js': 'cjs' }; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 25552cff0e..68a59ccc92 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -3,8 +3,7 @@ const { NativeModule } = require('internal/bootstrap/loaders'); const { ModuleWrap, callbackMap } = internalBinding('module_wrap'); const { - stripShebang, - stripBOM + stripShebang } = require('internal/modules/cjs/helpers'); const CJSModule = require('internal/modules/cjs/loader'); const internalURLModule = require('internal/url'); @@ -14,18 +13,13 @@ const fs = require('fs'); const { _makeLong } = require('path'); const { SafeMap, - JSON, - FunctionPrototype, - StringPrototype } = primordials; const { URL } = require('url'); const { debuglog, promisify } = require('util'); const esmLoader = require('internal/process/esm_loader'); const readFileAsync = promisify(fs.readFile); -const readFileSync = fs.readFileSync; -const StringReplace = FunctionPrototype.call.bind(StringPrototype.replace); -const JsonParse = JSON.parse; +const StringReplace = Function.call.bind(String.prototype.replace); const debug = debuglog('esm'); @@ -108,20 +102,3 @@ translators.set('addon', async (url) => { reflect.exports.default.set(module.exports); }); }); - -// Strategy for loading a JSON file -translators.set('json', async (url) => { - debug(`Translating JSONModule ${url}`); - return createDynamicModule(['default'], url, (reflect) => { - debug(`Loading JSONModule ${url}`); - const pathname = internalURLModule.fileURLToPath(new URL(url)); - const content = readFileSync(pathname, 'utf8'); - try { - const exports = JsonParse(stripBOM(content)); - reflect.exports.default.set(exports); - } catch (err) { - err.message = pathname + ': ' + err.message; - throw err; - } - }); -}); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 3d55d12cc3..4959f4cd14 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -44,7 +44,7 @@ using v8::String; using v8::Undefined; using v8::Value; -static const char* const EXTENSIONS[] = {".mjs", ".js", ".json", ".node"}; +static const char* const EXTENSIONS[] = {".mjs", ".js", ".node"}; ModuleWrap::ModuleWrap(Environment* env, Local object, diff --git a/test/es-module/test-esm-json.mjs b/test/es-module/test-esm-json.mjs deleted file mode 100644 index a7146d19a9..0000000000 --- a/test/es-module/test-esm-json.mjs +++ /dev/null @@ -1,8 +0,0 @@ -// Flags: --experimental-modules -import '../common'; -import assert from 'assert'; -import ok from '../fixtures/es-modules/test-esm-ok.mjs'; -import json from '../fixtures/es-modules/json.json'; - -assert(ok); -assert.strictEqual(json.val, 42); diff --git a/test/fixtures/es-modules/json.json b/test/fixtures/es-modules/json.json deleted file mode 100644 index 8288d42e2b..0000000000 --- a/test/fixtures/es-modules/json.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "val": 42 -} From e51b25b8dcf7f6dfc70e0362f47a2d85f0ff0298 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Wed, 12 Sep 2018 17:51:28 -0400 Subject: [PATCH 02/37] esm: remove .node support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs: https://github.com/nodejs/modules/pull/180 PR-URL: https://github.com/nodejs/ecmascript-modules/pull/6 Reviewed-By: Guy Bedford Reviewed-By: John-David Dalton Reviewed-By: Michaël Zasso --- doc/api/esm.md | 2 -- lib/internal/modules/esm/default_resolve.js | 1 - lib/internal/modules/esm/translators.js | 13 ------------- src/module_wrap.cc | 2 +- test/addons/hello-world-esm/binding.cc | 14 -------------- test/addons/hello-world-esm/binding.gyp | 9 --------- test/addons/hello-world-esm/test.js | 20 -------------------- test/addons/hello-world-esm/test.mjs | 6 ------ 8 files changed, 1 insertion(+), 66 deletions(-) delete mode 100644 test/addons/hello-world-esm/binding.cc delete mode 100644 test/addons/hello-world-esm/binding.gyp delete mode 100644 test/addons/hello-world-esm/test.js delete mode 100644 test/addons/hello-world-esm/test.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index d5bc002e01..a794cb90af 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -176,7 +176,6 @@ module. This can be one of the following: | `'esm'` | Load a standard JavaScript module | | `'cjs'` | Load a node-style CommonJS module | | `'builtin'` | Load a node builtin CommonJS module | -| `'addon'` | Load a [C++ Addon][addons] | | `'dynamic'` | Use a [dynamic instantiate hook][] | For example, a dummy loader to load JavaScript restricted to browser resolution @@ -253,5 +252,4 @@ then be called at the exact point of module evaluation order for that module in the import tree. [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md -[addons]: addons.html [dynamic instantiate hook]: #esm_dynamic_instantiate_hook diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 4dfd08dea3..1b0d8931f4 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -47,7 +47,6 @@ function search(target, base) { const extensionFormatMap = { '__proto__': null, '.mjs': 'esm', - '.node': 'addon', '.js': 'cjs' }; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 68a59ccc92..46e3bdd998 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -10,7 +10,6 @@ const internalURLModule = require('internal/url'); const createDynamicModule = require( 'internal/modules/esm/create_dynamic_module'); const fs = require('fs'); -const { _makeLong } = require('path'); const { SafeMap, } = primordials; @@ -90,15 +89,3 @@ translators.set('builtin', async (url) => { reflect.exports.default.set(module.exports); }); }); - -// Strategy for loading a node native module -translators.set('addon', async (url) => { - debug(`Translating NativeModule ${url}`); - return createDynamicModule(['default'], url, (reflect) => { - debug(`Loading NativeModule ${url}`); - const module = { exports: {} }; - const pathname = internalURLModule.fileURLToPath(new URL(url)); - process.dlopen(module, _makeLong(pathname)); - reflect.exports.default.set(module.exports); - }); -}); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 4959f4cd14..89a65a03a6 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -44,7 +44,7 @@ using v8::String; using v8::Undefined; using v8::Value; -static const char* const EXTENSIONS[] = {".mjs", ".js", ".node"}; +static const char* const EXTENSIONS[] = {".mjs", ".js"}; ModuleWrap::ModuleWrap(Environment* env, Local object, diff --git a/test/addons/hello-world-esm/binding.cc b/test/addons/hello-world-esm/binding.cc deleted file mode 100644 index 02eecec099..0000000000 --- a/test/addons/hello-world-esm/binding.cc +++ /dev/null @@ -1,14 +0,0 @@ -#include -#include - -void Method(const v8::FunctionCallbackInfo& args) { - v8::Isolate* isolate = args.GetIsolate(); - args.GetReturnValue().Set(v8::String::NewFromUtf8( - isolate, "world", v8::NewStringType::kNormal).ToLocalChecked()); -} - -void init(v8::Local exports) { - NODE_SET_METHOD(exports, "hello", Method); -} - -NODE_MODULE(NODE_GYP_MODULE_NAME, init) diff --git a/test/addons/hello-world-esm/binding.gyp b/test/addons/hello-world-esm/binding.gyp deleted file mode 100644 index 55fbe7050f..0000000000 --- a/test/addons/hello-world-esm/binding.gyp +++ /dev/null @@ -1,9 +0,0 @@ -{ - 'targets': [ - { - 'target_name': 'binding', - 'sources': [ 'binding.cc' ], - 'includes': ['../common.gypi'], - } - ] -} diff --git a/test/addons/hello-world-esm/test.js b/test/addons/hello-world-esm/test.js deleted file mode 100644 index d0faf65540..0000000000 --- a/test/addons/hello-world-esm/test.js +++ /dev/null @@ -1,20 +0,0 @@ -'use strict'; -const common = require('../../common'); - -const assert = require('assert'); -const { spawnSync } = require('child_process'); -const { copyFileSync } = require('fs'); -const { join } = require('path'); - -const buildDir = join(__dirname, 'build'); - -copyFileSync(join(buildDir, common.buildType, 'binding.node'), - join(buildDir, 'binding.node')); - -const result = spawnSync(process.execPath, - ['--experimental-modules', `${__dirname}/test.mjs`]); - -assert.ifError(result.error); -// TODO: Uncomment this once ESM is no longer experimental. -// assert.strictEqual(result.stderr.toString().trim(), ''); -assert.strictEqual(result.stdout.toString().trim(), 'binding.hello() = world'); diff --git a/test/addons/hello-world-esm/test.mjs b/test/addons/hello-world-esm/test.mjs deleted file mode 100644 index d98de5bf87..0000000000 --- a/test/addons/hello-world-esm/test.mjs +++ /dev/null @@ -1,6 +0,0 @@ -/* eslint-disable node-core/required-modules */ - -import assert from 'assert'; -import binding from './build/binding.node'; -assert.strictEqual(binding.hello(), 'world'); -console.log('binding.hello() =', binding.hello()); From 0416d3bdcea862d110f3e3020435030beb6056d0 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Mon, 1 Oct 2018 23:27:30 -0400 Subject: [PATCH 03/37] esm: remove .js support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs: https://github.com/nodejs/modules/pull/180 PR-URL: https://github.com/nodejs/ecmascript-modules/pull/6 Reviewed-By: Guy Bedford Reviewed-By: John-David Dalton Reviewed-By: Michaël Zasso --- doc/api/esm.md | 20 +++++++++++++------ lib/internal/modules/esm/default_resolve.js | 3 +-- src/module_wrap.cc | 2 +- test/common/index.mjs | 14 +++++++++++-- test/es-module/test-esm-double-encoding.mjs | 2 +- test/es-module/test-esm-dynamic-import.js | 1 + test/es-module/test-esm-error-cache.js | 6 +++--- test/es-module/test-esm-require-cache.mjs | 10 +++++++--- test/es-module/test-esm-shared-loader-dep.mjs | 6 ++++-- test/es-module/test-esm-snapshot.mjs | 7 ------- .../es-module-loaders/loader-shared-dep.mjs | 6 +++++- .../es-module-loaders/loader-with-dep.mjs | 6 +++++- .../es-modules/esm-snapshot-mutator.js | 4 ---- test/fixtures/es-modules/esm-snapshot.js | 2 -- test/fixtures/es-modules/pjson-main/main.js | 1 - test/fixtures/es-modules/pjson-main/main.mjs | 1 + .../es-modules/pjson-main/package.json | 2 +- ...=> test-esm-double-encoding-native%20.mjs} | 2 +- test/fixtures/syntax/bad_syntax.mjs | 1 + 19 files changed, 58 insertions(+), 38 deletions(-) delete mode 100644 test/es-module/test-esm-snapshot.mjs delete mode 100644 test/fixtures/es-modules/esm-snapshot-mutator.js delete mode 100644 test/fixtures/es-modules/esm-snapshot.js delete mode 100644 test/fixtures/es-modules/pjson-main/main.js create mode 100644 test/fixtures/es-modules/pjson-main/main.mjs rename test/fixtures/es-modules/{test-esm-double-encoding-native%20.js => test-esm-double-encoding-native%20.mjs} (86%) create mode 100644 test/fixtures/syntax/bad_syntax.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index a794cb90af..4911cdb6a4 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -54,6 +54,10 @@ property: ## Notable differences between `import` and `require` +### Only Support for .mjs + +ESM must have the `.mjs` extension. + ### No NODE_PATH `NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks @@ -95,12 +99,17 @@ 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 -// foo.js -module.exports = { one: 1 }; +// cjs.js +module.exports = 'cjs'; + +// esm.mjs +import { createRequireFromPath as createRequire } from 'module'; +import { fileURLToPath as fromPath } from 'url'; + +const require = createRequire(fromPath(import.meta.url)); -// bar.mjs -import foo from './foo.js'; -foo.one === 1; // true +const cjs = require('./cjs'); +cjs === 'cjs'; // true ``` Builtin modules will provide named exports of their public API, as well as a @@ -174,7 +183,6 @@ module. This can be one of the following: | `format` | Description | | --- | --- | | `'esm'` | Load a standard JavaScript module | -| `'cjs'` | Load a node-style CommonJS module | | `'builtin'` | Load a node builtin CommonJS module | | `'dynamic'` | Use a [dynamic instantiate hook][] | diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 1b0d8931f4..b9429b8d60 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -46,8 +46,7 @@ function search(target, base) { const extensionFormatMap = { '__proto__': null, - '.mjs': 'esm', - '.js': 'cjs' + '.mjs': 'esm' }; function resolve(specifier, parentURL) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 89a65a03a6..c34b6d278a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -44,7 +44,7 @@ using v8::String; using v8::Undefined; using v8::Value; -static const char* const EXTENSIONS[] = {".mjs", ".js"}; +static const char* const EXTENSIONS[] = {".mjs"}; ModuleWrap::ModuleWrap(Environment* env, Local object, diff --git a/test/common/index.mjs b/test/common/index.mjs index de9119f37e..41592098eb 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -1,6 +1,15 @@ // Flags: --experimental-modules /* eslint-disable node-core/required-modules */ -import common from './index.js'; + +import { createRequireFromPath } from 'module'; +import { fileURLToPath as toPath } from 'url'; + +function createRequire(metaUrl) { + return createRequireFromPath(toPath(metaUrl)); +} + +const require = createRequire(import.meta.url); +const common = require('./index.js'); const { isMainThread, @@ -91,5 +100,6 @@ export { getBufferSources, disableCrashOnUnhandledRejection, getTTYfd, - runWithInvalidFD + runWithInvalidFD, + createRequire }; diff --git a/test/es-module/test-esm-double-encoding.mjs b/test/es-module/test-esm-double-encoding.mjs index c81d0530d3..240104dd7f 100644 --- a/test/es-module/test-esm-double-encoding.mjs +++ b/test/es-module/test-esm-double-encoding.mjs @@ -3,4 +3,4 @@ import '../common'; // Assert we can import files with `%` in their pathname. -import '../fixtures/es-modules/test-esm-double-encoding-native%2520.js'; +import '../fixtures/es-modules/test-esm-double-encoding-native%2520.mjs'; diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js index b271d43c80..07294d4d24 100644 --- a/test/es-module/test-esm-dynamic-import.js +++ b/test/es-module/test-esm-dynamic-import.js @@ -1,4 +1,5 @@ // Flags: --experimental-modules + 'use strict'; const common = require('../common'); const assert = require('assert'); diff --git a/test/es-module/test-esm-error-cache.js b/test/es-module/test-esm-error-cache.js index 98244615ef..79f76357ec 100644 --- a/test/es-module/test-esm-error-cache.js +++ b/test/es-module/test-esm-error-cache.js @@ -1,11 +1,11 @@ -'use strict'; - // Flags: --experimental-modules +'use strict'; + require('../common'); const assert = require('assert'); -const file = '../fixtures/syntax/bad_syntax.js'; +const file = '../fixtures/syntax/bad_syntax.mjs'; let error; (async () => { diff --git a/test/es-module/test-esm-require-cache.mjs b/test/es-module/test-esm-require-cache.mjs index ff32cde36f..1228013eee 100644 --- a/test/es-module/test-esm-require-cache.mjs +++ b/test/es-module/test-esm-require-cache.mjs @@ -1,7 +1,11 @@ // Flags: --experimental-modules -import '../common'; -import '../fixtures/es-module-require-cache/preload.js'; -import '../fixtures/es-module-require-cache/counter.js'; +import { createRequire } from '../common'; import assert from 'assert'; +// +const require = createRequire(import.meta.url); + +require('../fixtures/es-module-require-cache/preload.js'); +require('../fixtures/es-module-require-cache/counter.js'); + assert.strictEqual(global.counter, 1); delete global.counter; diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs index 5c274d835c..637fbe3893 100644 --- a/test/es-module/test-esm-shared-loader-dep.mjs +++ b/test/es-module/test-esm-shared-loader-dep.mjs @@ -1,7 +1,9 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs -import '../common'; +import { createRequire } from '../common'; import assert from 'assert'; import '../fixtures/es-modules/test-esm-ok.mjs'; -import dep from '../fixtures/es-module-loaders/loader-dep.js'; + +const require = createRequire(import.meta.url); +const dep = require('../fixtures/es-module-loaders/loader-dep.js'); assert.strictEqual(dep.format, 'esm'); diff --git a/test/es-module/test-esm-snapshot.mjs b/test/es-module/test-esm-snapshot.mjs deleted file mode 100644 index 3d4b44bbdd..0000000000 --- a/test/es-module/test-esm-snapshot.mjs +++ /dev/null @@ -1,7 +0,0 @@ -// Flags: --experimental-modules -import '../common'; -import '../fixtures/es-modules/esm-snapshot-mutator'; -import one from '../fixtures/es-modules/esm-snapshot'; -import assert from 'assert'; - -assert.strictEqual(one, 1); diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs index 1a19e4c892..f7e15baf47 100644 --- a/test/fixtures/es-module-loaders/loader-shared-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-shared-dep.mjs @@ -1,6 +1,10 @@ -import dep from './loader-dep.js'; import assert from 'assert'; +import {createRequire} from '../../common'; + +const require = createRequire(import.meta.url); +const dep = require('./loader-dep.js'); + export function resolve(specifier, base, defaultResolve) { assert.strictEqual(dep.format, 'esm'); return defaultResolve(specifier, base); diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs index 944e6e438c..21798d446a 100644 --- a/test/fixtures/es-module-loaders/loader-with-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-with-dep.mjs @@ -1,4 +1,8 @@ -import dep from './loader-dep.js'; +import {createRequire} from '../../common'; + +const require = createRequire(import.meta.url); +const dep = require('./loader-dep.js'); + export function resolve (specifier, base, defaultResolve) { return { url: defaultResolve(specifier, base).url, diff --git a/test/fixtures/es-modules/esm-snapshot-mutator.js b/test/fixtures/es-modules/esm-snapshot-mutator.js deleted file mode 100644 index ee52c270f6..0000000000 --- a/test/fixtures/es-modules/esm-snapshot-mutator.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; -const shouldSnapshotFilePath = require.resolve('./esm-snapshot.js'); -require('./esm-snapshot.js'); -require.cache[shouldSnapshotFilePath].exports++; diff --git a/test/fixtures/es-modules/esm-snapshot.js b/test/fixtures/es-modules/esm-snapshot.js deleted file mode 100644 index 329a0ca3f4..0000000000 --- a/test/fixtures/es-modules/esm-snapshot.js +++ /dev/null @@ -1,2 +0,0 @@ -'use strict'; -module.exports = 1; diff --git a/test/fixtures/es-modules/pjson-main/main.js b/test/fixtures/es-modules/pjson-main/main.js deleted file mode 100644 index dfdd47b877..0000000000 --- a/test/fixtures/es-modules/pjson-main/main.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = 'main'; diff --git a/test/fixtures/es-modules/pjson-main/main.mjs b/test/fixtures/es-modules/pjson-main/main.mjs new file mode 100644 index 0000000000..3d1d2b15c9 --- /dev/null +++ b/test/fixtures/es-modules/pjson-main/main.mjs @@ -0,0 +1 @@ +export default 'main'; diff --git a/test/fixtures/es-modules/pjson-main/package.json b/test/fixtures/es-modules/pjson-main/package.json index c13b8cf6ac..ea9b784692 100644 --- a/test/fixtures/es-modules/pjson-main/package.json +++ b/test/fixtures/es-modules/pjson-main/package.json @@ -1,3 +1,3 @@ { - "main": "main.js" + "main": "main.mjs" } diff --git a/test/fixtures/es-modules/test-esm-double-encoding-native%20.js b/test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs similarity index 86% rename from test/fixtures/es-modules/test-esm-double-encoding-native%20.js rename to test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs index ea1caa81be..a3bfe972e5 100644 --- a/test/fixtures/es-modules/test-esm-double-encoding-native%20.js +++ b/test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs @@ -3,4 +3,4 @@ // Trivial test to assert we can load files with `%` in their pathname. // Imported by `test-esm-double-encoding.mjs`. -module.exports = 42; +export default 42; diff --git a/test/fixtures/syntax/bad_syntax.mjs b/test/fixtures/syntax/bad_syntax.mjs new file mode 100644 index 0000000000..c2cd118b23 --- /dev/null +++ b/test/fixtures/syntax/bad_syntax.mjs @@ -0,0 +1 @@ +var foo bar; From 4dfe73120ddda7168e66f73f310fe493dce29654 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 2 Oct 2018 00:41:36 -0400 Subject: [PATCH 04/37] esm: remove node specifier resolution algorithm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs: https://github.com/nodejs/modules/pull/180 PR-URL: https://github.com/nodejs/ecmascript-modules/pull/6 Reviewed-By: Guy Bedford Reviewed-By: John-David Dalton Reviewed-By: Michaël Zasso --- doc/api/esm.md | 25 ++-- lib/internal/modules/cjs/loader.js | 7 +- src/module_wrap.cc | 110 ++---------------- src/module_wrap.h | 8 +- test/es-module/test-esm-basic-imports.mjs | 3 +- .../test-esm-cyclic-dynamic-import.mjs | 5 +- test/es-module/test-esm-double-encoding.mjs | 3 +- test/es-module/test-esm-encoded-path.mjs | 3 +- test/es-module/test-esm-forbidden-globals.mjs | 3 +- test/es-module/test-esm-import-meta.mjs | 3 +- test/es-module/test-esm-live-binding.mjs | 3 +- .../test-esm-loader-invalid-format.mjs | 3 +- .../es-module/test-esm-loader-invalid-url.mjs | 4 +- ...oader-missing-dynamic-instantiate-hook.mjs | 3 +- test/es-module/test-esm-main-lookup.mjs | 26 ++++- test/es-module/test-esm-named-exports.mjs | 3 +- test/es-module/test-esm-namespace.mjs | 4 +- .../test-esm-preserve-symlinks-not-found.mjs | 2 +- test/es-module/test-esm-require-cache.mjs | 3 +- test/es-module/test-esm-shared-loader-dep.mjs | 4 +- test/es-module/test-esm-shebang.mjs | 3 +- test/es-module/test-esm-symlink-main.js | 2 +- test/es-module/test-esm-symlink.js | 10 +- test/es-module/test-esm-throw-undefined.mjs | 6 +- .../es-module-loaders/loader-invalid-url.mjs | 1 + .../es-module-loaders/loader-shared-dep.mjs | 2 +- .../es-module-loaders/loader-with-dep.mjs | 2 +- .../es-module-loaders/syntax-error-import.mjs | 2 +- .../es-module-loaders/throw-undefined.mjs | 1 + test/fixtures/es-modules/loop.mjs | 2 +- test/fixtures/es-modules/pjson-main/main.mjs | 2 +- .../esm_display_syntax_error_import.mjs | 6 +- .../esm_display_syntax_error_import.out | 2 +- ...esm_display_syntax_error_import_module.mjs | 5 +- ...esm_display_syntax_error_import_module.out | 4 +- .../esm_display_syntax_error_module.mjs | 5 +- .../test-module-main-extension-lookup.js | 2 +- 37 files changed, 120 insertions(+), 162 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 4911cdb6a4..29bd563c02 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -58,6 +58,14 @@ property: ESM must have the `.mjs` extension. +### Mandatory file extensions + +You must provide a file extension when using the `import` keyword. + +### No importing directories + +There is no support for importing directories. + ### No NODE_PATH `NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks @@ -82,21 +90,15 @@ 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" +import './foo.mjs?query=1'; // loads ./foo.mjs with query of "?query=1" +import './foo.mjs?query=2'; // loads ./foo.mjs with query of "?query=2" ``` For now, only modules using the `file:` protocol can be loaded. -## Interop with existing modules +## CommonJS, JSON, and Native Modules -CommonJS 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. +CommonJS, JSON, and Native modules can be used with [`module.createRequireFromPath()`][`module.createRequireFromPath()`]. ```js // cjs.js @@ -112,6 +114,8 @@ const cjs = require('./cjs'); cjs === 'cjs'; // true ``` +## Builtin modules + Builtin modules will provide named exports of their public API, as well as a default export which can be used for, among other things, modifying the named exports. Named exports of builtin modules are updated when the corresponding @@ -261,3 +265,4 @@ in the import tree. [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [dynamic instantiate hook]: #esm_dynamic_instantiate_hook +[`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 46b101f3a7..23dc2648c5 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -48,6 +48,7 @@ const manifest = getOptionValue('--experimental-policy') ? require('internal/process/policy').manifest : null; const { compileFunction } = internalBinding('contextify'); +const hasLoader = getOptionValue('--loader'); const { ERR_INVALID_ARG_VALUE, @@ -858,7 +859,11 @@ if (experimentalModules) { // bootstrap main module. Module.runMain = function() { // Load the main module--the command line argument. - if (experimentalModules) { + const base = path.basename(process.argv[1]); + const ext = path.extname(base); + const isESM = ext === '.mjs'; + + if (experimentalModules && (isESM || hasLoader)) { if (asyncESM === undefined) lazyLoadESM(); asyncESM.loaderPromise.then((loader) => { return loader.import(pathToFileURL(process.argv[1]).pathname); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index c34b6d278a..012cf702f2 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -27,7 +27,6 @@ using v8::HandleScope; using v8::Integer; using v8::IntegrityLevel; using v8::Isolate; -using v8::JSON; using v8::Just; using v8::Local; using v8::Maybe; @@ -506,70 +505,17 @@ Maybe CheckFile(const std::string& path, return Just(fd); } -using Exists = PackageConfig::Exists; -using IsValid = PackageConfig::IsValid; -using HasMain = PackageConfig::HasMain; - -const PackageConfig& GetPackageConfig(Environment* env, - const std::string& path) { - auto existing = env->package_json_cache.find(path); - if (existing != env->package_json_cache.end()) { - return existing->second; - } - Maybe check = CheckFile(path, LEAVE_OPEN_AFTER_CHECK); - if (check.IsNothing()) { - auto entry = env->package_json_cache.emplace(path, - PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" }); - return entry.first->second; - } - - Isolate* isolate = env->isolate(); - v8::HandleScope handle_scope(isolate); - - std::string pkg_src = ReadFile(check.FromJust()); - uv_fs_t fs_req; - CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr)); - uv_fs_req_cleanup(&fs_req); - - Local src; - if (!String::NewFromUtf8(isolate, - pkg_src.c_str(), - v8::NewStringType::kNormal, - pkg_src.length()).ToLocal(&src)) { - auto entry = env->package_json_cache.emplace(path, - PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" }); - return entry.first->second; - } - - Local pkg_json_v; - Local pkg_json; - - if (!JSON::Parse(env->context(), src).ToLocal(&pkg_json_v) || - !pkg_json_v->ToObject(env->context()).ToLocal(&pkg_json)) { - auto entry = env->package_json_cache.emplace(path, - PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "" }); - return entry.first->second; - } - - Local pkg_main; - HasMain has_main = HasMain::No; - std::string main_std; - if (pkg_json->Get(env->context(), env->main_string()).ToLocal(&pkg_main)) { - has_main = HasMain::Yes; - Utf8Value main_utf8(isolate, pkg_main); - main_std.assign(std::string(*main_utf8, main_utf8.length())); - } - - auto entry = env->package_json_cache.emplace(path, - PackageConfig { Exists::Yes, IsValid::Yes, has_main, main_std }); - return entry.first->second; -} - enum ResolveExtensionsOptions { TRY_EXACT_NAME, ONLY_VIA_EXTENSIONS }; +inline bool ResolvesToFile(const URL& search) { + std::string filePath = search.ToFilePath(); + Maybe check = CheckFile(filePath); + return !check.IsNothing(); +} + template Maybe ResolveExtensions(const URL& search) { if (options == TRY_EXACT_NAME) { @@ -595,24 +541,6 @@ inline Maybe ResolveIndex(const URL& search) { return ResolveExtensions(URL("index", search)); } -Maybe ResolveMain(Environment* env, const URL& search) { - URL pkg("package.json", &search); - - const PackageConfig& pjson = - GetPackageConfig(env, pkg.ToFilePath()); - // Note invalid package.json should throw in resolver - // currently we silently ignore which is incorrect - if (pjson.exists == Exists::No || - pjson.is_valid == IsValid::No || - pjson.has_main == HasMain::No) { - return Nothing(); - } - if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { - return Resolve(env, "./" + pjson.main, search, IgnoreMain); - } - return Resolve(env, pjson.main, search, IgnoreMain); -} - Maybe ResolveModule(Environment* env, const std::string& specifier, const URL& base) { @@ -621,7 +549,7 @@ Maybe ResolveModule(Environment* env, do { dir = parent; Maybe check = - Resolve(env, "./node_modules/" + specifier, dir, CheckMain); + Resolve(env, "./node_modules/" + specifier, dir); if (!check.IsNothing()) { const size_t limit = specifier.find('/'); const size_t spec_len = @@ -641,23 +569,11 @@ Maybe ResolveModule(Environment* env, return Nothing(); } -Maybe ResolveDirectory(Environment* env, - const URL& search, - PackageMainCheck check_pjson_main) { - if (check_pjson_main) { - Maybe main = ResolveMain(env, search); - if (!main.IsNothing()) - return main; - } - return ResolveIndex(search); -} - } // anonymous namespace Maybe Resolve(Environment* env, const std::string& specifier, - const URL& base, - PackageMainCheck check_pjson_main) { + const URL& base) { URL pure_url(specifier); if (!(pure_url.flags() & URL_FLAGS_FAILED)) { // just check existence, without altering @@ -672,13 +588,9 @@ Maybe Resolve(Environment* env, } if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { URL resolved(specifier, base); - Maybe file = ResolveExtensions(resolved); - if (!file.IsNothing()) - return file; - if (specifier.back() != '/') { - resolved = URL(specifier + "/", base); - } - return ResolveDirectory(env, resolved, check_pjson_main); + if (ResolvesToFile(resolved)) + return Just(resolved); + return Nothing(); } else { return ResolveModule(env, specifier, base); } diff --git a/src/module_wrap.h b/src/module_wrap.h index dc34685fed..d3089202ba 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -12,11 +12,6 @@ namespace node { namespace loader { -enum PackageMainCheck : bool { - CheckMain = true, - IgnoreMain = false -}; - enum ScriptType : int { kScript, kModule, @@ -31,8 +26,7 @@ enum HostDefinedOptions : int { v8::Maybe Resolve(Environment* env, const std::string& specifier, - const url::URL& base, - PackageMainCheck read_pkg_json = CheckMain); + const url::URL& base); class ModuleWrap : public BaseObject { public: diff --git a/test/es-module/test-esm-basic-imports.mjs b/test/es-module/test-esm-basic-imports.mjs index 78a4106f94..d9bb22be0a 100644 --- a/test/es-module/test-esm-basic-imports.mjs +++ b/test/es-module/test-esm-basic-imports.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; import assert from 'assert'; import ok from '../fixtures/es-modules/test-esm-ok.mjs'; import okShebang from './test-esm-shebang.mjs'; diff --git a/test/es-module/test-esm-cyclic-dynamic-import.mjs b/test/es-module/test-esm-cyclic-dynamic-import.mjs index c8dfff919c..a207efc73e 100644 --- a/test/es-module/test-esm-cyclic-dynamic-import.mjs +++ b/test/es-module/test-esm-cyclic-dynamic-import.mjs @@ -1,3 +1,4 @@ // Flags: --experimental-modules -import '../common'; -import('./test-esm-cyclic-dynamic-import'); +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import('./test-esm-cyclic-dynamic-import.mjs'); diff --git a/test/es-module/test-esm-double-encoding.mjs b/test/es-module/test-esm-double-encoding.mjs index 240104dd7f..9366d4bd6b 100644 --- a/test/es-module/test-esm-double-encoding.mjs +++ b/test/es-module/test-esm-double-encoding.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; // Assert we can import files with `%` in their pathname. diff --git a/test/es-module/test-esm-encoded-path.mjs b/test/es-module/test-esm-encoded-path.mjs index 365a425afa..2cabfdacff 100644 --- a/test/es-module/test-esm-encoded-path.mjs +++ b/test/es-module/test-esm-encoded-path.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; import assert from 'assert'; // ./test-esm-ok.mjs import ok from '../fixtures/es-modules/test-%65%73%6d-ok.mjs'; diff --git a/test/es-module/test-esm-forbidden-globals.mjs b/test/es-module/test-esm-forbidden-globals.mjs index 4e777412a3..cf110ff290 100644 --- a/test/es-module/test-esm-forbidden-globals.mjs +++ b/test/es-module/test-esm-forbidden-globals.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; // eslint-disable-next-line no-undef if (typeof arguments !== 'undefined') { diff --git a/test/es-module/test-esm-import-meta.mjs b/test/es-module/test-esm-import-meta.mjs index c17e0e20d4..4c34b337fb 100644 --- a/test/es-module/test-esm-import-meta.mjs +++ b/test/es-module/test-esm-import-meta.mjs @@ -1,6 +1,7 @@ // Flags: --experimental-modules +/* eslint-disable node-core/required-modules */ -import '../common'; +import '../common/index.mjs'; import assert from 'assert'; assert.strictEqual(Object.getPrototypeOf(import.meta), null); diff --git a/test/es-module/test-esm-live-binding.mjs b/test/es-module/test-esm-live-binding.mjs index d151e004df..880a6c389b 100644 --- a/test/es-module/test-esm-live-binding.mjs +++ b/test/es-module/test-esm-live-binding.mjs @@ -1,6 +1,7 @@ // Flags: --experimental-modules +/* eslint-disable node-core/required-modules */ -import '../common'; +import '../common/index.mjs'; import assert from 'assert'; import fs, { readFile, readFileSync } from 'fs'; diff --git a/test/es-module/test-esm-loader-invalid-format.mjs b/test/es-module/test-esm-loader-invalid-format.mjs index f8714d4aa1..c3f3a87407 100644 --- a/test/es-module/test-esm-loader-invalid-format.mjs +++ b/test/es-module/test-esm-loader-invalid-format.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs -import { expectsError, mustCall } from '../common'; +/* eslint-disable node-core/required-modules */ +import { expectsError, mustCall } from '../common/index.mjs'; import assert from 'assert'; import('../fixtures/es-modules/test-esm-ok.mjs') diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs index 43971a2e6e..9cf17b2478 100644 --- a/test/es-module/test-esm-loader-invalid-url.mjs +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -1,5 +1,7 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs -import { expectsError, mustCall } from '../common'; +/* eslint-disable node-core/required-modules */ + +import { expectsError, mustCall } from '../common/index.mjs'; import assert from 'assert'; import('../fixtures/es-modules/test-esm-ok.mjs') diff --git a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs index f2b37f7e8a..ab2da7adce 100644 --- a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs +++ b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs @@ -1,6 +1,7 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs +/* eslint-disable node-core/required-modules */ -import { expectsError } from '../common'; +import { expectsError } from '../common/index.mjs'; import('test').catch(expectsError({ code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', diff --git a/test/es-module/test-esm-main-lookup.mjs b/test/es-module/test-esm-main-lookup.mjs index ca313a1d26..76c6263853 100644 --- a/test/es-module/test-esm-main-lookup.mjs +++ b/test/es-module/test-esm-main-lookup.mjs @@ -1,6 +1,26 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; import assert from 'assert'; -import main from '../fixtures/es-modules/pjson-main'; -assert.strictEqual(main, 'main'); +async function main() { + let mod; + try { + mod = await import('../fixtures/es-modules/pjson-main'); + } catch (e) { + assert.strictEqual(e.code, 'MODULE_NOT_FOUND'); + } + + assert.strictEqual(mod, undefined); + + try { + mod = await import('../fixtures/es-modules/pjson-main/main.mjs'); + } catch (e) { + console.log(e); + assert.fail(); + } + + assert.strictEqual(mod.main, 'main'); +} + +main(); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs index 3aae9230de..e235f598cb 100644 --- a/test/es-module/test-esm-named-exports.mjs +++ b/test/es-module/test-esm-named-exports.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; import { readFile } from 'fs'; import assert from 'assert'; import ok from '../fixtures/es-modules/test-esm-ok.mjs'; diff --git a/test/es-module/test-esm-namespace.mjs b/test/es-module/test-esm-namespace.mjs index da1286d0f4..38b7ef12d5 100644 --- a/test/es-module/test-esm-namespace.mjs +++ b/test/es-module/test-esm-namespace.mjs @@ -1,5 +1,7 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ + +import '../common/index.mjs'; import * as fs from 'fs'; import assert from 'assert'; import Module from 'module'; diff --git a/test/es-module/test-esm-preserve-symlinks-not-found.mjs b/test/es-module/test-esm-preserve-symlinks-not-found.mjs index 5119957bae..b5be2d7e63 100644 --- a/test/es-module/test-esm-preserve-symlinks-not-found.mjs +++ b/test/es-module/test-esm-preserve-symlinks-not-found.mjs @@ -1,3 +1,3 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs /* eslint-disable node-core/required-modules */ -import './not-found'; +import './not-found.mjs'; diff --git a/test/es-module/test-esm-require-cache.mjs b/test/es-module/test-esm-require-cache.mjs index 1228013eee..09030e0578 100644 --- a/test/es-module/test-esm-require-cache.mjs +++ b/test/es-module/test-esm-require-cache.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules -import { createRequire } from '../common'; +/* eslint-disable node-core/required-modules */ +import { createRequire } from '../common/index.mjs'; import assert from 'assert'; // const require = createRequire(import.meta.url); diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs index 637fbe3893..20d156fb8b 100644 --- a/test/es-module/test-esm-shared-loader-dep.mjs +++ b/test/es-module/test-esm-shared-loader-dep.mjs @@ -1,5 +1,7 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs -import { createRequire } from '../common'; +/* eslint-disable node-core/required-modules */ +import { createRequire } from '../common/index.mjs'; + import assert from 'assert'; import '../fixtures/es-modules/test-esm-ok.mjs'; diff --git a/test/es-module/test-esm-shebang.mjs b/test/es-module/test-esm-shebang.mjs index d5faace479..486e04dade 100644 --- a/test/es-module/test-esm-shebang.mjs +++ b/test/es-module/test-esm-shebang.mjs @@ -1,6 +1,7 @@ #! }]) // isn't js // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; const isJs = true; export default isJs; diff --git a/test/es-module/test-esm-symlink-main.js b/test/es-module/test-esm-symlink-main.js index f7631ef2e5..871180f5cc 100644 --- a/test/es-module/test-esm-symlink-main.js +++ b/test/es-module/test-esm-symlink-main.js @@ -9,7 +9,7 @@ const fs = require('fs'); tmpdir.refresh(); const realPath = path.resolve(__dirname, '../fixtures/es-modules/symlink.mjs'); -const symlinkPath = path.resolve(tmpdir.path, 'symlink.js'); +const symlinkPath = path.resolve(tmpdir.path, 'symlink.mjs'); try { fs.symlinkSync(realPath, symlinkPath); diff --git a/test/es-module/test-esm-symlink.js b/test/es-module/test-esm-symlink.js index 232925a52e..9b9eb98cd9 100644 --- a/test/es-module/test-esm-symlink.js +++ b/test/es-module/test-esm-symlink.js @@ -12,8 +12,8 @@ const tmpDir = tmpdir.path; const entry = path.join(tmpDir, 'entry.mjs'); const real = path.join(tmpDir, 'index.mjs'); -const link_absolute_path = path.join(tmpDir, 'absolute'); -const link_relative_path = path.join(tmpDir, 'relative'); +const link_absolute_path = path.join(tmpDir, 'absolute.mjs'); +const link_relative_path = path.join(tmpDir, 'relative.mjs'); const link_ignore_extension = path.join(tmpDir, 'ignore_extension.json'); const link_directory = path.join(tmpDir, 'directory'); @@ -22,15 +22,13 @@ fs.writeFileSync(real, 'export default [];'); fs.writeFileSync(entry, ` import assert from 'assert'; import real from './index.mjs'; -import absolute from './absolute'; -import relative from './relative'; +import absolute from './absolute.mjs'; +import relative from './relative.mjs'; import ignoreExtension from './ignore_extension.json'; -import directory from './directory'; assert.strictEqual(absolute, real); assert.strictEqual(relative, real); assert.strictEqual(ignoreExtension, real); -assert.strictEqual(directory, real); `); try { diff --git a/test/es-module/test-esm-throw-undefined.mjs b/test/es-module/test-esm-throw-undefined.mjs index 541127eee5..97e917da5e 100644 --- a/test/es-module/test-esm-throw-undefined.mjs +++ b/test/es-module/test-esm-throw-undefined.mjs @@ -1,11 +1,13 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ + +import '../common/index.mjs'; import assert from 'assert'; async function doTest() { await assert.rejects( async () => { - await import('../fixtures/es-module-loaders/throw-undefined'); + await import('../fixtures/es-module-loaders/throw-undefined.mjs'); }, (e) => e === undefined ); diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs index 12efbb5021..f653155899 100644 --- a/test/fixtures/es-module-loaders/loader-invalid-url.mjs +++ b/test/fixtures/es-module-loaders/loader-invalid-url.mjs @@ -1,3 +1,4 @@ +/* eslint-disable node-core/required-modules */ export async function resolve(specifier, parentModuleURL, defaultResolve) { if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { return { diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs index f7e15baf47..419c75bdbf 100644 --- a/test/fixtures/es-module-loaders/loader-shared-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-shared-dep.mjs @@ -1,6 +1,6 @@ import assert from 'assert'; -import {createRequire} from '../../common'; +import {createRequire} from '../../common/index.mjs'; const require = createRequire(import.meta.url); const dep = require('./loader-dep.js'); diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs index 21798d446a..5afd3b2e21 100644 --- a/test/fixtures/es-module-loaders/loader-with-dep.mjs +++ b/test/fixtures/es-module-loaders/loader-with-dep.mjs @@ -1,4 +1,4 @@ -import {createRequire} from '../../common'; +import {createRequire} from '../../common/index.mjs'; const require = createRequire(import.meta.url); const dep = require('./loader-dep.js'); diff --git a/test/fixtures/es-module-loaders/syntax-error-import.mjs b/test/fixtures/es-module-loaders/syntax-error-import.mjs index 9cad68c7ce..3a6bc5effc 100644 --- a/test/fixtures/es-module-loaders/syntax-error-import.mjs +++ b/test/fixtures/es-module-loaders/syntax-error-import.mjs @@ -1 +1 @@ -import { foo, notfound } from './module-named-exports'; +import { foo, notfound } from './module-named-exports.mjs'; diff --git a/test/fixtures/es-module-loaders/throw-undefined.mjs b/test/fixtures/es-module-loaders/throw-undefined.mjs index f062276767..0349ae112d 100644 --- a/test/fixtures/es-module-loaders/throw-undefined.mjs +++ b/test/fixtures/es-module-loaders/throw-undefined.mjs @@ -1,3 +1,4 @@ 'use strict'; +/* eslint-disable node-core/required-modules */ throw undefined; diff --git a/test/fixtures/es-modules/loop.mjs b/test/fixtures/es-modules/loop.mjs index 1b5cab10ed..3d2ddd2eb7 100644 --- a/test/fixtures/es-modules/loop.mjs +++ b/test/fixtures/es-modules/loop.mjs @@ -1,4 +1,4 @@ -import { message } from './message'; +import { message } from './message.mjs'; var t = 1; var k = 1; diff --git a/test/fixtures/es-modules/pjson-main/main.mjs b/test/fixtures/es-modules/pjson-main/main.mjs index 3d1d2b15c9..9eb0aade18 100644 --- a/test/fixtures/es-modules/pjson-main/main.mjs +++ b/test/fixtures/es-modules/pjson-main/main.mjs @@ -1 +1 @@ -export default 'main'; +export const main = 'main' diff --git a/test/message/esm_display_syntax_error_import.mjs b/test/message/esm_display_syntax_error_import.mjs index 87cedf1d4e..12d10270e9 100644 --- a/test/message/esm_display_syntax_error_import.mjs +++ b/test/message/esm_display_syntax_error_import.mjs @@ -1,7 +1,7 @@ // Flags: --experimental-modules -/* eslint-disable no-unused-vars */ -import '../common'; +/* eslint-disable no-unused-vars, node-core/required-modules */ +import '../common/index.mjs'; import { foo, notfound -} from '../fixtures/es-module-loaders/module-named-exports'; +} from '../fixtures/es-module-loaders/module-named-exports.mjs'; diff --git a/test/message/esm_display_syntax_error_import.out b/test/message/esm_display_syntax_error_import.out index 31ee2b6f4b..48f2e2fb74 100644 --- a/test/message/esm_display_syntax_error_import.out +++ b/test/message/esm_display_syntax_error_import.out @@ -2,5 +2,5 @@ file:///*/test/message/esm_display_syntax_error_import.mjs:6 notfound ^^^^^^^^ -SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports' does not provide an export named 'notfound' +SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports.mjs' does not provide an export named 'notfound' at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*) diff --git a/test/message/esm_display_syntax_error_import_module.mjs b/test/message/esm_display_syntax_error_import_module.mjs index 32c0edb350..a53bbbcd19 100644 --- a/test/message/esm_display_syntax_error_import_module.mjs +++ b/test/message/esm_display_syntax_error_import_module.mjs @@ -1,3 +1,4 @@ // Flags: --experimental-modules -import '../common'; -import '../fixtures/es-module-loaders/syntax-error-import'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import '../fixtures/es-module-loaders/syntax-error-import.mjs'; diff --git a/test/message/esm_display_syntax_error_import_module.out b/test/message/esm_display_syntax_error_import_module.out index b067a77942..3e1024db8a 100644 --- a/test/message/esm_display_syntax_error_import_module.out +++ b/test/message/esm_display_syntax_error_import_module.out @@ -1,6 +1,6 @@ (node:*) ExperimentalWarning: The ESM module loader is experimental. file:///*/test/fixtures/es-module-loaders/syntax-error-import.mjs:1 -import { foo, notfound } from './module-named-exports'; +import { foo, notfound } from './module-named-exports.mjs'; ^^^^^^^^ -SyntaxError: The requested module './module-named-exports' does not provide an export named 'notfound' +SyntaxError: The requested module './module-named-exports.mjs' does not provide an export named 'notfound' at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*) diff --git a/test/message/esm_display_syntax_error_module.mjs b/test/message/esm_display_syntax_error_module.mjs index e74b70bec8..5905d2a954 100644 --- a/test/message/esm_display_syntax_error_module.mjs +++ b/test/message/esm_display_syntax_error_module.mjs @@ -1,3 +1,4 @@ // Flags: --experimental-modules -import '../common'; -import '../fixtures/es-module-loaders/syntax-error'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import '../fixtures/es-module-loaders/syntax-error.mjs'; diff --git a/test/parallel/test-module-main-extension-lookup.js b/test/parallel/test-module-main-extension-lookup.js index 3d20316647..9e7eab295e 100644 --- a/test/parallel/test-module-main-extension-lookup.js +++ b/test/parallel/test-module-main-extension-lookup.js @@ -6,6 +6,6 @@ const { execFileSync } = require('child_process'); const node = process.argv[0]; execFileSync(node, ['--experimental-modules', - fixtures.path('es-modules', 'test-esm-ok')]); + fixtures.path('es-modules', 'test-esm-ok.mjs')]); execFileSync(node, ['--experimental-modules', fixtures.path('es-modules', 'noext')]); From c8bbebe774b965ed6be19126f6ef60d16b1b81fd Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 2 Oct 2018 01:53:35 -0400 Subject: [PATCH 05/37] doc: document minimal kernel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/ecmascript-modules/pull/6 Reviewed-By: Guy Bedford Reviewed-By: John-David Dalton Reviewed-By: Michaël Zasso --- doc/api/esm.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 29bd563c02..83c2d7ab76 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -8,10 +8,11 @@ Node.js contains support for ES Modules based upon the -[Node.js EP for ES Modules][]. +[Node.js EP for ES Modules][] and the [ESM Minimal Kernel][]. -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. +The minimal feature set is designed to be compatible with all potential +future implementations. Expect major changes in the implementation including +interoperability support, specifier resolution, and default behavior. ## Enabling @@ -96,9 +97,9 @@ import './foo.mjs?query=2'; // loads ./foo.mjs with query of "?query=2" For now, only modules using the `file:` protocol can be loaded. -## CommonJS, JSON, and Native Modules +## CommonJS, JSON, and Native Modules -CommonJS, JSON, and Native modules can be used with [`module.createRequireFromPath()`][`module.createRequireFromPath()`]. +CommonJS, JSON, and Native modules can be used with [`module.createRequireFromPath()`][]. ```js // cjs.js @@ -266,3 +267,4 @@ in the import tree. [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [dynamic instantiate hook]: #esm_dynamic_instantiate_hook [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename +[ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md From 68a0551155c605569f28713632ef6474e5e7e366 Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Tue, 2 Oct 2018 16:02:08 -0700 Subject: [PATCH 06/37] esm: Remove --loader. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/ecmascript-modules/pull/6 Reviewed-By: Guy Bedford Reviewed-By: Myles Borins Reviewed-By: Michaël Zasso --- .eslintrc.js | 1 - doc/api/cli.md | 9 -- doc/api/esm.md | 119 ------------------ lib/internal/modules/cjs/loader.js | 3 +- lib/internal/process/esm_loader.js | 10 +- src/node_options.cc | 9 -- src/node_options.h | 1 - test/es-module/test-esm-example-loader.js | 6 - test/es-module/test-esm-loader-dependency.mjs | 5 - .../test-esm-loader-invalid-format.mjs | 12 -- .../es-module/test-esm-loader-invalid-url.mjs | 14 --- ...oader-missing-dynamic-instantiate-hook.mjs | 10 -- test/es-module/test-esm-named-exports.mjs | 9 -- ...-esm-preserve-symlinks-not-found-plain.mjs | 3 - .../test-esm-preserve-symlinks-not-found.mjs | 3 - test/es-module/test-esm-resolve-hook.mjs | 8 -- test/es-module/test-esm-shared-loader-dep.mjs | 11 -- test/es-module/test-esm-throw-undefined.mjs | 16 --- .../builtin-named-exports-loader.mjs | 29 ----- .../es-module-loaders/example-loader.mjs | 37 ------ test/fixtures/es-module-loaders/js-as-esm.js | 1 - test/fixtures/es-module-loaders/js-loader.mjs | 24 ---- test/fixtures/es-module-loaders/loader-dep.js | 1 - .../loader-invalid-format.mjs | 8 -- .../es-module-loaders/loader-invalid-url.mjs | 10 -- .../es-module-loaders/loader-shared-dep.mjs | 11 -- .../loader-unknown-builtin-module.mjs | 6 - .../es-module-loaders/loader-with-dep.mjs | 11 -- .../missing-dynamic-instantiate-hook.mjs | 6 - .../module-named-exports.mjs | 2 - .../not-found-assert-loader.mjs | 22 ---- .../es-module-loaders/syntax-error-import.mjs | 1 - .../es-module-loaders/syntax-error.mjs | 2 - .../es-module-loaders/throw-undefined.mjs | 4 - .../esm_display_syntax_error_import.mjs | 7 -- .../esm_display_syntax_error_import.out | 6 - ...esm_display_syntax_error_import_module.mjs | 4 - ...esm_display_syntax_error_import_module.out | 6 - .../esm_display_syntax_error_module.mjs | 4 - .../esm_display_syntax_error_module.out | 6 - .../test-loaders-unknown-builtin-module.mjs | 12 -- 41 files changed, 2 insertions(+), 467 deletions(-) delete mode 100644 test/es-module/test-esm-example-loader.js delete mode 100644 test/es-module/test-esm-loader-dependency.mjs delete mode 100644 test/es-module/test-esm-loader-invalid-format.mjs delete mode 100644 test/es-module/test-esm-loader-invalid-url.mjs delete mode 100644 test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs delete mode 100644 test/es-module/test-esm-named-exports.mjs delete mode 100644 test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs delete mode 100644 test/es-module/test-esm-preserve-symlinks-not-found.mjs delete mode 100644 test/es-module/test-esm-resolve-hook.mjs delete mode 100644 test/es-module/test-esm-shared-loader-dep.mjs delete mode 100644 test/es-module/test-esm-throw-undefined.mjs delete mode 100644 test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs delete mode 100644 test/fixtures/es-module-loaders/example-loader.mjs delete mode 100644 test/fixtures/es-module-loaders/js-as-esm.js delete mode 100644 test/fixtures/es-module-loaders/js-loader.mjs delete mode 100644 test/fixtures/es-module-loaders/loader-dep.js delete mode 100644 test/fixtures/es-module-loaders/loader-invalid-format.mjs delete mode 100644 test/fixtures/es-module-loaders/loader-invalid-url.mjs delete mode 100644 test/fixtures/es-module-loaders/loader-shared-dep.mjs delete mode 100644 test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs delete mode 100644 test/fixtures/es-module-loaders/loader-with-dep.mjs delete mode 100644 test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs delete mode 100644 test/fixtures/es-module-loaders/module-named-exports.mjs delete mode 100644 test/fixtures/es-module-loaders/not-found-assert-loader.mjs delete mode 100644 test/fixtures/es-module-loaders/syntax-error-import.mjs delete mode 100644 test/fixtures/es-module-loaders/syntax-error.mjs delete mode 100644 test/fixtures/es-module-loaders/throw-undefined.mjs delete mode 100644 test/message/esm_display_syntax_error_import.mjs delete mode 100644 test/message/esm_display_syntax_error_import.out delete mode 100644 test/message/esm_display_syntax_error_import_module.mjs delete mode 100644 test/message/esm_display_syntax_error_import_module.out delete mode 100644 test/message/esm_display_syntax_error_module.mjs delete mode 100644 test/message/esm_display_syntax_error_module.out delete mode 100644 test/parallel/test-loaders-unknown-builtin-module.mjs diff --git a/.eslintrc.js b/.eslintrc.js index 03dd8e47b8..b53a8831de 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -39,7 +39,6 @@ module.exports = { files: [ 'doc/api/esm.md', '*.mjs', - 'test/es-module/test-esm-example-loader.js', ], parserOptions: { sourceType: 'module' }, }, diff --git a/doc/api/cli.md b/doc/api/cli.md index 7e7ef3e285..f8fa53a5cc 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -268,13 +268,6 @@ default) is not firewall-protected.** See the [debugging security implications][] section for more information. -### `--loader=file` - - -Specify the `file` of the custom [experimental ECMAScript Module][] loader. - ### `--max-http-header-size=size` - -To customize the default module resolution, loader hooks can optionally be -provided via a `--loader ./loader-name.mjs` argument to Node.js. - -When hooks are used they only apply to ES module loading and not to any -CommonJS modules loaded. - -### Resolve hook - -The resolve hook returns the resolved file URL and module format for a -given module specifier and parent file URL: - -```js -const baseURL = new URL('file://'); -baseURL.pathname = `${process.cwd()}/`; - -export async function resolve(specifier, - parentModuleURL = baseURL, - defaultResolver) { - return { - url: new URL(specifier, parentModuleURL).href, - format: 'esm' - }; -} -``` - -The `parentModuleURL` is provided as `undefined` when performing main Node.js -load itself. - -The default Node.js ES module resolution function is provided as a third -argument to the resolver for easy compatibility workflows. - -In addition to returning the resolved file URL value, the resolve hook also -returns a `format` property specifying the module format of the resolved -module. This can be one of the following: - -| `format` | Description | -| --- | --- | -| `'esm'` | Load a standard JavaScript module | -| `'builtin'` | Load a node builtin CommonJS module | -| `'dynamic'` | Use a [dynamic instantiate hook][] | - -For example, a dummy loader to load JavaScript restricted to browser resolution -rules with only JS file extension and Node.js builtin modules support could -be written: - -```js -import path from 'path'; -import process from 'process'; -import Module from 'module'; - -const builtins = Module.builtinModules; -const JS_EXTENSIONS = new Set(['.js', '.mjs']); - -const baseURL = new URL('file://'); -baseURL.pathname = `${process.cwd()}/`; - -export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { - if (builtins.includes(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { - // For node_modules support: - // return defaultResolve(specifier, parentModuleURL); - throw new Error( - `imports must begin with '/', './', or '../'; '${specifier}' does not`); - } - const resolved = new URL(specifier, parentModuleURL); - const ext = path.extname(resolved.pathname); - if (!JS_EXTENSIONS.has(ext)) { - throw new Error( - `Cannot load file with non-JavaScript file extension ${ext}.`); - } - return { - url: resolved.href, - format: 'esm' - }; -} -``` - -With this loader, running: - -```console -NODE_OPTIONS='--experimental-modules --loader ./custom-loader.mjs' node x.js -``` - -would load the module `x.js` as an ES module with relative resolution support -(with `node_modules` loading skipped in this example). - -### Dynamic instantiate hook - -To create a custom dynamic module that doesn't correspond to one of the -existing `format` interpretations, the `dynamicInstantiate` hook can be used. -This hook is called only for modules that return `format: 'dynamic'` from -the `resolve` hook. - -```js -export async function dynamicInstantiate(url) { - return { - exports: ['customExportName'], - execute: (exports) => { - // Get and set functions provided for pre-allocated export names - exports.customExportName.set('value'); - } - }; -} -``` - -With the list of module exports provided upfront, the `execute` function will -then be called at the exact point of module evaluation order for that module -in the import tree. - [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md -[dynamic instantiate hook]: #esm_dynamic_instantiate_hook [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename [ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 23dc2648c5..2969fffc12 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -48,7 +48,6 @@ const manifest = getOptionValue('--experimental-policy') ? require('internal/process/policy').manifest : null; const { compileFunction } = internalBinding('contextify'); -const hasLoader = getOptionValue('--loader'); const { ERR_INVALID_ARG_VALUE, @@ -863,7 +862,7 @@ Module.runMain = function() { const ext = path.extname(base); const isESM = ext === '.mjs'; - if (experimentalModules && (isESM || hasLoader)) { + if (experimentalModules && isESM) { if (asyncESM === undefined) lazyLoadESM(); asyncESM.loaderPromise.then((loader) => { return loader.import(pathToFileURL(process.argv[1]).pathname); diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 0b7f1be6ff..6ed43970cb 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -4,7 +4,6 @@ const { callbackMap, } = internalBinding('module_wrap'); -const { pathToFileURL } = require('internal/url'); const Loader = require('internal/modules/esm/loader'); const { wrapToModuleMap, @@ -41,15 +40,8 @@ exports.loaderPromise = new Promise((resolve, reject) => { exports.ESMLoader = undefined; exports.initializeLoader = function(cwd, userLoader) { - let ESMLoader = new Loader(); + const ESMLoader = new Loader(); const loaderPromise = (async () => { - if (userLoader) { - const hooks = await ESMLoader.import( - userLoader, pathToFileURL(`${cwd}/`).href); - ESMLoader = new Loader(); - ESMLoader.hook(hooks); - exports.ESMLoader = ESMLoader; - } return ESMLoader; })(); loaderResolve(loaderPromise); diff --git a/src/node_options.cc b/src/node_options.cc index f2961b2147..30d57b1808 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -101,10 +101,6 @@ void PerIsolateOptions::CheckOptions(std::vector* errors) { } void EnvironmentOptions::CheckOptions(std::vector* errors) { - if (!userland_loader.empty() && !experimental_modules) { - errors->push_back("--loader requires --experimental-modules be enabled"); - } - if (syntax_check_only && has_eval_string) { errors->push_back("either --check or --eval can be used, not both"); } @@ -246,11 +242,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "(default: llhttp).", &EnvironmentOptions::http_parser, kAllowedInEnvironment); - AddOption("--loader", - "(with --experimental-modules) use the specified file as a " - "custom loader", - &EnvironmentOptions::userland_loader, - kAllowedInEnvironment); AddOption("--no-deprecation", "silence deprecation warnings", &EnvironmentOptions::no_deprecation, diff --git a/src/node_options.h b/src/node_options.h index a49425388c..7346e955db 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -104,7 +104,6 @@ class EnvironmentOptions : public Options { bool trace_deprecation = false; bool trace_sync_io = false; bool trace_warnings = false; - std::string userland_loader; bool syntax_check_only = false; bool has_eval_string = false; diff --git a/test/es-module/test-esm-example-loader.js b/test/es-module/test-esm-example-loader.js deleted file mode 100644 index 0b0001acea..0000000000 --- a/test/es-module/test-esm-example-loader.js +++ /dev/null @@ -1,6 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/example-loader.mjs -/* eslint-disable node-core/required-modules */ -import assert from 'assert'; -import ok from '../fixtures/es-modules/test-esm-ok.mjs'; - -assert(ok); diff --git a/test/es-module/test-esm-loader-dependency.mjs b/test/es-module/test-esm-loader-dependency.mjs deleted file mode 100644 index 1ed8685a6f..0000000000 --- a/test/es-module/test-esm-loader-dependency.mjs +++ /dev/null @@ -1,5 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-with-dep.mjs -/* eslint-disable node-core/required-modules */ -import '../fixtures/es-modules/test-esm-ok.mjs'; - -// We just test that this module doesn't fail loading diff --git a/test/es-module/test-esm-loader-invalid-format.mjs b/test/es-module/test-esm-loader-invalid-format.mjs deleted file mode 100644 index c3f3a87407..0000000000 --- a/test/es-module/test-esm-loader-invalid-format.mjs +++ /dev/null @@ -1,12 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs -/* eslint-disable node-core/required-modules */ -import { expectsError, mustCall } from '../common/index.mjs'; -import assert from 'assert'; - -import('../fixtures/es-modules/test-esm-ok.mjs') -.then(assert.fail, expectsError({ - code: 'ERR_INVALID_RETURN_PROPERTY_VALUE', - message: 'Expected string to be returned for the "format" from the ' + - '"loader resolve" function but got type undefined.' -})) -.then(mustCall()); diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs deleted file mode 100644 index 9cf17b2478..0000000000 --- a/test/es-module/test-esm-loader-invalid-url.mjs +++ /dev/null @@ -1,14 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs -/* eslint-disable node-core/required-modules */ - -import { expectsError, mustCall } from '../common/index.mjs'; -import assert from 'assert'; - -import('../fixtures/es-modules/test-esm-ok.mjs') -.then(assert.fail, expectsError({ - code: 'ERR_INVALID_RETURN_PROPERTY', - message: 'Expected a valid url to be returned for the "url" from the ' + - '"loader resolve" function but got ' + - '../fixtures/es-modules/test-esm-ok.mjs.' -})) -.then(mustCall()); diff --git a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs deleted file mode 100644 index ab2da7adce..0000000000 --- a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs +++ /dev/null @@ -1,10 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs -/* eslint-disable node-core/required-modules */ - -import { expectsError } from '../common/index.mjs'; - -import('test').catch(expectsError({ - code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', - message: 'The ES Module loader may not return a format of \'dynamic\' ' + - 'when no dynamicInstantiate function was provided' -})); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs deleted file mode 100644 index e235f598cb..0000000000 --- a/test/es-module/test-esm-named-exports.mjs +++ /dev/null @@ -1,9 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs -/* eslint-disable node-core/required-modules */ -import '../common/index.mjs'; -import { readFile } from 'fs'; -import assert from 'assert'; -import ok from '../fixtures/es-modules/test-esm-ok.mjs'; - -assert(ok); -assert(readFile); diff --git a/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs b/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs deleted file mode 100644 index 2ca0f56581..0000000000 --- a/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs +++ /dev/null @@ -1,3 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs -/* eslint-disable node-core/required-modules */ -import './not-found.js'; diff --git a/test/es-module/test-esm-preserve-symlinks-not-found.mjs b/test/es-module/test-esm-preserve-symlinks-not-found.mjs deleted file mode 100644 index b5be2d7e63..0000000000 --- a/test/es-module/test-esm-preserve-symlinks-not-found.mjs +++ /dev/null @@ -1,3 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs -/* eslint-disable node-core/required-modules */ -import './not-found.mjs'; diff --git a/test/es-module/test-esm-resolve-hook.mjs b/test/es-module/test-esm-resolve-hook.mjs deleted file mode 100644 index e326d20b6d..0000000000 --- a/test/es-module/test-esm-resolve-hook.mjs +++ /dev/null @@ -1,8 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/js-loader.mjs -/* eslint-disable node-core/required-modules */ -import { namedExport } from '../fixtures/es-module-loaders/js-as-esm.js'; -import assert from 'assert'; -import ok from '../fixtures/es-modules/test-esm-ok.mjs'; - -assert(ok); -assert(namedExport); diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs deleted file mode 100644 index 20d156fb8b..0000000000 --- a/test/es-module/test-esm-shared-loader-dep.mjs +++ /dev/null @@ -1,11 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs -/* eslint-disable node-core/required-modules */ -import { createRequire } from '../common/index.mjs'; - -import assert from 'assert'; -import '../fixtures/es-modules/test-esm-ok.mjs'; - -const require = createRequire(import.meta.url); -const dep = require('../fixtures/es-module-loaders/loader-dep.js'); - -assert.strictEqual(dep.format, 'esm'); diff --git a/test/es-module/test-esm-throw-undefined.mjs b/test/es-module/test-esm-throw-undefined.mjs deleted file mode 100644 index 97e917da5e..0000000000 --- a/test/es-module/test-esm-throw-undefined.mjs +++ /dev/null @@ -1,16 +0,0 @@ -// Flags: --experimental-modules -/* eslint-disable node-core/required-modules */ - -import '../common/index.mjs'; -import assert from 'assert'; - -async function doTest() { - await assert.rejects( - async () => { - await import('../fixtures/es-module-loaders/throw-undefined.mjs'); - }, - (e) => e === undefined - ); -} - -doTest(); diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs deleted file mode 100644 index 28ccd6ecf2..0000000000 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ /dev/null @@ -1,29 +0,0 @@ -import module from 'module'; - -const builtins = new Set( - Object.keys(process.binding('natives')).filter(str => - /^(?!(?:internal|node|v8)\/)/.test(str)) -); - -export function dynamicInstantiate(url) { - const builtinInstance = module._load(url.substr(5)); - const builtinExports = ['default', ...Object.keys(builtinInstance)]; - return { - exports: builtinExports, - execute: exports => { - for (let name of builtinExports) - exports[name].set(builtinInstance[name]); - exports.default.set(builtinInstance); - } - }; -} - -export function resolve(specifier, base, defaultResolver) { - if (builtins.has(specifier)) { - return { - url: `node:${specifier}`, - format: 'dynamic' - }; - } - return defaultResolver(specifier, base); -} diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs deleted file mode 100644 index acb4486edc..0000000000 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ /dev/null @@ -1,37 +0,0 @@ -import url from 'url'; -import path from 'path'; -import process from 'process'; - -const builtins = new Set( - Object.keys(process.binding('natives')).filter((str) => - /^(?!(?:internal|node|v8)\/)/.test(str)) -); -const JS_EXTENSIONS = new Set(['.js', '.mjs']); - -const baseURL = new url.URL('file://'); -baseURL.pathname = process.cwd() + '/'; - -export function resolve(specifier, parentModuleURL = baseURL /*, defaultResolve */) { - if (builtins.has(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { - // For node_modules support: - // return defaultResolve(specifier, parentModuleURL); - throw new Error( - `imports must begin with '/', './', or '../'; '${specifier}' does not`); - } - const resolved = new url.URL(specifier, parentModuleURL); - const ext = path.extname(resolved.pathname); - if (!JS_EXTENSIONS.has(ext)) { - throw new Error( - `Cannot load file with non-JavaScript file extension ${ext}.`); - } - return { - url: resolved.href, - format: 'esm' - }; -} diff --git a/test/fixtures/es-module-loaders/js-as-esm.js b/test/fixtures/es-module-loaders/js-as-esm.js deleted file mode 100644 index b4d2741b2f..0000000000 --- a/test/fixtures/es-module-loaders/js-as-esm.js +++ /dev/null @@ -1 +0,0 @@ -export const namedExport = 'named-export'; diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs deleted file mode 100644 index 9fa6b9eed4..0000000000 --- a/test/fixtures/es-module-loaders/js-loader.mjs +++ /dev/null @@ -1,24 +0,0 @@ -import { URL } from 'url'; - -const builtins = new Set( - Object.keys(process.binding('natives')).filter(str => - /^(?!(?:internal|node|v8)\/)/.test(str)) -) - -const baseURL = new URL('file://'); -baseURL.pathname = process.cwd() + '/'; - -export function resolve (specifier, base = baseURL) { - if (builtins.has(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - // load all dependencies as esm, regardless of file extension - const url = new URL(specifier, base).href; - return { - url, - format: 'esm' - }; -} diff --git a/test/fixtures/es-module-loaders/loader-dep.js b/test/fixtures/es-module-loaders/loader-dep.js deleted file mode 100644 index cf821afec1..0000000000 --- a/test/fixtures/es-module-loaders/loader-dep.js +++ /dev/null @@ -1 +0,0 @@ -exports.format = 'esm'; diff --git a/test/fixtures/es-module-loaders/loader-invalid-format.mjs b/test/fixtures/es-module-loaders/loader-invalid-format.mjs deleted file mode 100644 index 17a0dcd04d..0000000000 --- a/test/fixtures/es-module-loaders/loader-invalid-format.mjs +++ /dev/null @@ -1,8 +0,0 @@ -export async function resolve(specifier, parentModuleURL, defaultResolve) { - if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { - return { - url: 'file:///asdf' - }; - } - return defaultResolve(specifier, parentModuleURL); -} diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs deleted file mode 100644 index f653155899..0000000000 --- a/test/fixtures/es-module-loaders/loader-invalid-url.mjs +++ /dev/null @@ -1,10 +0,0 @@ -/* eslint-disable node-core/required-modules */ -export async function resolve(specifier, parentModuleURL, defaultResolve) { - if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { - return { - url: specifier, - format: 'esm' - }; - } - return defaultResolve(specifier, parentModuleURL); -} diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs deleted file mode 100644 index 419c75bdbf..0000000000 --- a/test/fixtures/es-module-loaders/loader-shared-dep.mjs +++ /dev/null @@ -1,11 +0,0 @@ -import assert from 'assert'; - -import {createRequire} from '../../common/index.mjs'; - -const require = createRequire(import.meta.url); -const dep = require('./loader-dep.js'); - -export function resolve(specifier, base, defaultResolve) { - assert.strictEqual(dep.format, 'esm'); - return defaultResolve(specifier, base); -} diff --git a/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs b/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs deleted file mode 100644 index e7c6c8ff34..0000000000 --- a/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs +++ /dev/null @@ -1,6 +0,0 @@ -export async function resolve(specifier, parent, defaultResolve) { - if (specifier === 'unknown-builtin-module') { - return { url: 'unknown-builtin-module', format: 'builtin' }; - } - return defaultResolve(specifier, parent); -} \ No newline at end of file diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs deleted file mode 100644 index 5afd3b2e21..0000000000 --- a/test/fixtures/es-module-loaders/loader-with-dep.mjs +++ /dev/null @@ -1,11 +0,0 @@ -import {createRequire} from '../../common/index.mjs'; - -const require = createRequire(import.meta.url); -const dep = require('./loader-dep.js'); - -export function resolve (specifier, base, defaultResolve) { - return { - url: defaultResolve(specifier, base).url, - format: dep.format - }; -} diff --git a/test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs b/test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs deleted file mode 100644 index 6993747fcc..0000000000 --- a/test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs +++ /dev/null @@ -1,6 +0,0 @@ -export function resolve(specifier, parentModule, defaultResolver) { - if (specifier !== 'test') { - return defaultResolver(specifier, parentModule); - } - return { url: 'file://', format: 'dynamic' }; -} diff --git a/test/fixtures/es-module-loaders/module-named-exports.mjs b/test/fixtures/es-module-loaders/module-named-exports.mjs deleted file mode 100644 index 04f7f43ebd..0000000000 --- a/test/fixtures/es-module-loaders/module-named-exports.mjs +++ /dev/null @@ -1,2 +0,0 @@ -export const foo = 'foo'; -export const bar = 'bar'; diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs deleted file mode 100644 index d15f294fe6..0000000000 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ /dev/null @@ -1,22 +0,0 @@ -import assert from 'assert'; - -// a loader that asserts that the defaultResolve will throw "not found" -// (skipping the top-level main of course) -let mainLoad = true; -export async function resolve (specifier, base, defaultResolve) { - if (mainLoad) { - mainLoad = false; - return defaultResolve(specifier, base); - } - try { - await defaultResolve(specifier, base); - } - catch (e) { - assert.strictEqual(e.code, 'MODULE_NOT_FOUND'); - return { - format: 'builtin', - url: 'fs' - }; - } - assert.fail(`Module resolution for ${specifier} should be throw MODULE_NOT_FOUND`); -} diff --git a/test/fixtures/es-module-loaders/syntax-error-import.mjs b/test/fixtures/es-module-loaders/syntax-error-import.mjs deleted file mode 100644 index 3a6bc5effc..0000000000 --- a/test/fixtures/es-module-loaders/syntax-error-import.mjs +++ /dev/null @@ -1 +0,0 @@ -import { foo, notfound } from './module-named-exports.mjs'; diff --git a/test/fixtures/es-module-loaders/syntax-error.mjs b/test/fixtures/es-module-loaders/syntax-error.mjs deleted file mode 100644 index bda4a7e6eb..0000000000 --- a/test/fixtures/es-module-loaders/syntax-error.mjs +++ /dev/null @@ -1,2 +0,0 @@ -'use strict'; -await async () => 0; diff --git a/test/fixtures/es-module-loaders/throw-undefined.mjs b/test/fixtures/es-module-loaders/throw-undefined.mjs deleted file mode 100644 index 0349ae112d..0000000000 --- a/test/fixtures/es-module-loaders/throw-undefined.mjs +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; -/* eslint-disable node-core/required-modules */ - -throw undefined; diff --git a/test/message/esm_display_syntax_error_import.mjs b/test/message/esm_display_syntax_error_import.mjs deleted file mode 100644 index 12d10270e9..0000000000 --- a/test/message/esm_display_syntax_error_import.mjs +++ /dev/null @@ -1,7 +0,0 @@ -// Flags: --experimental-modules -/* eslint-disable no-unused-vars, node-core/required-modules */ -import '../common/index.mjs'; -import { - foo, - notfound -} from '../fixtures/es-module-loaders/module-named-exports.mjs'; diff --git a/test/message/esm_display_syntax_error_import.out b/test/message/esm_display_syntax_error_import.out deleted file mode 100644 index 48f2e2fb74..0000000000 --- a/test/message/esm_display_syntax_error_import.out +++ /dev/null @@ -1,6 +0,0 @@ -(node:*) ExperimentalWarning: The ESM module loader is experimental. -file:///*/test/message/esm_display_syntax_error_import.mjs:6 - notfound - ^^^^^^^^ -SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports.mjs' does not provide an export named 'notfound' - at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*) diff --git a/test/message/esm_display_syntax_error_import_module.mjs b/test/message/esm_display_syntax_error_import_module.mjs deleted file mode 100644 index a53bbbcd19..0000000000 --- a/test/message/esm_display_syntax_error_import_module.mjs +++ /dev/null @@ -1,4 +0,0 @@ -// Flags: --experimental-modules -/* eslint-disable node-core/required-modules */ -import '../common/index.mjs'; -import '../fixtures/es-module-loaders/syntax-error-import.mjs'; diff --git a/test/message/esm_display_syntax_error_import_module.out b/test/message/esm_display_syntax_error_import_module.out deleted file mode 100644 index 3e1024db8a..0000000000 --- a/test/message/esm_display_syntax_error_import_module.out +++ /dev/null @@ -1,6 +0,0 @@ -(node:*) ExperimentalWarning: The ESM module loader is experimental. -file:///*/test/fixtures/es-module-loaders/syntax-error-import.mjs:1 -import { foo, notfound } from './module-named-exports.mjs'; - ^^^^^^^^ -SyntaxError: The requested module './module-named-exports.mjs' does not provide an export named 'notfound' - at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*) diff --git a/test/message/esm_display_syntax_error_module.mjs b/test/message/esm_display_syntax_error_module.mjs deleted file mode 100644 index 5905d2a954..0000000000 --- a/test/message/esm_display_syntax_error_module.mjs +++ /dev/null @@ -1,4 +0,0 @@ -// Flags: --experimental-modules -/* eslint-disable node-core/required-modules */ -import '../common/index.mjs'; -import '../fixtures/es-module-loaders/syntax-error.mjs'; diff --git a/test/message/esm_display_syntax_error_module.out b/test/message/esm_display_syntax_error_module.out deleted file mode 100644 index e636abad9e..0000000000 --- a/test/message/esm_display_syntax_error_module.out +++ /dev/null @@ -1,6 +0,0 @@ -(node:*) ExperimentalWarning: The ESM module loader is experimental. -file:///*/test/fixtures/es-module-loaders/syntax-error.mjs:2 -await async () => 0; -^^^^^ -SyntaxError: Unexpected reserved word - at translators.set (internal/modules/esm/translators.js:*:*) diff --git a/test/parallel/test-loaders-unknown-builtin-module.mjs b/test/parallel/test-loaders-unknown-builtin-module.mjs deleted file mode 100644 index db3cfa3582..0000000000 --- a/test/parallel/test-loaders-unknown-builtin-module.mjs +++ /dev/null @@ -1,12 +0,0 @@ -// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs -import { expectsError, mustCall } from '../common'; -import assert from 'assert'; - -const unknownBuiltinModule = 'unknown-builtin-module'; - -import(unknownBuiltinModule) -.then(assert.fail, expectsError({ - code: 'ERR_UNKNOWN_BUILTIN_MODULE', - message: `No such built-in module: ${unknownBuiltinModule}` -})) -.then(mustCall()); From ac001a75e861bfa52fbfecd80de2ca0cc006f881 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 24 Oct 2018 16:50:32 +0100 Subject: [PATCH 07/37] esm: minimal resolver spec and implementation refinements PR URL: https://github.com/nodejs/ecmascript-modules/pull/12 --- doc/api/errors.md | 13 +- doc/api/esm.md | 50 ++++++- lib/internal/errors.js | 16 +- lib/internal/modules/esm/default_resolve.js | 15 +- src/module_wrap.cc | 158 +++++++------------- src/node_errors.h | 2 +- test/es-module/test-esm-dynamic-import.js | 2 +- test/es-module/test-esm-loader-search.js | 17 --- test/es-module/test-esm-main-lookup.mjs | 2 +- 9 files changed, 116 insertions(+), 159 deletions(-) delete mode 100644 test/es-module/test-esm-loader-search.js diff --git a/doc/api/errors.md b/doc/api/errors.md index b9d2642149..b1078d57f6 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1452,13 +1452,6 @@ a `dynamicInstantiate` hook. A `MessagePort` was found in the object passed to a `postMessage()` call, but not provided in the `transferList` for that call. - -### ERR_MISSING_MODULE - -> Stability: 1 - Experimental - -An [ES6 module][] could not be resolved. - ### ERR_MISSING_PLATFORM_FOR_WORKER @@ -1466,12 +1459,12 @@ The V8 platform used by this instance of Node.js does not support creating Workers. This is caused by lack of embedder support for Workers. In particular, this error will not occur with standard builds of Node.js. - -### ERR_MODULE_RESOLUTION_LEGACY + +### ERR_MODULE_NOT_FOUND > Stability: 1 - Experimental -A failure occurred resolving imports in an [ES6 module][]. +An [ESM module][] could not be resolved. ### ERR_MULTIPLE_CALLBACK diff --git a/doc/api/esm.md b/doc/api/esm.md index 6eaee64920..d2277044a9 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -63,10 +63,6 @@ ESM must have the `.mjs` extension. You must provide a file extension when using the `import` keyword. -### No importing directories - -There is no support for importing directories. - ### No NODE_PATH `NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks @@ -146,6 +142,52 @@ fs.readFileSync = () => Buffer.from('Hello, ESM'); fs.readFileSync === readFileSync; ``` +## Resolution Algorithm + +### Features + +The resolver has the following properties: + +* FileURL-based resolution as is used by ES modules +* Support for builtin module loading +* Relative and absolute URL resolution +* No default extensions +* No folder mains +* Bare specifier package resolution lookup through node_modules + +### Resolver Algorithm + +The algorithm to resolve an ES module specifier is provided through +_ESM_RESOLVE_: + +**ESM_RESOLVE**(_specifier_, _parentURL_) +> 1. Let _resolvedURL_ be _undefined_. +> 1. If _specifier_ is a valid URL then, +> 1. Set _resolvedURL_ to the result of parsing and reserializing +> _specifier_ as a URL. +> 1. Otherwise, if _specifier_ starts with _"/"_, _"./"_ or _"../"_ then, +> 1. Set _resolvedURL_ to the URL resolution of _specifier_ relative to +> _parentURL_. +> 1. Otherwise, +> 1. Note: _specifier_ is now a bare specifier. +> 1. Set _resolvedURL_ the result of +> **PACKAGE_RESOLVE**(_specifier_, _parentURL_). +> 1. If the file at _resolvedURL_ does not exist then, +> 1. Throw a _Module Not Found_ error. +> 1. Return _resolvedURL_. + +**PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_) +> 1. Assert: _packageSpecifier_ is a bare specifier. +> 1. If _packageSpecifier_ is a Node.js builtin module then, +> 1. Return the string _"node:"_ concatenated with _packageSpecifier_. +> 1. While _parentURL_ contains a non-empty _pathname_, +> 1. Set _parentURL_ to the parent folder URL of _parentURL_. +> 1. Let _packageURL_ be the URL resolution of the string concatenation of +> _parentURL_, _"/node_modules/"_ and _"packageSpecifier"_. +> 1. If the file at _packageURL_ exists then, +> 1. Return _packageURL_. +> 1. Throw a _Module Not Found_ error. + [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename [ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md diff --git a/lib/internal/errors.js b/lib/internal/errors.js index b3b745252a..67081de5ef 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -865,11 +865,13 @@ E('ERR_MISSING_ARGS', E('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', 'The ES Module loader may not return a format of \'dynamic\' when no ' + 'dynamicInstantiate function was provided', Error); -E('ERR_MISSING_MODULE', 'Cannot find module %s', Error); -E('ERR_MODULE_RESOLUTION_LEGACY', - '%s not found by import in %s.' + - ' Legacy behavior in require() would have found it at %s', - Error); +E('ERR_MODULE_NOT_FOUND', (module, base, legacyResolution) => { + let msg = `Cannot find module '${module}' imported from ${base}.`; + if (legacyResolution) + msg += ' Legacy behavior in require() would have found it at ' + + legacyResolution; + return msg; +}, Error); E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times', Error); E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function', TypeError); E('ERR_NAPI_INVALID_DATAVIEW_ARGS', @@ -967,11 +969,13 @@ E('ERR_UNHANDLED_ERROR', if (err === undefined) return msg; return `${msg} (${err})`; }, Error); +// This should probably be a `TypeError`. E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error); E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError); // This should probably be a `TypeError`. -E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s', Error); +E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: \'%s\' imported ' + + 'from %s', Error); E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError); E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError); diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index b9429b8d60..f2aef327ad 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -10,21 +10,15 @@ const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const { - ERR_MISSING_MODULE, - ERR_MODULE_RESOLUTION_LEGACY, + ERR_MODULE_NOT_FOUND, ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); -const StringStartsWith = Function.call.bind(String.prototype.startsWith); const { pathToFileURL, fileURLToPath } = require('internal/url'); const realpathCache = new Map(); function search(target, base) { - if (base === undefined) { - // We cannot search without a base. - throw new ERR_MISSING_MODULE(target); - } try { return moduleWrapResolve(target, base); } catch (e) { @@ -36,7 +30,7 @@ function search(target, base) { tmpMod.paths = CJSmodule._nodeModulePaths( new URL('./', questionedBase).pathname); const found = CJSmodule._resolveFilename(target, tmpMod); - error = new ERR_MODULE_RESOLUTION_LEGACY(target, base, found); + error = new ERR_MODULE_NOT_FOUND(target, fileURLToPath(base), found); } catch { // ignore } @@ -90,12 +84,11 @@ function resolve(specifier, parentURL) { if (isMain) format = 'cjs'; else - throw new ERR_UNKNOWN_FILE_EXTENSION(url.pathname); + throw new ERR_UNKNOWN_FILE_EXTENSION(url.pathname, + fileURLToPath(parentURL)); } return { url: `${url}`, format }; } module.exports = resolve; -// exported for tests -module.exports.search = search; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 012cf702f2..3880551338 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -468,104 +468,37 @@ std::string ReadFile(uv_file file) { return contents; } -enum CheckFileOptions { - LEAVE_OPEN_AFTER_CHECK, - CLOSE_AFTER_CHECK +enum DescriptorType { + NONE, + FILE, + DIRECTORY }; -Maybe CheckFile(const std::string& path, - CheckFileOptions opt = CLOSE_AFTER_CHECK) { +DescriptorType CheckDescriptor(const std::string& path) { uv_fs_t fs_req; - if (path.empty()) { - return Nothing(); - } - - uv_file fd = uv_fs_open(nullptr, &fs_req, path.c_str(), O_RDONLY, 0, nullptr); - uv_fs_req_cleanup(&fs_req); - - if (fd < 0) { - return Nothing(); - } - - uv_fs_fstat(nullptr, &fs_req, fd, nullptr); - uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR; - uv_fs_req_cleanup(&fs_req); - - if (is_directory) { - CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); - uv_fs_req_cleanup(&fs_req); - return Nothing(); - } - - if (opt == CLOSE_AFTER_CHECK) { - CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); + int rc = uv_fs_stat(nullptr, &fs_req, path.c_str(), nullptr); + if (rc == 0) { + uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR; uv_fs_req_cleanup(&fs_req); + return is_directory ? DIRECTORY : FILE; } - - return Just(fd); -} - -enum ResolveExtensionsOptions { - TRY_EXACT_NAME, - ONLY_VIA_EXTENSIONS -}; - -inline bool ResolvesToFile(const URL& search) { - std::string filePath = search.ToFilePath(); - Maybe check = CheckFile(filePath); - return !check.IsNothing(); -} - -template -Maybe ResolveExtensions(const URL& search) { - if (options == TRY_EXACT_NAME) { - std::string filePath = search.ToFilePath(); - Maybe check = CheckFile(filePath); - if (!check.IsNothing()) { - return Just(search); - } - } - - for (const char* extension : EXTENSIONS) { - URL guess(search.path() + extension, &search); - Maybe check = CheckFile(guess.ToFilePath()); - if (!check.IsNothing()) { - return Just(guess); - } - } - - return Nothing(); -} - -inline Maybe ResolveIndex(const URL& search) { - return ResolveExtensions(URL("index", search)); + uv_fs_req_cleanup(&fs_req); + return NONE; } -Maybe ResolveModule(Environment* env, - const std::string& specifier, - const URL& base) { +Maybe PackageResolve(Environment* env, + const std::string& specifier, + const URL& base) { URL parent(".", base); - URL dir(""); + std::string last_path; do { - dir = parent; - Maybe check = - Resolve(env, "./node_modules/" + specifier, dir); - if (!check.IsNothing()) { - const size_t limit = specifier.find('/'); - const size_t spec_len = - limit == std::string::npos ? specifier.length() : - limit + 1; - std::string chroot = - dir.path() + "node_modules/" + specifier.substr(0, spec_len); - if (check.FromJust().path().substr(0, chroot.length()) != chroot) { - return Nothing(); - } - return check; - } else { - // TODO(bmeck) PREVENT FALLTHROUGH - } - parent = URL("..", &dir); - } while (parent.path() != dir.path()); + URL pkg_url("./node_modules/" + specifier, &parent); + DescriptorType check = CheckDescriptor(pkg_url.ToFilePath()); + if (check == FILE) return Just(pkg_url); + last_path = parent.path(); + parent = URL("..", &parent); + // cross-platform root check + } while (parent.path() != last_path); return Nothing(); } @@ -574,26 +507,27 @@ Maybe ResolveModule(Environment* env, Maybe Resolve(Environment* env, const std::string& specifier, const URL& base) { - URL pure_url(specifier); - if (!(pure_url.flags() & URL_FLAGS_FAILED)) { - // just check existence, without altering - Maybe check = CheckFile(pure_url.ToFilePath()); - if (check.IsNothing()) { - return Nothing(); + // Order swapped from spec for minor perf gain. + // Ok since relative URLs cannot parse as URLs. + URL resolved; + if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { + resolved = URL(specifier, base); + } else { + URL pure_url(specifier); + if (!(pure_url.flags() & URL_FLAGS_FAILED)) { + resolved = pure_url; + } else { + return PackageResolve(env, specifier, base); } - return Just(pure_url); - } - if (specifier.length() == 0) { - return Nothing(); } - if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { - URL resolved(specifier, base); - if (ResolvesToFile(resolved)) - return Just(resolved); + DescriptorType check = CheckDescriptor(resolved.ToFilePath()); + if (check != FILE) { + std::string msg = "Cannot find module '" + resolved.ToFilePath() + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); return Nothing(); - } else { - return ResolveModule(env, specifier, base); } + return Just(resolved); } void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { @@ -615,10 +549,18 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { env, "second argument is not a URL string"); } + TryCatchScope try_catch(env); Maybe result = node::loader::Resolve(env, specifier_std, url); - if (result.IsNothing() || (result.FromJust().flags() & URL_FLAGS_FAILED)) { - std::string msg = "Cannot find module " + specifier_std; - return node::THROW_ERR_MISSING_MODULE(env, msg.c_str()); + if (try_catch.HasCaught()) { + try_catch.ReThrow(); + return; + } else if (result.IsNothing() || + (result.FromJust().flags() & URL_FLAGS_FAILED)) { + std::string msg = "Cannot find module '" + specifier_std + + "' imported from " + url.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + try_catch.ReThrow(); + return; } MaybeLocal obj = result.FromJust().ToObject(env); diff --git a/src/node_errors.h b/src/node_errors.h index 28bf6a670f..39c08f317e 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -52,8 +52,8 @@ void FatalException(const v8::FunctionCallbackInfo& args); V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MISSING_ARGS, TypeError) \ V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \ - V(ERR_MISSING_MODULE, Error) \ V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \ + V(ERR_MODULE_NOT_FOUND, Error) \ V(ERR_OUT_OF_RANGE, RangeError) \ V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \ V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \ diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js index 07294d4d24..ca9c99007b 100644 --- a/test/es-module/test-esm-dynamic-import.js +++ b/test/es-module/test-esm-dynamic-import.js @@ -18,7 +18,7 @@ function expectErrorProperty(result, propertyKey, value) { } function expectMissingModuleError(result) { - expectErrorProperty(result, 'code', 'MODULE_NOT_FOUND'); + expectErrorProperty(result, 'code', 'ERR_MODULE_NOT_FOUND'); } function expectOkNamespace(result) { diff --git a/test/es-module/test-esm-loader-search.js b/test/es-module/test-esm-loader-search.js deleted file mode 100644 index 0ca8990cb7..0000000000 --- a/test/es-module/test-esm-loader-search.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict'; -// Flags: --expose-internals - -// This test ensures that search throws errors appropriately - -const common = require('../common'); - -const { search } = require('internal/modules/esm/default_resolve'); - -common.expectsError( - () => search('target', undefined), - { - code: 'ERR_MISSING_MODULE', - type: Error, - message: 'Cannot find module target' - } -); diff --git a/test/es-module/test-esm-main-lookup.mjs b/test/es-module/test-esm-main-lookup.mjs index 76c6263853..19c025beab 100644 --- a/test/es-module/test-esm-main-lookup.mjs +++ b/test/es-module/test-esm-main-lookup.mjs @@ -8,7 +8,7 @@ async function main() { try { mod = await import('../fixtures/es-modules/pjson-main'); } catch (e) { - assert.strictEqual(e.code, 'MODULE_NOT_FOUND'); + assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND'); } assert.strictEqual(mod, undefined); From 07a711648f20a9f4859518745f30f080db90bd4e Mon Sep 17 00:00:00 2001 From: guybedford Date: Thu, 29 Nov 2018 14:26:58 +0200 Subject: [PATCH 08/37] specify import file specifier resolution proposal --- doc/api/esm.md | 127 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 107 insertions(+), 20 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index d2277044a9..6987c7bf55 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -55,10 +55,6 @@ property: ## Notable differences between `import` and `require` -### Only Support for .mjs - -ESM must have the `.mjs` extension. - ### Mandatory file extensions You must provide a file extension when using the `import` keyword. @@ -157,37 +153,128 @@ The resolver has the following properties: ### Resolver Algorithm -The algorithm to resolve an ES module specifier is provided through -_ESM_RESOLVE_: +The algorithm to load an ES module specifier is given through the +**ESM_RESOLVE** method below. It returns the resolved URL for a +module specifier relative to a parentURL, in addition to the unique module +format for that resolved URL given by the **ESM_FORMAT** routine. + +The _"esm"_ format is returned for an ECMAScript Module, while the +_"legacy"_ format is used to indicate loading through the legacy +CommonJS loader. Additional formats such as _"wasm"_ or _"addon"_ can be +extended in future updates. -**ESM_RESOLVE**(_specifier_, _parentURL_) -> 1. Let _resolvedURL_ be _undefined_. -> 1. If _specifier_ is a valid URL then, +In the following algorithms, all subroutine errors are propogated as errors +of these top-level routines. + +_isMain_ is **true** when resolving the Node.js application entry point. + +**ESM_RESOLVE(_specifier_, _parentURL_, _isMain_)** +> 1. Let _resolvedURL_ be **undefined**. +> 1. If _specifier_ is a valid URL, then > 1. Set _resolvedURL_ to the result of parsing and reserializing > _specifier_ as a URL. -> 1. Otherwise, if _specifier_ starts with _"/"_, _"./"_ or _"../"_ then, +> 1. Otherwise, if _specifier_ starts with _"/"_, then +> 1. Throw an _Invalid Specifier_ error. +> 1. Otherwise, if _specifier_ starts with _"./"_ or _"../"_, then > 1. Set _resolvedURL_ to the URL resolution of _specifier_ relative to > _parentURL_. > 1. Otherwise, > 1. Note: _specifier_ is now a bare specifier. > 1. Set _resolvedURL_ the result of > **PACKAGE_RESOLVE**(_specifier_, _parentURL_). -> 1. If the file at _resolvedURL_ does not exist then, +> 1. If the file at _resolvedURL_ does not exist, then > 1. Throw a _Module Not Found_ error. -> 1. Return _resolvedURL_. - -**PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_) -> 1. Assert: _packageSpecifier_ is a bare specifier. -> 1. If _packageSpecifier_ is a Node.js builtin module then, +> 1. Let _format_ be the result of **ESM_FORMAT**(_url_, _isMain_). +> 1. Load _resolvedURL_ as module format, _format_. + +PACKAGE_RESOLVE(_packageSpecifier_, _parentURL_) +> 1. Let _packageName_ be *undefined*. +> 1. Let _packageSubpath_ be *undefined*. +> 1. If _packageSpecifier_ is an empty string, then +> 1. Throw an _Invalid Specifier_ error. +> 1. If _packageSpecifier_ does not start with _"@"_, then +> 1. Set _packageName_ to the substring of _packageSpecifier_ until the +> first _"/"_ separator or the end of the string. +> 1. Otherwise, +> 1. If _packageSpecifier_ does not contain a _"/"_ separator, then +> 1. Throw an _Invalid Specifier_ error. +> 1. Set _packageName_ to the substring of _packageSpecifier_ +> until the second _"/"_ separator or the end of the string. +> 1. Let _packageSubpath_ be the substring of _packageSpecifier_ from the +> position at the length of _packageName_ plus one, if any. +> 1. Assert: _packageName_ is a valid package name or scoped package name. +> 1. Assert: _packageSubpath_ is either empty, or a path without a leading +> separator. +> 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent +> encoded strings for _"/"_ or _"\"_ then, +> 1. Throw an _Invalid Specifier_ error. +> 1. If _packageSubpath_ is empty and _packageName_ is a Node.js builtin +> module, then > 1. Return the string _"node:"_ concatenated with _packageSpecifier_. -> 1. While _parentURL_ contains a non-empty _pathname_, +> 1. While _parentURL_ is not the file system root, > 1. Set _parentURL_ to the parent folder URL of _parentURL_. > 1. Let _packageURL_ be the URL resolution of the string concatenation of -> _parentURL_, _"/node_modules/"_ and _"packageSpecifier"_. -> 1. If the file at _packageURL_ exists then, -> 1. Return _packageURL_. +> _parentURL_, _"/node_modules/"_ and _packageSpecifier_. +> 1. If the folder at _packageURL_ does not exist, then +> 1. Set _parentURL_ to the parent URL path of _parentURL_. +> 1. Continue the next loop iteration. +> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_). +> 1. If _packageSubpath_ is empty, then +> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, +> _pjson_). +> 1. Otherwise, +> 1. Return the URL resolution of _packageSubpath_ in _packageURL_. > 1. Throw a _Module Not Found_ error. +PACKAGE_MAIN_RESOLVE(_packageURL_, _pjson_) +> 1. If _pjson_ is **null**, then +> 1. Throw a _Module Not Found_ error. +> 1. If _pjson.main_ is a String, then +> 1. Let _resolvedMain_ be the concatenation of _packageURL_, "/", and +> _pjson.main_. +> 1. If the file at _resolvedMain_ exists, then +> 1. Return _resolvedMain_. +> 1. If _pjson.type_ is equal to _"esm"_, then +> 1. Throw a _Module Not Found_ error. +> 1. Let _legacyMainURL_ be the result applying the legacy +> **LOAD_AS_DIRECTORY** CommonJS resolver to _packageURL_, throwing a +> _Module Not Found_ error for no resolution. +> 1. If _legacyMainURL_ does not end in _".js"_ then, +> 1. Throw an _Unsupported File Extension_ error. +> 1. Return _legacyMainURL_. + +**ESM_FORMAT(_url_, _isMain_)** +> 1. Assert: _url_ corresponds to an existing file. +> 1. Let _pjson_ be the result of **READ_PACKAGE_BOUNDARY**(_url_). +> 1. If _pjson_ is **null** and _isMain_ is **true**, then +> 1. Return _"legacy"_. +> 1. If _pjson.type_ exists and is _"esm"_, then +> 1. If _url_ does not end in _".js"_ or _".mjs"_, then +> 1. Throw an _Unsupported File Extension_ error. +> 1. Return _"esm"_. +> 1. Otherwise, +> 1. If _url_ ends in _".mjs"_, then +> 1. Return _"esm"_. +> 1. Otherwise, +> 1. Return _"legacy"_. + +READ_PACKAGE_BOUNDARY(_url_) +> 1. Let _boundaryURL_ be _url_. +> 1. While _boundaryURL_ is not the file system root, +> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_boundaryURL_). +> 1. If _pjson_ is not **null**, then +> 1. Return _pjson_. +> 1. Set _boundaryURL_ to the parent URL of _boundaryURL_. +> 1. Return **null**. + +READ_PACKAGE_JSON(_packageURL_) +> 1. Let _pjsonURL_ be the resolution of _"package.json"_ within _packageURL_. +> 1. If the file at _pjsonURL_ does not exist, then +> 1. Return **null**. +> 1. If the file at _packageURL_ does not parse as valid JSON, then +> 1. Throw an _Invalid Package Configuration_ error. +> 1. Return the parsed JSON source of the file at _pjsonURL_. + [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename [ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md From fedb9f0358c7cd67c0e155df1664e67df2149744 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Thu, 24 Jan 2019 19:33:28 +0200 Subject: [PATCH 09/37] irp type implementation --- lib/internal/errors.js | 11 +- lib/internal/modules/esm/default_resolve.js | 67 ++-- lib/internal/modules/esm/loader.js | 5 +- lib/internal/modules/esm/module_job.js | 2 +- lib/internal/modules/esm/translators.js | 13 +- lib/internal/process/esm_loader.js | 4 +- src/env.h | 27 +- src/module_wrap.cc | 395 ++++++++++++++++++-- src/module_wrap.h | 11 +- src/node_errors.h | 2 + test/message/esm_display_syntax_error.out | 2 +- 11 files changed, 423 insertions(+), 116 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 67081de5ef..3a013de377 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -865,13 +865,6 @@ E('ERR_MISSING_ARGS', E('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', 'The ES Module loader may not return a format of \'dynamic\' when no ' + 'dynamicInstantiate function was provided', Error); -E('ERR_MODULE_NOT_FOUND', (module, base, legacyResolution) => { - let msg = `Cannot find module '${module}' imported from ${base}.`; - if (legacyResolution) - msg += ' Legacy behavior in require() would have found it at ' + - legacyResolution; - return msg; -}, Error); E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times', Error); E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function', TypeError); E('ERR_NAPI_INVALID_DATAVIEW_ARGS', @@ -974,10 +967,10 @@ E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error); E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError); // This should probably be a `TypeError`. -E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: \'%s\' imported ' + - 'from %s', Error); E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError); E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError); +E('ERR_UNSUPPORTED_FILE_EXTENSION', 'Unsupported file extension: \'%s\' ' + + 'imported from %s', Error); E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl', diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index f2aef327ad..1e784e710a 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -1,7 +1,5 @@ 'use strict'; -const { URL } = require('url'); -const CJSmodule = require('internal/modules/cjs/loader'); const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { extname } = require('path'); @@ -9,38 +7,24 @@ const { realpathSync } = require('fs'); const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); -const { - ERR_MODULE_NOT_FOUND, - ERR_UNKNOWN_FILE_EXTENSION -} = require('internal/errors').codes; +const { ERR_UNSUPPORTED_FILE_EXTENSION } = require('internal/errors').codes; const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); const { pathToFileURL, fileURLToPath } = require('internal/url'); const realpathCache = new Map(); -function search(target, base) { - try { - return moduleWrapResolve(target, base); - } catch (e) { - e.stack; // cause V8 to generate stack before rethrow - let error = e; - try { - const questionedBase = new URL(base); - const tmpMod = new CJSmodule(questionedBase.pathname, null); - tmpMod.paths = CJSmodule._nodeModulePaths( - new URL('./', questionedBase).pathname); - const found = CJSmodule._resolveFilename(target, tmpMod); - error = new ERR_MODULE_NOT_FOUND(target, fileURLToPath(base), found); - } catch { - // ignore - } - throw error; - } -} - const extensionFormatMap = { '__proto__': null, - '.mjs': 'esm' + '.mjs': 'esm', + '.js': 'esm' +}; + +const legacyExtensionFormatMap = { + '__proto__': null, + '.js': 'cjs', + '.json': 'cjs', + '.mjs': 'esm', + '.node': 'cjs' }; function resolve(specifier, parentURL) { @@ -51,21 +35,14 @@ function resolve(specifier, parentURL) { }; } - let url; - try { - url = search(specifier, - parentURL || pathToFileURL(`${process.cwd()}/`).href); - } catch (e) { - if (typeof e.message === 'string' && - StringStartsWith(e.message, 'Cannot find module')) { - e.code = 'MODULE_NOT_FOUND'; - // TODO: also add e.requireStack to match behavior with CJS - // MODULE_NOT_FOUND. - } - throw e; - } - const isMain = parentURL === undefined; + if (isMain) + parentURL = pathToFileURL(`${process.cwd()}/`).href; + + const resolved = moduleWrapResolve(specifier, parentURL, isMain); + + let url = resolved.url; + const legacy = resolved.legacy; if (isMain ? !preserveSymlinksMain : !preserveSymlinks) { const real = realpathSync(fileURLToPath(url), { @@ -78,14 +55,14 @@ function resolve(specifier, parentURL) { } const ext = extname(url.pathname); + let format = (legacy ? legacyExtensionFormatMap : extensionFormatMap)[ext]; - let format = extensionFormatMap[ext]; if (!format) { if (isMain) - format = 'cjs'; + format = legacy ? 'cjs' : 'esm'; else - throw new ERR_UNKNOWN_FILE_EXTENSION(url.pathname, - fileURLToPath(parentURL)); + throw new ERR_UNSUPPORTED_FILE_EXTENSION(fileURLToPath(url), + fileURLToPath(parentURL)); } return { url: `${url}`, format }; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index a1a1621909..1776d3da48 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -14,7 +14,7 @@ const ModuleJob = require('internal/modules/esm/module_job'); const defaultResolve = require('internal/modules/esm/default_resolve'); const createDynamicModule = require( 'internal/modules/esm/create_dynamic_module'); -const translators = require('internal/modules/esm/translators'); +const { translators } = require('internal/modules/esm/translators'); const FunctionBind = Function.call.bind(Function.prototype.bind); @@ -32,6 +32,9 @@ class Loader { // Registry of loaded modules, akin to `require.cache` this.moduleMap = new ModuleMap(); + // map of already-loaded CJS modules to use + this.cjsCache = new Map(); + // The resolver has the signature // (specifier : string, parentURL : string, defaultResolve) // -> Promise<{ url : string, format: string }> diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 016495096c..5cbf4e3126 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -23,7 +23,7 @@ class ModuleJob { // This is a Promise<{ module, reflect }>, whose fields will be copied // onto `this` by `link()` below once it has been resolved. - this.modulePromise = moduleProvider(url, isMain); + this.modulePromise = moduleProvider.call(loader, url, isMain); this.module = undefined; this.reflect = undefined; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 46e3bdd998..4fc0d70b53 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -23,7 +23,7 @@ const StringReplace = Function.call.bind(String.prototype.replace); const debug = debuglog('esm'); const translators = new SafeMap(); -module.exports = translators; +exports.translators = translators; function initializeImportMeta(meta, { url }) { meta.url = url; @@ -35,7 +35,7 @@ async function importModuleDynamically(specifier, { url }) { } // Strategy for loading a standard JavaScript module -translators.set('esm', async (url) => { +translators.set('esm', async function(url) { const source = `${await readFileAsync(new URL(url))}`; debug(`Translating StandardModule ${url}`); const module = new ModuleWrap(stripShebang(source), url); @@ -52,9 +52,14 @@ translators.set('esm', async (url) => { // Strategy for loading a node-style CommonJS module const isWindows = process.platform === 'win32'; const winSepRegEx = /\//g; -translators.set('cjs', async (url, isMain) => { +translators.set('cjs', async function(url, isMain) { debug(`Translating CJSModule ${url}`); const pathname = internalURLModule.fileURLToPath(new URL(url)); + const cached = this.cjsCache.get(url); + if (cached) { + this.cjsCache.delete(url); + return cached; + } const module = CJSModule._cache[ isWindows ? StringReplace(pathname, winSepRegEx, '\\') : pathname]; if (module && module.loaded) { @@ -74,7 +79,7 @@ translators.set('cjs', async (url, isMain) => { // Strategy for loading a node builtin CommonJS module that isn't // through normal resolution -translators.set('builtin', async (url) => { +translators.set('builtin', async function(url) { debug(`Translating BuiltinModule ${url}`); // slice 'node:' scheme const id = url.slice(5); diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 6ed43970cb..98e1f26d94 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -33,9 +33,7 @@ exports.importModuleDynamicallyCallback = async function(wrap, specifier) { }; let loaderResolve; -exports.loaderPromise = new Promise((resolve, reject) => { - loaderResolve = resolve; -}); +exports.loaderPromise = new Promise((resolve) => loaderResolve = resolve); exports.ESMLoader = undefined; diff --git a/src/env.h b/src/env.h index 5ab3e1baf3..aebde14b40 100644 --- a/src/env.h +++ b/src/env.h @@ -73,14 +73,23 @@ namespace loader { class ModuleWrap; struct PackageConfig { - enum class Exists { Yes, No }; - enum class IsValid { Yes, No }; - enum class HasMain { Yes, No }; - - Exists exists; - IsValid is_valid; - HasMain has_main; - std::string main; + struct Exists { + enum Bool { No, Yes }; + }; + struct IsValid { + enum Bool { No, Yes }; + }; + struct HasMain { + enum Bool { No, Yes }; + }; + struct IsESM { + enum Bool { No, Yes }; + }; + const Exists::Bool exists; + const IsValid::Bool is_valid; + const HasMain::Bool has_main; + const std::string main; + const IsESM::Bool esm; }; } // namespace loader @@ -169,6 +178,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(env_var_settings_string, "envVarSettings") \ V(errno_string, "errno") \ V(error_string, "error") \ + V(esm_string, "esm") \ V(exchange_string, "exchange") \ V(exit_code_string, "exitCode") \ V(expire_string, "expire") \ @@ -207,6 +217,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(kill_signal_string, "killSignal") \ V(kind_string, "kind") \ V(library_string, "library") \ + V(legacy_string, "legacy") \ V(mac_string, "mac") \ V(main_string, "main") \ V(max_buffer_string, "maxBuffer") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 3880551338..a736a2d38a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -43,8 +43,6 @@ using v8::String; using v8::Undefined; using v8::Value; -static const char* const EXTENSIONS[] = {".mjs"}; - ModuleWrap::ModuleWrap(Environment* env, Local object, Local module, @@ -469,14 +467,30 @@ std::string ReadFile(uv_file file) { } enum DescriptorType { - NONE, FILE, - DIRECTORY + DIRECTORY, + NONE }; -DescriptorType CheckDescriptor(const std::string& path) { +// When DescriptorType cache is added, this can also return +// Nothing for the "null" cache entries. +inline Maybe OpenDescriptor(const std::string& path) { + uv_fs_t fs_req; + uv_file fd = uv_fs_open(nullptr, &fs_req, path.c_str(), O_RDONLY, 0, nullptr); + uv_fs_req_cleanup(&fs_req); + if (fd < 0) return Nothing(); + return Just(fd); +} + +inline void CloseDescriptor(uv_file fd) { + uv_fs_t fs_req; + uv_fs_close(nullptr, &fs_req, fd, nullptr); + uv_fs_req_cleanup(&fs_req); +} + +inline DescriptorType CheckDescriptorAtFile(uv_file fd) { uv_fs_t fs_req; - int rc = uv_fs_stat(nullptr, &fs_req, path.c_str(), nullptr); + int rc = uv_fs_fstat(nullptr, &fs_req, fd, nullptr); if (rc == 0) { uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR; uv_fs_req_cleanup(&fs_req); @@ -486,27 +500,320 @@ DescriptorType CheckDescriptor(const std::string& path) { return NONE; } -Maybe PackageResolve(Environment* env, +// TODO(@guybedford): Add a DescriptorType cache layer here. +// Should be directory based -> if path/to/dir doesn't exist +// then the cache should early-fail any path/to/dir/file check. +DescriptorType CheckDescriptorAtPath(const std::string& path) { + Maybe fd = OpenDescriptor(path); + if (fd.IsNothing()) return NONE; + DescriptorType type = CheckDescriptorAtFile(fd.FromJust()); + CloseDescriptor(fd.FromJust()); + return type; +} + +Maybe ReadIfFile(const std::string& path) { + Maybe fd = OpenDescriptor(path); + if (fd.IsNothing()) return Nothing(); + DescriptorType type = CheckDescriptorAtFile(fd.FromJust()); + if (type != FILE) return Nothing(); + std::string source = ReadFile(fd.FromJust()); + CloseDescriptor(fd.FromJust()); + return Just(source); +} + +using Exists = PackageConfig::Exists; +using IsValid = PackageConfig::IsValid; +using HasMain = PackageConfig::HasMain; +using IsESM = PackageConfig::IsESM; + +Maybe GetPackageConfig(Environment* env, + const std::string& path, + const URL& base) { + auto existing = env->package_json_cache.find(path); + if (existing != env->package_json_cache.end()) { + return Just(&existing->second); + } + + Maybe source = ReadIfFile(path); + + if (source.IsNothing()) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "", + IsESM::No }); + return Just(&entry.first->second); + } + + std::string pkg_src = source.FromJust(); + + Isolate* isolate = env->isolate(); + v8::HandleScope handle_scope(isolate); + + bool parsed = false; + Local pkg_json; + { + Local src; + Local pkg_json_v; + if (String::NewFromUtf8(isolate, + pkg_src.c_str(), + v8::NewStringType::kNormal, + pkg_src.length()).ToLocal(&src) && + v8::JSON::Parse(env->context(), src).ToLocal(&pkg_json_v) && + pkg_json_v->ToObject(env->context()).ToLocal(&pkg_json)) { + parsed = true; + } + } + + if (!parsed) { + (void)env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "", + IsESM::No }); + std::string msg = "Invalid JSON in '" + path + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); + return Nothing(); + } + + Local pkg_main; + HasMain::Bool has_main = HasMain::No; + std::string main_std; + if (pkg_json->Get(env->context(), env->main_string()).ToLocal(&pkg_main)) { + if (pkg_main->IsString()) { + has_main = HasMain::Yes; + } + Utf8Value main_utf8(isolate, pkg_main); + main_std.assign(std::string(*main_utf8, main_utf8.length())); + } + + IsESM::Bool esm = IsESM::No; + Local type_v; + if (pkg_json->Get(env->context(), env->type_string()).ToLocal(&type_v)) { + if (type_v->StrictEquals(env->esm_string())) { + esm = IsESM::Yes; + } + } + + Local exports_v; + if (pkg_json->Get(env->context(), + env->exports_string()).ToLocal(&exports_v) && + (exports_v->IsObject() || exports_v->IsString() || + exports_v->IsBoolean())) { + Persistent exports; + // esm = IsESM::Yes; + exports.Reset(env->isolate(), exports_v); + + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::Yes, has_main, main_std, + esm }); + return Just(&entry.first->second); + } + + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::Yes, has_main, main_std, + esm }); + return Just(&entry.first->second); +} + +Maybe GetPackageBoundaryConfig(Environment* env, + const URL& search, + const URL& base) { + URL pjson_url("package.json", &search); + while (true) { + Maybe pkg_cfg = + GetPackageConfig(env, pjson_url.ToFilePath(), base); + if (pkg_cfg.IsNothing()) return pkg_cfg; + if (pkg_cfg.FromJust()->exists == Exists::Yes) return pkg_cfg; + + URL last_pjson_url = pjson_url; + pjson_url = URL("../package.json", pjson_url); + + // Terminates at root where ../package.json equals ../../package.json + // (can't just check "/package.json" for Windows support). + if (pjson_url.path() == last_pjson_url.path()) { + auto entry = env->package_json_cache.emplace(pjson_url.ToFilePath(), + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "", + IsESM::No }); + return Just(&entry.first->second); + } + } +} + +/* + * Legacy CommonJS main resolution: + * 1. let M = pkg_url + (json main field) + * 2. TRY(M, M.js, M.json, M.node) + * 3. TRY(M/index.js, M/index.json, M/index.node) + * 4. TRY(pkg_url/index.js, pkg_url/index.json, pkg_url/index.node) + * 5. NOT_FOUND + */ +inline bool FileExists(const URL& url) { + return CheckDescriptorAtPath(url.ToFilePath()) == FILE; +} +Maybe LegacyMainResolve(const URL& pjson_url, + const PackageConfig& pcfg) { + URL guess; + if (pcfg.has_main == HasMain::Yes) { + // Note: fs check redundances will be handled by Descriptor cache here. + if (FileExists(guess = URL("./" + pcfg.main, pjson_url))) { + return Just(guess); + } + if (FileExists(guess = URL("./" + pcfg.main + ".js", pjson_url))) { + return Just(guess); + } + if (FileExists(guess = URL("./" + pcfg.main + ".json", pjson_url))) { + return Just(guess); + } + if (FileExists(guess = URL("./" + pcfg.main + ".node", pjson_url))) { + return Just(guess); + } + if (FileExists(guess = URL("./" + pcfg.main + "/index.js", pjson_url))) { + return Just(guess); + } + // Such stat. + if (FileExists(guess = URL("./" + pcfg.main + "/index.json", pjson_url))) { + return Just(guess); + } + if (FileExists(guess = URL("./" + pcfg.main + "/index.node", pjson_url))) { + return Just(guess); + } + // Fallthrough. + } + if (FileExists(guess = URL("./index.js", pjson_url))) { + return Just(guess); + } + // So fs. + if (FileExists(guess = URL("./index.json", pjson_url))) { + return Just(guess); + } + if (FileExists(guess = URL("./index.node", pjson_url))) { + return Just(guess); + } + // Not found. + return Nothing(); +} + +Maybe FinalizeResolution(Environment* env, + const URL& resolved, + const URL& base, + bool check_exists, + bool is_main) { + const std::string& path = resolved.ToFilePath(); + + if (check_exists && CheckDescriptorAtPath(path) != FILE) { + std::string msg = "Cannot find module '" + path + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + return Nothing(); + } + + Maybe pcfg = + GetPackageBoundaryConfig(env, resolved, base); + if (pcfg.IsNothing()) return Nothing(); + + if (pcfg.FromJust()->exists == Exists::No) { + return Just(ModuleResolution { resolved, true }); + } + + return Just(ModuleResolution { + resolved, pcfg.FromJust()->esm == IsESM::No }); +} + +Maybe PackageMainResolve(Environment* env, + const URL& pjson_url, + const PackageConfig& pcfg, + const URL& base) { + if (pcfg.exists == Exists::No || ( + pcfg.esm == IsESM::Yes && pcfg.has_main == HasMain::No)) { + std::string msg = "Cannot find main entry point for '" + + URL(".", pjson_url).ToFilePath() + "' imported from " + + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + return Nothing(); + } + if (pcfg.has_main == HasMain::Yes && + pcfg.main.substr(pcfg.main.length() - 4, 4) == ".mjs") { + return FinalizeResolution(env, URL(pcfg.main, pjson_url), base, true, + true); + } + if (pcfg.esm == IsESM::Yes && + pcfg.main.substr(pcfg.main.length() - 3, 3) == ".js") { + return FinalizeResolution(env, URL(pcfg.main, pjson_url), base, true, + true); + } + + Maybe resolved = LegacyMainResolve(pjson_url, pcfg); + // Legacy main resolution error + if (resolved.IsNothing()) { + return Nothing(); + } + return FinalizeResolution(env, resolved.FromJust(), base, false, true); +} + +Maybe PackageResolve(Environment* env, const std::string& specifier, const URL& base) { - URL parent(".", base); + size_t sep_index = specifier.find('/'); + if (specifier[0] == '@' && (sep_index == std::string::npos || + specifier.length() == 0)) { + std::string msg = "Invalid package name '" + specifier + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_MODULE_SPECIFIER(env, msg.c_str()); + return Nothing(); + } + bool scope = false; + if (specifier[0] == '@') { + scope = true; + sep_index = specifier.find('/', sep_index + 1); + } + std::string pkg_name = specifier.substr(0, + sep_index == std::string::npos ? std::string::npos : sep_index); + std::string pkg_subpath; + if ((sep_index == std::string::npos || + sep_index == specifier.length() - 1)) { + pkg_subpath = ""; + } else { + pkg_subpath = "." + specifier.substr(sep_index); + } + URL pjson_url("./node_modules/" + pkg_name + "/package.json", &base); + std::string pjson_path = pjson_url.ToFilePath(); std::string last_path; do { - URL pkg_url("./node_modules/" + specifier, &parent); - DescriptorType check = CheckDescriptor(pkg_url.ToFilePath()); - if (check == FILE) return Just(pkg_url); - last_path = parent.path(); - parent = URL("..", &parent); - // cross-platform root check - } while (parent.path() != last_path); - return Nothing(); + DescriptorType check = + CheckDescriptorAtPath(pjson_path.substr(0, pjson_path.length() - 13)); + if (check != DIRECTORY) { + last_path = pjson_path; + pjson_url = URL((scope ? + "../../../../node_modules/" : "../../../node_modules/") + + pkg_name + "/package.json", &pjson_url); + pjson_path = pjson_url.ToFilePath(); + continue; + } + + // Package match. + Maybe pcfg = GetPackageConfig(env, pjson_path, base); + // Invalid package configuration error. + if (pcfg.IsNothing()) return Nothing(); + if (!pkg_subpath.length()) { + return PackageMainResolve(env, pjson_url, *pcfg.FromJust(), base); + } else { + return FinalizeResolution(env, URL(pkg_subpath, pjson_url), + base, true, false); + } + CHECK(false); + // Cross-platform root check. + } while (pjson_url.path().length() != last_path.length()); + + std::string msg = "Cannot find package '" + pkg_name + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + return Nothing(); } } // anonymous namespace -Maybe Resolve(Environment* env, - const std::string& specifier, - const URL& base) { +Maybe Resolve(Environment* env, + const std::string& specifier, + const URL& base, + bool is_main) { // Order swapped from spec for minor perf gain. // Ok since relative URLs cannot parse as URLs. URL resolved; @@ -520,21 +827,14 @@ Maybe Resolve(Environment* env, return PackageResolve(env, specifier, base); } } - DescriptorType check = CheckDescriptor(resolved.ToFilePath()); - if (check != FILE) { - std::string msg = "Cannot find module '" + resolved.ToFilePath() + - "' imported from " + base.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); - return Nothing(); - } - return Just(resolved); + return FinalizeResolution(env, resolved, base, true, is_main); } void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - // module.resolve(specifier, url) - CHECK_EQ(args.Length(), 2); + // module.resolve(specifier, url, is_main) + CHECK_EQ(args.Length(), 3); CHECK(args[0]->IsString()); Utf8Value specifier_utf8(env->isolate(), args[0]); @@ -544,28 +844,41 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { Utf8Value url_utf8(env->isolate(), args[1]); URL url(*url_utf8, url_utf8.length()); + CHECK(args[2]->IsBoolean()); + if (url.flags() & URL_FLAGS_FAILED) { return node::THROW_ERR_INVALID_ARG_TYPE( env, "second argument is not a URL string"); } TryCatchScope try_catch(env); - Maybe result = node::loader::Resolve(env, specifier_std, url); - if (try_catch.HasCaught()) { - try_catch.ReThrow(); - return; - } else if (result.IsNothing() || - (result.FromJust().flags() & URL_FLAGS_FAILED)) { - std::string msg = "Cannot find module '" + specifier_std + - "' imported from " + url.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + Maybe result = + node::loader::Resolve(env, specifier_std, url, args[2]->IsTrue()); + if (result.IsNothing()) { + CHECK(try_catch.HasCaught()); try_catch.ReThrow(); return; } + CHECK(!try_catch.HasCaught()); + + ModuleResolution resolution = result.FromJust(); + CHECK(!(resolution.url.flags() & URL_FLAGS_FAILED)); + + Local resolved = Object::New(env->isolate()); + + resolved->DefineOwnProperty( + env->context(), + env->url_string(), + resolution.url.ToObject(env).ToLocalChecked(), + v8::ReadOnly).FromJust(); + + resolved->DefineOwnProperty( + env->context(), + env->legacy_string(), + v8::Boolean::New(env->isolate(), resolution.legacy), + v8::ReadOnly).FromJust(); - MaybeLocal obj = result.FromJust().ToObject(env); - if (!obj.IsEmpty()) - args.GetReturnValue().Set(obj.ToLocalChecked()); + args.GetReturnValue().Set(resolved); } static MaybeLocal ImportModuleDynamically( diff --git a/src/module_wrap.h b/src/module_wrap.h index d3089202ba..58f052e6ec 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -24,9 +24,14 @@ enum HostDefinedOptions : int { kLength = 10, }; -v8::Maybe Resolve(Environment* env, - const std::string& specifier, - const url::URL& base); +struct ModuleResolution { + url::URL url; + bool legacy; +}; + +v8::Maybe Resolve(Environment* env, + const std::string& specifier, + const url::URL& base); class ModuleWrap : public BaseObject { public: diff --git a/src/node_errors.h b/src/node_errors.h index 39c08f317e..bcf4154ff5 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -48,6 +48,8 @@ void FatalException(const v8::FunctionCallbackInfo& args); V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \ V(ERR_INVALID_ARG_VALUE, TypeError) \ V(ERR_INVALID_ARG_TYPE, TypeError) \ + V(ERR_INVALID_MODULE_SPECIFIER, TypeError) \ + V(ERR_INVALID_PACKAGE_CONFIG, SyntaxError) \ V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \ V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MISSING_ARGS, TypeError) \ diff --git a/test/message/esm_display_syntax_error.out b/test/message/esm_display_syntax_error.out index ed2e928eb1..2700fd894c 100644 --- a/test/message/esm_display_syntax_error.out +++ b/test/message/esm_display_syntax_error.out @@ -3,4 +3,4 @@ file:///*/test/message/esm_display_syntax_error.mjs:3 await async () => 0; ^^^^^ SyntaxError: Unexpected reserved word - at translators.set (internal/modules/esm/translators.js:*:*) + at Loader. (internal/modules/esm/translators.js:*:*) From 5b9c060060d341fb07ef35aa4d8a89b8971c3358 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 11 Feb 2019 00:26:38 +0200 Subject: [PATCH 10/37] esm: top-level --type, -m flags, spec updates --- doc/api/cli.md | 12 ++++++++++++ doc/api/esm.md | 28 ++++++++++++++++++++-------- doc/node.1 | 3 +++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index f8fa53a5cc..de822b6a06 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -504,6 +504,18 @@ added: v2.4.0 Track heap object allocations for heap snapshots. +### `-m`, `--type=type` + +When using `--experimental-modules`, this informs the module resolution type +to interpret the top-level entry into Node.js. + +Works with stdin, `--eval`, `--print` as well as standard execution. + +Valid values are `"commonjs"` and `"module"`, where the default is to infer +from the file extension and package type boundary. + +`-m` is an alias for `--type=module`. + ### `--use-bundled-ca`, `--use-openssl-ca` + +Specify the `file` of the custom [experimental ECMAScript Module][] loader. + ### `--max-http-header-size=size` + +To customize the default module resolution, loader hooks can optionally be +provided via a `--loader ./loader-name.mjs` argument to Node.js. + +When hooks are used they only apply to ES module loading and not to any +CommonJS modules loaded. + +### Resolve hook + +The resolve hook returns the resolved file URL and module format for a +given module specifier and parent file URL: + +```js +const baseURL = new URL('file://'); +baseURL.pathname = `${process.cwd()}/`; + +export async function resolve(specifier, + parentModuleURL = baseURL, + defaultResolver) { + return { + url: new URL(specifier, parentModuleURL).href, + format: 'esm' + }; +} +``` + +The `parentModuleURL` is provided as `undefined` when performing main Node.js +load itself. + +The default Node.js ES module resolution function is provided as a third +argument to the resolver for easy compatibility workflows. + +In addition to returning the resolved file URL value, the resolve hook also +returns a `format` property specifying the module format of the resolved +module. This can be one of the following: + +| `format` | Description | +| --- | --- | +| `'module'` | Load a standard JavaScript module | +| `'commonjs'` | Load a Node.js CommonJS module | +| `'builtin'` | Load a Node.js builtin module | +| `'dynamic'` | Use a [dynamic instantiate hook][] | + +For example, a dummy loader to load JavaScript restricted to browser resolution +rules with only JS file extension and Node.js builtin modules support could +be written: + +```js +import path from 'path'; +import process from 'process'; +import Module from 'module'; + +const builtins = Module.builtinModules; +const JS_EXTENSIONS = new Set(['.js', '.mjs']); + +const baseURL = new URL('file://'); +baseURL.pathname = `${process.cwd()}/`; + +export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { + if (builtins.includes(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { + // For node_modules support: + // return defaultResolve(specifier, parentModuleURL); + throw new Error( + `imports must begin with '/', './', or '../'; '${specifier}' does not`); + } + const resolved = new URL(specifier, parentModuleURL); + const ext = path.extname(resolved.pathname); + if (!JS_EXTENSIONS.has(ext)) { + throw new Error( + `Cannot load file with non-JavaScript file extension ${ext}.`); + } + return { + url: resolved.href, + format: 'esm' + }; +} +``` + +With this loader, running: + +```console +NODE_OPTIONS='--experimental-modules --loader ./custom-loader.mjs' node x.js +``` + +would load the module `x.js` as an ES module with relative resolution support +(with `node_modules` loading skipped in this example). + +### Dynamic instantiate hook + +To create a custom dynamic module that doesn't correspond to one of the +existing `format` interpretations, the `dynamicInstantiate` hook can be used. +This hook is called only for modules that return `format: 'dynamic'` from +the `resolve` hook. + +```js +export async function dynamicInstantiate(url) { + return { + exports: ['customExportName'], + execute: (exports) => { + // Get and set functions provided for pre-allocated export names + exports.customExportName.set('value'); + } + }; +} +``` + +With the list of module exports provided upfront, the `execute` function will +then be called at the exact point of module evaluation order for that module +in the import tree. + [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md +[dynamic instantiate hook]: #esm_dynamic_instantiate_hook [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename [ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index e405dc8b30..ed387a1554 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -14,6 +14,7 @@ if (type && type !== 'commonjs' && type !== 'module') exports.typeFlag = type; const { Loader } = require('internal/modules/esm/loader'); +const { pathToFileURL } = require('internal/url'); const { wrapToModuleMap, } = require('internal/vm/source_text_module'); @@ -44,8 +45,15 @@ exports.loaderPromise = new Promise((resolve) => loaderResolve = resolve); exports.ESMLoader = undefined; exports.initializeLoader = function(cwd, userLoader) { - const ESMLoader = new Loader(); + let ESMLoader = new Loader(); const loaderPromise = (async () => { + if (userLoader) { + const hooks = await ESMLoader.import( + userLoader, pathToFileURL(`${cwd}/`).href); + ESMLoader = new Loader(); + ESMLoader.hook(hooks); + exports.ESMLoader = ESMLoader; + } return ESMLoader; })(); loaderResolve(loaderPromise); diff --git a/src/node_options.cc b/src/node_options.cc index e76e1a0282..413169fde4 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -101,6 +101,10 @@ void PerIsolateOptions::CheckOptions(std::vector* errors) { } void EnvironmentOptions::CheckOptions(std::vector* errors) { + if (!userland_loader.empty() && !experimental_modules) { + errors->push_back("--loader requires --experimental-modules be enabled"); + } + if (syntax_check_only && has_eval_string) { errors->push_back("either --check or --eval can be used, not both"); } @@ -242,6 +246,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "(default: llhttp).", &EnvironmentOptions::http_parser, kAllowedInEnvironment); + AddOption("--loader", + "(with --experimental-modules) use the specified file as a " + "custom loader", + &EnvironmentOptions::userland_loader, + kAllowedInEnvironment); AddOption("--no-deprecation", "silence deprecation warnings", &EnvironmentOptions::no_deprecation, diff --git a/src/node_options.h b/src/node_options.h index ee4cc98c9b..83c3be8674 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -105,6 +105,7 @@ class EnvironmentOptions : public Options { bool trace_deprecation = false; bool trace_sync_io = false; bool trace_warnings = false; + std::string userland_loader; bool syntax_check_only = false; bool has_eval_string = false; diff --git a/test/es-module/test-esm-example-loader.js b/test/es-module/test-esm-example-loader.js new file mode 100644 index 0000000000..0b0001acea --- /dev/null +++ b/test/es-module/test-esm-example-loader.js @@ -0,0 +1,6 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/example-loader.mjs +/* eslint-disable node-core/required-modules */ +import assert from 'assert'; +import ok from '../fixtures/es-modules/test-esm-ok.mjs'; + +assert(ok); diff --git a/test/es-module/test-esm-loader-dependency.mjs b/test/es-module/test-esm-loader-dependency.mjs new file mode 100644 index 0000000000..1ed8685a6f --- /dev/null +++ b/test/es-module/test-esm-loader-dependency.mjs @@ -0,0 +1,5 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-with-dep.mjs +/* eslint-disable node-core/required-modules */ +import '../fixtures/es-modules/test-esm-ok.mjs'; + +// We just test that this module doesn't fail loading diff --git a/test/es-module/test-esm-loader-invalid-format.mjs b/test/es-module/test-esm-loader-invalid-format.mjs new file mode 100644 index 0000000000..c3f3a87407 --- /dev/null +++ b/test/es-module/test-esm-loader-invalid-format.mjs @@ -0,0 +1,12 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs +/* eslint-disable node-core/required-modules */ +import { expectsError, mustCall } from '../common/index.mjs'; +import assert from 'assert'; + +import('../fixtures/es-modules/test-esm-ok.mjs') +.then(assert.fail, expectsError({ + code: 'ERR_INVALID_RETURN_PROPERTY_VALUE', + message: 'Expected string to be returned for the "format" from the ' + + '"loader resolve" function but got type undefined.' +})) +.then(mustCall()); diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs new file mode 100644 index 0000000000..9cf17b2478 --- /dev/null +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -0,0 +1,14 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs +/* eslint-disable node-core/required-modules */ + +import { expectsError, mustCall } from '../common/index.mjs'; +import assert from 'assert'; + +import('../fixtures/es-modules/test-esm-ok.mjs') +.then(assert.fail, expectsError({ + code: 'ERR_INVALID_RETURN_PROPERTY', + message: 'Expected a valid url to be returned for the "url" from the ' + + '"loader resolve" function but got ' + + '../fixtures/es-modules/test-esm-ok.mjs.' +})) +.then(mustCall()); diff --git a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs new file mode 100644 index 0000000000..ab2da7adce --- /dev/null +++ b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs @@ -0,0 +1,10 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs +/* eslint-disable node-core/required-modules */ + +import { expectsError } from '../common/index.mjs'; + +import('test').catch(expectsError({ + code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', + message: 'The ES Module loader may not return a format of \'dynamic\' ' + + 'when no dynamicInstantiate function was provided' +})); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs new file mode 100644 index 0000000000..e235f598cb --- /dev/null +++ b/test/es-module/test-esm-named-exports.mjs @@ -0,0 +1,9 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import { readFile } from 'fs'; +import assert from 'assert'; +import ok from '../fixtures/es-modules/test-esm-ok.mjs'; + +assert(ok); +assert(readFile); diff --git a/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs b/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs new file mode 100644 index 0000000000..2ca0f56581 --- /dev/null +++ b/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs @@ -0,0 +1,3 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs +/* eslint-disable node-core/required-modules */ +import './not-found.js'; diff --git a/test/es-module/test-esm-preserve-symlinks-not-found.mjs b/test/es-module/test-esm-preserve-symlinks-not-found.mjs new file mode 100644 index 0000000000..b5be2d7e63 --- /dev/null +++ b/test/es-module/test-esm-preserve-symlinks-not-found.mjs @@ -0,0 +1,3 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs +/* eslint-disable node-core/required-modules */ +import './not-found.mjs'; diff --git a/test/es-module/test-esm-resolve-hook.mjs b/test/es-module/test-esm-resolve-hook.mjs new file mode 100644 index 0000000000..e326d20b6d --- /dev/null +++ b/test/es-module/test-esm-resolve-hook.mjs @@ -0,0 +1,8 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/js-loader.mjs +/* eslint-disable node-core/required-modules */ +import { namedExport } from '../fixtures/es-module-loaders/js-as-esm.js'; +import assert from 'assert'; +import ok from '../fixtures/es-modules/test-esm-ok.mjs'; + +assert(ok); +assert(namedExport); diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs new file mode 100644 index 0000000000..b8953ab1ec --- /dev/null +++ b/test/es-module/test-esm-shared-loader-dep.mjs @@ -0,0 +1,11 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs +/* eslint-disable node-core/required-modules */ +import { createRequire } from '../common/index.mjs'; + +import assert from 'assert'; +import '../fixtures/es-modules/test-esm-ok.mjs'; + +const require = createRequire(import.meta.url); +const dep = require('../fixtures/es-module-loaders/loader-dep.js'); + +assert.strictEqual(dep.format, 'module'); diff --git a/test/es-module/test-esm-throw-undefined.mjs b/test/es-module/test-esm-throw-undefined.mjs new file mode 100644 index 0000000000..97e917da5e --- /dev/null +++ b/test/es-module/test-esm-throw-undefined.mjs @@ -0,0 +1,16 @@ +// Flags: --experimental-modules +/* eslint-disable node-core/required-modules */ + +import '../common/index.mjs'; +import assert from 'assert'; + +async function doTest() { + await assert.rejects( + async () => { + await import('../fixtures/es-module-loaders/throw-undefined.mjs'); + }, + (e) => e === undefined + ); +} + +doTest(); diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs new file mode 100644 index 0000000000..28ccd6ecf2 --- /dev/null +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -0,0 +1,29 @@ +import module from 'module'; + +const builtins = new Set( + Object.keys(process.binding('natives')).filter(str => + /^(?!(?:internal|node|v8)\/)/.test(str)) +); + +export function dynamicInstantiate(url) { + const builtinInstance = module._load(url.substr(5)); + const builtinExports = ['default', ...Object.keys(builtinInstance)]; + return { + exports: builtinExports, + execute: exports => { + for (let name of builtinExports) + exports[name].set(builtinInstance[name]); + exports.default.set(builtinInstance); + } + }; +} + +export function resolve(specifier, base, defaultResolver) { + if (builtins.has(specifier)) { + return { + url: `node:${specifier}`, + format: 'dynamic' + }; + } + return defaultResolver(specifier, base); +} diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs new file mode 100644 index 0000000000..d0a560633a --- /dev/null +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -0,0 +1,37 @@ +import url from 'url'; +import path from 'path'; +import process from 'process'; + +const builtins = new Set( + Object.keys(process.binding('natives')).filter((str) => + /^(?!(?:internal|node|v8)\/)/.test(str)) +); +const JS_EXTENSIONS = new Set(['.js', '.mjs']); + +const baseURL = new url.URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve(specifier, parentModuleURL = baseURL /*, defaultResolve */) { + if (builtins.has(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { + // For node_modules support: + // return defaultResolve(specifier, parentModuleURL); + throw new Error( + `imports must begin with '/', './', or '../'; '${specifier}' does not`); + } + const resolved = new url.URL(specifier, parentModuleURL); + const ext = path.extname(resolved.pathname); + if (!JS_EXTENSIONS.has(ext)) { + throw new Error( + `Cannot load file with non-JavaScript file extension ${ext}.`); + } + return { + url: resolved.href, + format: 'module' + }; +} diff --git a/test/fixtures/es-module-loaders/js-as-esm.js b/test/fixtures/es-module-loaders/js-as-esm.js new file mode 100644 index 0000000000..b4d2741b2f --- /dev/null +++ b/test/fixtures/es-module-loaders/js-as-esm.js @@ -0,0 +1 @@ +export const namedExport = 'named-export'; diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs new file mode 100644 index 0000000000..5b9bf4ce4e --- /dev/null +++ b/test/fixtures/es-module-loaders/js-loader.mjs @@ -0,0 +1,24 @@ +import { URL } from 'url'; + +const builtins = new Set( + Object.keys(process.binding('natives')).filter(str => + /^(?!(?:internal|node|v8)\/)/.test(str)) +) + +const baseURL = new URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve (specifier, base = baseURL) { + if (builtins.has(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + // load all dependencies as esm, regardless of file extension + const url = new URL(specifier, base).href; + return { + url, + format: 'module' + }; +} diff --git a/test/fixtures/es-module-loaders/loader-dep.js b/test/fixtures/es-module-loaders/loader-dep.js new file mode 100644 index 0000000000..c8154ac5db --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-dep.js @@ -0,0 +1 @@ +exports.format = 'module'; diff --git a/test/fixtures/es-module-loaders/loader-invalid-format.mjs b/test/fixtures/es-module-loaders/loader-invalid-format.mjs new file mode 100644 index 0000000000..17a0dcd04d --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-invalid-format.mjs @@ -0,0 +1,8 @@ +export async function resolve(specifier, parentModuleURL, defaultResolve) { + if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { + return { + url: 'file:///asdf' + }; + } + return defaultResolve(specifier, parentModuleURL); +} diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs new file mode 100644 index 0000000000..f653155899 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-invalid-url.mjs @@ -0,0 +1,10 @@ +/* eslint-disable node-core/required-modules */ +export async function resolve(specifier, parentModuleURL, defaultResolve) { + if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { + return { + url: specifier, + format: 'esm' + }; + } + return defaultResolve(specifier, parentModuleURL); +} diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs new file mode 100644 index 0000000000..3acafcce1e --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-shared-dep.mjs @@ -0,0 +1,11 @@ +import assert from 'assert'; + +import {createRequire} from '../../common/index.mjs'; + +const require = createRequire(import.meta.url); +const dep = require('./loader-dep.js'); + +export function resolve(specifier, base, defaultResolve) { + assert.strictEqual(dep.format, 'module'); + return defaultResolve(specifier, base); +} diff --git a/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs b/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs new file mode 100644 index 0000000000..e7c6c8ff34 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs @@ -0,0 +1,6 @@ +export async function resolve(specifier, parent, defaultResolve) { + if (specifier === 'unknown-builtin-module') { + return { url: 'unknown-builtin-module', format: 'builtin' }; + } + return defaultResolve(specifier, parent); +} \ No newline at end of file diff --git a/test/fixtures/es-module-loaders/loader-with-dep.mjs b/test/fixtures/es-module-loaders/loader-with-dep.mjs new file mode 100644 index 0000000000..5afd3b2e21 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-with-dep.mjs @@ -0,0 +1,11 @@ +import {createRequire} from '../../common/index.mjs'; + +const require = createRequire(import.meta.url); +const dep = require('./loader-dep.js'); + +export function resolve (specifier, base, defaultResolve) { + return { + url: defaultResolve(specifier, base).url, + format: dep.format + }; +} diff --git a/test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs b/test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs new file mode 100644 index 0000000000..6993747fcc --- /dev/null +++ b/test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs @@ -0,0 +1,6 @@ +export function resolve(specifier, parentModule, defaultResolver) { + if (specifier !== 'test') { + return defaultResolver(specifier, parentModule); + } + return { url: 'file://', format: 'dynamic' }; +} diff --git a/test/fixtures/es-module-loaders/module-named-exports.mjs b/test/fixtures/es-module-loaders/module-named-exports.mjs new file mode 100644 index 0000000000..04f7f43ebd --- /dev/null +++ b/test/fixtures/es-module-loaders/module-named-exports.mjs @@ -0,0 +1,2 @@ +export const foo = 'foo'; +export const bar = 'bar'; diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs new file mode 100644 index 0000000000..d3eebcd47e --- /dev/null +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -0,0 +1,22 @@ +import assert from 'assert'; + +// a loader that asserts that the defaultResolve will throw "not found" +// (skipping the top-level main of course) +let mainLoad = true; +export async function resolve (specifier, base, defaultResolve) { + if (mainLoad) { + mainLoad = false; + return defaultResolve(specifier, base); + } + try { + await defaultResolve(specifier, base); + } + catch (e) { + assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND'); + return { + format: 'builtin', + url: 'fs' + }; + } + assert.fail(`Module resolution for ${specifier} should be throw ERR_MODULE_NOT_FOUND`); +} diff --git a/test/fixtures/es-module-loaders/syntax-error-import.mjs b/test/fixtures/es-module-loaders/syntax-error-import.mjs new file mode 100644 index 0000000000..3a6bc5effc --- /dev/null +++ b/test/fixtures/es-module-loaders/syntax-error-import.mjs @@ -0,0 +1 @@ +import { foo, notfound } from './module-named-exports.mjs'; diff --git a/test/fixtures/es-module-loaders/syntax-error.mjs b/test/fixtures/es-module-loaders/syntax-error.mjs new file mode 100644 index 0000000000..bda4a7e6eb --- /dev/null +++ b/test/fixtures/es-module-loaders/syntax-error.mjs @@ -0,0 +1,2 @@ +'use strict'; +await async () => 0; diff --git a/test/fixtures/es-module-loaders/throw-undefined.mjs b/test/fixtures/es-module-loaders/throw-undefined.mjs new file mode 100644 index 0000000000..0349ae112d --- /dev/null +++ b/test/fixtures/es-module-loaders/throw-undefined.mjs @@ -0,0 +1,4 @@ +'use strict'; +/* eslint-disable node-core/required-modules */ + +throw undefined; diff --git a/test/message/esm_display_syntax_error_import.mjs b/test/message/esm_display_syntax_error_import.mjs new file mode 100644 index 0000000000..12d10270e9 --- /dev/null +++ b/test/message/esm_display_syntax_error_import.mjs @@ -0,0 +1,7 @@ +// Flags: --experimental-modules +/* eslint-disable no-unused-vars, node-core/required-modules */ +import '../common/index.mjs'; +import { + foo, + notfound +} from '../fixtures/es-module-loaders/module-named-exports.mjs'; diff --git a/test/message/esm_display_syntax_error_import.out b/test/message/esm_display_syntax_error_import.out new file mode 100644 index 0000000000..48f2e2fb74 --- /dev/null +++ b/test/message/esm_display_syntax_error_import.out @@ -0,0 +1,6 @@ +(node:*) ExperimentalWarning: The ESM module loader is experimental. +file:///*/test/message/esm_display_syntax_error_import.mjs:6 + notfound + ^^^^^^^^ +SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports.mjs' does not provide an export named 'notfound' + at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*) diff --git a/test/message/esm_display_syntax_error_import_module.mjs b/test/message/esm_display_syntax_error_import_module.mjs new file mode 100644 index 0000000000..a53bbbcd19 --- /dev/null +++ b/test/message/esm_display_syntax_error_import_module.mjs @@ -0,0 +1,4 @@ +// Flags: --experimental-modules +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import '../fixtures/es-module-loaders/syntax-error-import.mjs'; diff --git a/test/message/esm_display_syntax_error_import_module.out b/test/message/esm_display_syntax_error_import_module.out new file mode 100644 index 0000000000..3e1024db8a --- /dev/null +++ b/test/message/esm_display_syntax_error_import_module.out @@ -0,0 +1,6 @@ +(node:*) ExperimentalWarning: The ESM module loader is experimental. +file:///*/test/fixtures/es-module-loaders/syntax-error-import.mjs:1 +import { foo, notfound } from './module-named-exports.mjs'; + ^^^^^^^^ +SyntaxError: The requested module './module-named-exports.mjs' does not provide an export named 'notfound' + at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*) diff --git a/test/message/esm_display_syntax_error_module.mjs b/test/message/esm_display_syntax_error_module.mjs new file mode 100644 index 0000000000..5905d2a954 --- /dev/null +++ b/test/message/esm_display_syntax_error_module.mjs @@ -0,0 +1,4 @@ +// Flags: --experimental-modules +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import '../fixtures/es-module-loaders/syntax-error.mjs'; diff --git a/test/message/esm_display_syntax_error_module.out b/test/message/esm_display_syntax_error_module.out new file mode 100644 index 0000000000..e636abad9e --- /dev/null +++ b/test/message/esm_display_syntax_error_module.out @@ -0,0 +1,6 @@ +(node:*) ExperimentalWarning: The ESM module loader is experimental. +file:///*/test/fixtures/es-module-loaders/syntax-error.mjs:2 +await async () => 0; +^^^^^ +SyntaxError: Unexpected reserved word + at translators.set (internal/modules/esm/translators.js:*:*) diff --git a/test/parallel/test-loaders-unknown-builtin-module.mjs b/test/parallel/test-loaders-unknown-builtin-module.mjs new file mode 100644 index 0000000000..db3cfa3582 --- /dev/null +++ b/test/parallel/test-loaders-unknown-builtin-module.mjs @@ -0,0 +1,12 @@ +// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs +import { expectsError, mustCall } from '../common'; +import assert from 'assert'; + +const unknownBuiltinModule = 'unknown-builtin-module'; + +import(unknownBuiltinModule) +.then(assert.fail, expectsError({ + code: 'ERR_UNKNOWN_BUILTIN_MODULE', + message: `No such built-in module: ${unknownBuiltinModule}` +})) +.then(mustCall()); From 4406f9990632321e35550e3574c0d804df944a29 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 5 Mar 2019 12:19:24 +0200 Subject: [PATCH 27/37] esm: add experimental warning for --loader --- lib/internal/process/esm_loader.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index ed387a1554..803c854d9a 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -7,6 +7,7 @@ const { ERR_INVALID_TYPE_FLAG, ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, } = require('internal/errors').codes; +const { emitExperimentalWarning } = require('internal/util'); const type = require('internal/options').getOptionValue('--type'); if (type && type !== 'commonjs' && type !== 'module') @@ -48,6 +49,7 @@ exports.initializeLoader = function(cwd, userLoader) { let ESMLoader = new Loader(); const loaderPromise = (async () => { if (userLoader) { + emitExperimentalWarning('--loader'); const hooks = await ESMLoader.import( userLoader, pathToFileURL(`${cwd}/`).href); ESMLoader = new Loader(); From 0237465ad428dc5b2ede143bd515d5ebf86f77c5 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 5 Mar 2019 22:17:11 -0500 Subject: [PATCH 28/37] fixup: fix broken tests --- doc/api/cli.md | 2 +- test/message/esm_display_syntax_error.out | 1 + test/message/esm_display_syntax_error_module.out | 3 ++- test/parallel/test-loaders-unknown-builtin-module.mjs | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index b88d35bf32..62e13496c0 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -911,7 +911,7 @@ greater than `4` (its current default value). For more information, see the [debugger]: debugger.html [debugging security implications]: https://nodejs.org/en/docs/guides/debugging-getting-started/#security-implications [emit_warning]: process.html#process_process_emitwarning_warning_type_code_ctor -[experimental ECMAScript Module]: esm.html#esm_loader_hooks +[experimental ECMAScript Module]: esm.html#esm_experimental_loader_hooks [libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html [remote code execution]: https://www.owasp.org/index.php/Code_Injection [secureProtocol]: tls.html#tls_tls_createsecurecontext_options diff --git a/test/message/esm_display_syntax_error.out b/test/message/esm_display_syntax_error.out index 2700fd894c..8f17d5cd7a 100644 --- a/test/message/esm_display_syntax_error.out +++ b/test/message/esm_display_syntax_error.out @@ -2,5 +2,6 @@ file:///*/test/message/esm_display_syntax_error.mjs:3 await async () => 0; ^^^^^ + SyntaxError: Unexpected reserved word at Loader. (internal/modules/esm/translators.js:*:*) diff --git a/test/message/esm_display_syntax_error_module.out b/test/message/esm_display_syntax_error_module.out index e636abad9e..7dbbd32f1f 100644 --- a/test/message/esm_display_syntax_error_module.out +++ b/test/message/esm_display_syntax_error_module.out @@ -2,5 +2,6 @@ file:///*/test/fixtures/es-module-loaders/syntax-error.mjs:2 await async () => 0; ^^^^^ + SyntaxError: Unexpected reserved word - at translators.set (internal/modules/esm/translators.js:*:*) + at Loader. (internal/modules/esm/translators.js:*:*) diff --git a/test/parallel/test-loaders-unknown-builtin-module.mjs b/test/parallel/test-loaders-unknown-builtin-module.mjs index db3cfa3582..5f47f191f5 100644 --- a/test/parallel/test-loaders-unknown-builtin-module.mjs +++ b/test/parallel/test-loaders-unknown-builtin-module.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs -import { expectsError, mustCall } from '../common'; +/* eslint-disable node-core/required-modules */ +import { expectsError, mustCall } from '../common/index.mjs'; import assert from 'assert'; const unknownBuiltinModule = 'unknown-builtin-module'; From 47bdf9dcfbdb53ece3c533007d8f8efa7dcdbbeb Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Mon, 4 Mar 2019 21:02:08 -0800 Subject: [PATCH 29/37] doc: esm flags, package scope and file extensions rules (wip) --- doc/api/esm.md | 120 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 106 insertions(+), 14 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 25c2e82673..9e5aea2b2f 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -8,26 +8,118 @@ Node.js contains support for ES Modules based upon the -[Node.js EP for ES Modules][] and the [ESM Minimal Kernel][]. +[Node.js EP for ES Modules][] and the [ecmascript-modules implementation][]. -The minimal feature set is designed to be compatible with all potential -future implementations. Expect major changes in the implementation including -interoperability support, specifier resolution, and default behavior. +Expect major changes in the implementation including interoperability support, +specifier resolution, and default behavior. ## Enabling The `--experimental-modules` flag can be used to enable features for loading -ESM modules. +ECMAScript modules. -Once this has been set, files ending with `.mjs` will be able to be loaded -as ES Modules. +Once this has been set, there are a few different ways to run a file as an ES module: + +### .mjs extension + +Files ending with `.mjs` will be loaded as ES modules. ```sh node --experimental-modules my-app.mjs ``` +### --type=module / -m flag + +Files ending with `.js` or `.mjs`, or lacking any extension, +will be loaded as ES modules when the `--type=module` flag is set. +This flag also has a shorthand alias `-m`. + +```sh +node --experimental-modules --type=module my-app.js +# or +node --experimental-modules -m my-app.js +``` + +For completeness there is also `--type=commonjs`, for explicitly running a +`.js` file as CommonJS. +This is the default behavior if `--type` or `-m` is unspecified. + +The `--type=module` or `-m` flags can also be used to tell Node.js to treat as an ES module input sent in via `--eval` or `--print` (or `-e` or `-p`) or piped to Node.js via `STDIN`. + +```sh +node --experimental-modules --type=module --eval \ + "import { sep } from 'path'; console.log(sep);" + +coffee --print file-containing-import-statements.coffee | \ + node --experimental-modules --type=module +``` + +### package.json "type" field + +Files ending with `.js` or `.mjs`, or lacking any extension, +will be loaded as ES modules when the nearest parent `package.json` file +contains a top-level field `"type"` with a value of `"module"`. + +The nearest parent `package.json` is defined as the first `package.json` found +when searching in the current folder, that folder’s parent, and so on up +until the root of the volume is reached. + +```js +// package.json +{ + "type": "module" +} +``` + +```sh +# In same folder as above package.json +node --experimental-modules my-app.js # Runs as ES module +``` + +If the nearest parent `package.json` lacks a `"type"` field, or contains +`"type": "commonjs"`, extensionless and `.js` files are treated as CommonJS. +If the volume root is reached and no `package.json` is found, +Node.js behaves the same as if it had found a `package.json` with no `"type"` +field. + +## Package Scope and File Extensions + +The folder containing a `package.json` file, and all subfolders below that +folder down until the next folder containing another `package.json`, is +considered a _package scope_. The `"type"` field defines how `.js` and +extensionless files should be treated within a particular `package.json` file’s +package scope. Every package in a project’s `node_modules` folder contains its +own `package.json` file, so each project’s dependencies have their own package +scopes. A `package.json` lacking a `"type"` field is treated as if it contained +`"type": "commonjs"`. + +The package scope applies not only to initial entry points (`node --experimental-modules my-app.js`) but also to files referenced by `import` statements and `import()` expressions. + +```js +// my-app.js, in an ES module package scope because there is a package.json +// file in the same folder with "type": "module" + +import './startup/init.js'; +// Loaded as ES module since ./startup contains no package.json file, +// and therefore inherits the ES module package scope from one level up + +import './node_modules/commonjs-package/index.js'; +// Loaded as CommonJS since ./node_modules/commonjs-package contains a +// package.json file with no "type" field, or "type": "commonjs" +``` + +Files ending with `.mjs` are always loaded as ES modules regardless of package scope. + +Files ending with `.cjs` are always loaded as CommonJS regardless of package scope. + +You can use the `.mjs` and `.cjs` extensions to mix types within the same package scope: + +- Within a `"type": "module"` package scope, Node.js can be instructed to interpret a particular file as CommonJS by naming it with a `.cjs` extension (since both `.js` and `.mjs` files are treated as ES modules within a `"module"` package scope). + +- Within a `"type": "commonjs"` package scope, Node.js can be instructed to interpret a particular file as an ES module by naming it with an `.mjs` extension (since both `.js` and `.cjs` files are treated as CommonJS within a `"commonjs"` package scope). + ## Features @@ -256,7 +348,7 @@ PACKAGE_MAIN_RESOLVE(_packageURL_, _pjson_) > 1. If _url_ ends with _".cjs"_, then > 1. Throw a _Type Mismatch_ error. > 1. Return _"module"_. -> 1. Let _pjson_ be the result of **READ_PACKAGE_BOUNDARY**(_url_). +> 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_). > 1. If _pjson_ is **null** and _isMain_ is **true**, then > 1. If _url_ ends in _".mjs"_, then > 1. Return _"module"_. @@ -272,13 +364,13 @@ PACKAGE_MAIN_RESOLVE(_packageURL_, _pjson_) > 1. Throw an _Unsupported File Extension_ error. > 1. Return _"commonjs"_. -READ_PACKAGE_BOUNDARY(_url_) -> 1. Let _boundaryURL_ be _url_. -> 1. While _boundaryURL_ is not the file system root, -> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_boundaryURL_). +READ_PACKAGE_SCOPE(_url_) +> 1. Let _scopeURL_ be _url_. +> 1. While _scopeURL_ is not the file system root, +> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_scopeURL_). > 1. If _pjson_ is not **null**, then > 1. Return _pjson_. -> 1. Set _boundaryURL_ to the parent URL of _boundaryURL_. +> 1. Set _scopeURL_ to the parent URL of _scopeURL_. > 1. Return **null**. READ_PACKAGE_JSON(_packageURL_) @@ -413,4 +505,4 @@ in the import tree. [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [dynamic instantiate hook]: #esm_dynamic_instantiate_hook [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename -[ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md +[ecmascript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md From 0f701a0ca3f16a1d947634ffbad76a79f92b03d4 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Tue, 5 Mar 2019 17:01:38 -0800 Subject: [PATCH 30/37] doc: esm features, differences/interoperability with commonjs --- doc/api/cli.md | 10 ++-- doc/api/esm.md | 155 +++++++++++++++++++++++++++++++++++++------------ 2 files changed, 122 insertions(+), 43 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 62e13496c0..0e4f9280a8 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -513,13 +513,13 @@ Track heap object allocations for heap snapshots. ### `-m`, `--type=type` -When using `--experimental-modules`, this informs the module resolution type -to interpret the top-level entry into Node.js. +With `--experimental-modules`, this tells Node.js whether to interpret the +initial entry point as CommonJS or as an ES module. -Works with stdin, `--eval`, `--print` as well as standard execution. +Valid values are `"commonjs"` and `"module"`, where the default is to infer from +the file extension and the `"type"` field in the nearest parent `package.json`. -Valid values are `"commonjs"` and `"module"`, where the default is to infer -from the file extension and package type boundary. +Works with standard file input as well as `--eval`, `--print`, `STDIN`. `-m` is an alias for `--type=module`. diff --git a/doc/api/esm.md b/doc/api/esm.md index 9e5aea2b2f..9d862cd9da 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -20,7 +20,8 @@ specifier resolution, and default behavior. The `--experimental-modules` flag can be used to enable features for loading ECMAScript modules. -Once this has been set, there are a few different ways to run a file as an ES module: +Once this has been set, there are a few different ways to run a file as an ES +module: ### .mjs extension @@ -42,11 +43,13 @@ node --experimental-modules --type=module my-app.js node --experimental-modules -m my-app.js ``` -For completeness there is also `--type=commonjs`, for explicitly running a -`.js` file as CommonJS. -This is the default behavior if `--type` or `-m` is unspecified. +For completeness there is also `--type=commonjs`, for explicitly running a `.js` +file as CommonJS. This is the default behavior if `--type` or `-m` is +unspecified. -The `--type=module` or `-m` flags can also be used to tell Node.js to treat as an ES module input sent in via `--eval` or `--print` (or `-e` or `-p`) or piped to Node.js via `STDIN`. +The `--type=module` or `-m` flags can also be used to tell Node.js to treat as +an ES module input sent in via `--eval` or `--print` (or `-e` or `-p`) or piped +to Node.js via `STDIN`. ```sh node --experimental-modules --type=module --eval \ @@ -66,6 +69,7 @@ The nearest parent `package.json` is defined as the first `package.json` found when searching in the current folder, that folder’s parent, and so on up until the root of the volume is reached. + ```js // package.json { @@ -86,7 +90,7 @@ field. ## Package Scope and File Extensions -The folder containing a `package.json` file, and all subfolders below that +A folder containing a `package.json` file, and all subfolders below that folder down until the next folder containing another `package.json`, is considered a _package scope_. The `"type"` field defines how `.js` and extensionless files should be treated within a particular `package.json` file’s @@ -95,7 +99,9 @@ own `package.json` file, so each project’s dependencies have their own package scopes. A `package.json` lacking a `"type"` field is treated as if it contained `"type": "commonjs"`. -The package scope applies not only to initial entry points (`node --experimental-modules my-app.js`) but also to files referenced by `import` statements and `import()` expressions. +The package scope applies not only to initial entry points (`node +--experimental-modules my-app.js`) but also to files referenced by `import` +statements and `import()` expressions. ```js // my-app.js, in an ES module package scope because there is a package.json @@ -106,31 +112,42 @@ import './startup/init.js'; // and therefore inherits the ES module package scope from one level up import './node_modules/commonjs-package/index.js'; -// Loaded as CommonJS since ./node_modules/commonjs-package contains a -// package.json file with no "type" field, or "type": "commonjs" -``` - -Files ending with `.mjs` are always loaded as ES modules regardless of package scope. +// Loaded as CommonJS since ./node_modules/commonjs-package/package.json +// lacks a "type" field or contains "type": "commonjs" -Files ending with `.cjs` are always loaded as CommonJS regardless of package scope. +import 'commonjs-package'; +// Loaded as CommonJS since ./node_modules/commonjs-package/package.json +// lacks a "type" field or contains "type": "commonjs" +``` -You can use the `.mjs` and `.cjs` extensions to mix types within the same package scope: +Files ending with `.mjs` are always loaded as ES modules regardless of package +scope. -- Within a `"type": "module"` package scope, Node.js can be instructed to interpret a particular file as CommonJS by naming it with a `.cjs` extension (since both `.js` and `.mjs` files are treated as ES modules within a `"module"` package scope). +Files ending with `.cjs` are always loaded as CommonJS regardless of package +scope. -- Within a `"type": "commonjs"` package scope, Node.js can be instructed to interpret a particular file as an ES module by naming it with an `.mjs` extension (since both `.js` and `.cjs` files are treated as CommonJS within a `"commonjs"` package scope). +```js +import './legacy-file.cjs'; +// Loaded as CommonJS since .cjs is always loaded as CommonJS -## Features +import 'commonjs-package/src/index.mjs'; +// Loaded as ES module since .mjs is always loaded as ES module +``` - +You can use the `.mjs` and `.cjs` extensions to mix types within the same +package scope: -### Supported +- Within a `"type": "module"` package scope, Node.js can be instructed to + interpret a particular file as CommonJS by naming it with a `.cjs` extension + (since both `.js` and `.mjs` files are treated as ES modules within a + `"module"` package scope). -Only the CLI argument for the main entry point to the program can be an entry -point into an ESM graph. Dynamic import can also be used to create entry points -into ESM graphs at runtime. +- Within a `"type": "commonjs"` package scope, Node.js can be instructed to + interpret a particular file as an ES module by naming it with an `.mjs` + extension (since both `.js` and `.cjs` files are treated as CommonJS within a + `"commonjs"` package scope). -#### import.meta +## import.meta * {Object} @@ -139,37 +156,44 @@ property: * `url` {string} The absolute `file:` URL of the module. -### Unsupported - -| Feature | Reason | -| --- | --- | -| `require('./foo.mjs')` | ES Modules have differing resolution and timing, use dynamic import | - -## Notable differences between `import` and `require` +## Differences Between ES Modules and CommonJS ### Mandatory file extensions -You must provide a file extension when using the `import` keyword. +You must provide a file extension when using the `import` keyword. Directory +indexes (e.g. `'./startup/index.js'`) must also be fully specified. + +This behavior matches how `import` behaves in browser environments, assuming a +typically configured server. -### No NODE_PATH +### No NODE_PATH `NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks if this behavior is desired. -### No `require.extensions` +### No require, exports, module.exports, \_\_filename, \_\_dirname + +These CommonJS variables are not available in ES modules. + +`require` can be imported into an ES module using +[`module.createRequireFromPath()`][]. + +An equivalent for `__filename` and `__dirname` is [`import.meta.url`][]. + +### 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` +### No require.cache `require.cache` is not used by `import`. It has a separate cache. -### URL based paths +### URL-based paths -ESM are resolved and cached based upon [URL](https://url.spec.whatwg.org/) -semantics. This means that files containing special characters such as `#` and -`?` need to be escaped. +ES modules are resolved and cached based upon +[URL](https://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. @@ -181,6 +205,59 @@ import './foo.mjs?query=2'; // loads ./foo.mjs with query of "?query=2" For now, only modules using the `file:` protocol can be loaded. +## Interoperability with CommonJS + +### require + +`require` always treats the files it references as CommonJS. This applies +whether `require` is used the traditional way within a CommonJS environment, or +in an ES module environment using [`module.createRequireFromPath()`][]. + +To include an ES module into CommonJS, use [`import()`][]. + +### import statements + +An `import` statement can reference either ES module or CommonJS JavaScript. +Other file types such as JSON and Native modules are not supported. For those, +use [`module.createRequireFromPath()`][]. + +`import` statements are permitted only in ES modules. For similar functionality +in CommonJS, see [`import()`][]. + +The _specifier_ of an `import` statement (the string after the `from` keyword) +can either be an URL-style relative path like `'./file.mjs'` or a package name +like `'fs'`. + +Like in CommonJS, files within packages can be accessed by appending a path to +the package name. + +```js +import { sin, cos } from 'geometry/trigonometry-functions.mjs'; +``` + +> Currently only the “default export” is supported for CommonJS files or +> packages: +> +> +> ```js +> import _ from 'underscore'; // Works +> +> import { shuffle } from 'underscore'; // Errors +> ``` +> +> There are ongoing efforts to make the latter code possible. + +### import() expressions + +Dynamic `import()` is supported in both CommonJS and ES modules. It can be used +to include ES module files from CommonJS code. + +```js +(async () => { + await import('./my-app.mjs'); +})(); +``` + ## CommonJS, JSON, and Native Modules CommonJS, JSON, and Native modules can be used with [`module.createRequireFromPath()`][]. @@ -505,4 +582,6 @@ in the import tree. [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [dynamic instantiate hook]: #esm_dynamic_instantiate_hook [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename +[`import.meta.url`]: esm.html#importmeta +[`import()`]: esm.html#import-expressions [ecmascript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md From 80d6ab18b02f6cd81d4623b9f31f4432ac1a6adf Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Tue, 5 Mar 2019 22:36:15 -0800 Subject: [PATCH 31/37] doc: package entry points in esm --- doc/api/esm.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/doc/api/esm.md b/doc/api/esm.md index 9d862cd9da..989631eb74 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -147,6 +147,48 @@ package scope: extension (since both `.js` and `.cjs` files are treated as CommonJS within a `"commonjs"` package scope). +## Package Entry Points + +When a `package.json` contains `"type": "module"`, its `"main"` field defines +the ES module entry point for the package. + + +```js +// ./node_modules/es-module-package/package.json +{ + "type": "module", + "main": "./src/index.js" +} +``` +```js +// ./my-app.mjs + +import { something } from 'es-module-package'; +// Loads from ./node_modules/es-module-package/src/index.js +``` + +An attempt to `require` the above `es-module-package` would attempt to load +`./node_modules/es-module-package/src/index.js` as CommonJS, which would throw +an error as Node.js would not be able to parse the `export` statement in +CommonJS. + +Even if the `package.json` `"main"` points to a file ending in `.mjs`, the +`"type": "module"` is required. + +As with `import` statements, for ES module usage the value of `"main"` must be +a full path including extension: `"./index.mjs"`, not `"./index"`. + +> Currently a package can define _either_ a CommonJS entry point or an ES module +> entry point; there is no way to specify separate entry points for CommonJS and +> ES module usage. This means that a package entry point can be included via +> `require` or via `import` but not both. +> +> Such a limitation makes it difficult for packages to support both new versions +> of Node.js that understand ES modules and older versions of Node.js that +> understand only CommonJS. There is work ongoing to remove this limitation, and +> it will very likely entail changes to the behavior of `"main"` as defined +> here. + ## import.meta * {Object} From ecc07e2a8a508828063161ea8e2621dd3c91a1f7 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 6 Mar 2019 17:23:56 -0800 Subject: [PATCH 32/37] doc: notes from call --- doc/api/esm.md | 312 +++++++++++++++++++++++++++---------------------- 1 file changed, 174 insertions(+), 138 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 989631eb74..8480676a6a 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -17,15 +17,17 @@ specifier resolution, and default behavior. -The `--experimental-modules` flag can be used to enable features for loading -ECMAScript modules. +The `--experimental-modules` flag can be used to enable support for +ECMAScript modules (ES modules). -Once this has been set, there are a few different ways to run a file as an ES -module: +## Running Node.js with an ECMAScript Module -### .mjs extension +There are a few ways to start Node.js with an ES module as its input. -Files ending with `.mjs` will be loaded as ES modules. +### Initial entry point with an .mjs extension + +A file ending with `.mjs` passed to Node.js as an initial entry point will be +loaded as an ES module. ```sh node --experimental-modules my-app.mjs @@ -55,7 +57,7 @@ to Node.js via `STDIN`. node --experimental-modules --type=module --eval \ "import { sep } from 'path'; console.log(sep);" -coffee --print file-containing-import-statements.coffee | \ +echo "import { sep } from 'path'; console.log(sep);" | \ node --experimental-modules --type=module ``` @@ -111,11 +113,11 @@ import './startup/init.js'; // Loaded as ES module since ./startup contains no package.json file, // and therefore inherits the ES module package scope from one level up -import './node_modules/commonjs-package/index.js'; +import 'commonjs-package'; // Loaded as CommonJS since ./node_modules/commonjs-package/package.json // lacks a "type" field or contains "type": "commonjs" -import 'commonjs-package'; +import './node_modules/commonjs-package/index.js'; // Loaded as CommonJS since ./node_modules/commonjs-package/package.json // lacks a "type" field or contains "type": "commonjs" ``` @@ -172,16 +174,13 @@ An attempt to `require` the above `es-module-package` would attempt to load an error as Node.js would not be able to parse the `export` statement in CommonJS. -Even if the `package.json` `"main"` points to a file ending in `.mjs`, the -`"type": "module"` is required. - As with `import` statements, for ES module usage the value of `"main"` must be a full path including extension: `"./index.mjs"`, not `"./index"`. -> Currently a package can define _either_ a CommonJS entry point or an ES module -> entry point; there is no way to specify separate entry points for CommonJS and -> ES module usage. This means that a package entry point can be included via -> `require` or via `import` but not both. +> Currently a package can define _either_ a CommonJS entry point **or** an ES +> module entry point; there is no way to specify separate entry points for +> CommonJS and ES module usage. This means that a package entry point can be +> included via `require` or via `import` but not both. > > Such a limitation makes it difficult for packages to support both new versions > of Node.js that understand ES modules and older versions of Node.js that @@ -189,6 +188,38 @@ a full path including extension: `"./index.mjs"`, not `"./index"`. > it will very likely entail changes to the behavior of `"main"` as defined > here. +## import Specifiers + +### Terminology + +The _specifier_ of an `import` statement is the string after the `from` keyword, +e.g. `'path'` in `import { sep } from 'path'`. Specifiers are also used in +`export from` statements, and as the argument to an `import()` expression. + +There are four types of specifiers: + +- _Bare specifiers_ like `'some-package'`. They refer to an entry point of a + package by the package name. + +- _Deep import specifiers_ like `'some-package/lib/shuffle.mjs'`. They refer to + a path within a package prefixed by the package name. + +- _Relative specifiers_ like `'./startup.js'` or `'../config.mjs'`. They refer + to a path relative to the location of the importing file. + +- _Absolute specifiers_ like `'file:///opt/nodejs/config.js'`. They refer + directly and explicitly to a full path. + +Bare specifiers, and the bare specifier portion of deep import specifiers, are +strings; but everything else in a specifier is a URL. + +Only `file://` URLs are supported. A specifier like +`'https://example.com/app.js'` may be supported by browsers but it is not +supported in Node.js. + +Specifiers may not begin with `/` or `//`. These are reserved for potential +future use. The root of the current volume may be referenced via `file:///`. + ## import.meta * {Object} @@ -282,9 +313,9 @@ import { sin, cos } from 'geometry/trigonometry-functions.mjs'; > > > ```js -> import _ from 'underscore'; // Works +> import packageMain from 'commonjs-package'; // Works > -> import { shuffle } from 'underscore'; // Errors +> import { method } from 'commonjs-package'; // Errors > ``` > > There are ongoing efforts to make the latter code possible. @@ -349,6 +380,127 @@ fs.readFileSync = () => Buffer.from('Hello, ESM'); fs.readFileSync === readFileSync; ``` +## Experimental Loader hooks + +**Note: This API is currently being redesigned and will still change.**. + + + +To customize the default module resolution, loader hooks can optionally be +provided via a `--loader ./loader-name.mjs` argument to Node.js. + +When hooks are used they only apply to ES module loading and not to any +CommonJS modules loaded. + +### Resolve hook + +The resolve hook returns the resolved file URL and module format for a +given module specifier and parent file URL: + +```js +const baseURL = new URL('file://'); +baseURL.pathname = `${process.cwd()}/`; + +export async function resolve(specifier, + parentModuleURL = baseURL, + defaultResolver) { + return { + url: new URL(specifier, parentModuleURL).href, + format: 'esm' + }; +} +``` + +The `parentModuleURL` is provided as `undefined` when performing main Node.js +load itself. + +The default Node.js ES module resolution function is provided as a third +argument to the resolver for easy compatibility workflows. + +In addition to returning the resolved file URL value, the resolve hook also +returns a `format` property specifying the module format of the resolved +module. This can be one of the following: + +| `format` | Description | +| --- | --- | +| `'module'` | Load a standard JavaScript module | +| `'commonjs'` | Load a Node.js CommonJS module | +| `'builtin'` | Load a Node.js builtin module | +| `'dynamic'` | Use a [dynamic instantiate hook][] | + +For example, a dummy loader to load JavaScript restricted to browser resolution +rules with only JS file extension and Node.js builtin modules support could +be written: + +```js +import path from 'path'; +import process from 'process'; +import Module from 'module'; + +const builtins = Module.builtinModules; +const JS_EXTENSIONS = new Set(['.js', '.mjs']); + +const baseURL = new URL('file://'); +baseURL.pathname = `${process.cwd()}/`; + +export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { + if (builtins.includes(specifier)) { + return { + url: specifier, + format: 'builtin' + }; + } + if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { + // For node_modules support: + // return defaultResolve(specifier, parentModuleURL); + throw new Error( + `imports must begin with '/', './', or '../'; '${specifier}' does not`); + } + const resolved = new URL(specifier, parentModuleURL); + const ext = path.extname(resolved.pathname); + if (!JS_EXTENSIONS.has(ext)) { + throw new Error( + `Cannot load file with non-JavaScript file extension ${ext}.`); + } + return { + url: resolved.href, + format: 'esm' + }; +} +``` + +With this loader, running: + +```console +NODE_OPTIONS='--experimental-modules --loader ./custom-loader.mjs' node x.js +``` + +would load the module `x.js` as an ES module with relative resolution support +(with `node_modules` loading skipped in this example). + +### Dynamic instantiate hook + +To create a custom dynamic module that doesn't correspond to one of the +existing `format` interpretations, the `dynamicInstantiate` hook can be used. +This hook is called only for modules that return `format: 'dynamic'` from +the `resolve` hook. + +```js +export async function dynamicInstantiate(url) { + return { + exports: ['customExportName'], + execute: (exports) => { + // Get and set functions provided for pre-allocated export names + exports.customExportName.set('value'); + } + }; +} +``` + +With the list of module exports provided upfront, the `execute` function will +then be called at the exact point of module evaluation order for that module +in the import tree. + ## Resolution Algorithm ### Features @@ -385,6 +537,9 @@ entirely for the CommonJS loader. If the top-level `--type` is _"module"_, then the ESM resolver is used as described here, with the conditional `--type` check in **ESM_FORMAT**. +
+Resolver algorithm psuedocode + **ESM_RESOLVE(_specifier_, _parentURL_, _isMain_)** > 1. Let _resolvedURL_ be **undefined**. > 1. If _specifier_ is a valid URL, then @@ -500,126 +655,7 @@ READ_PACKAGE_JSON(_packageURL_) > 1. Throw an _Invalid Package Configuration_ error. > 1. Return the parsed JSON source of the file at _pjsonURL_. -## Experimental Loader hooks - -**Note: This API is currently being redesigned and will still change.**. - - - -To customize the default module resolution, loader hooks can optionally be -provided via a `--loader ./loader-name.mjs` argument to Node.js. - -When hooks are used they only apply to ES module loading and not to any -CommonJS modules loaded. - -### Resolve hook - -The resolve hook returns the resolved file URL and module format for a -given module specifier and parent file URL: - -```js -const baseURL = new URL('file://'); -baseURL.pathname = `${process.cwd()}/`; - -export async function resolve(specifier, - parentModuleURL = baseURL, - defaultResolver) { - return { - url: new URL(specifier, parentModuleURL).href, - format: 'esm' - }; -} -``` - -The `parentModuleURL` is provided as `undefined` when performing main Node.js -load itself. - -The default Node.js ES module resolution function is provided as a third -argument to the resolver for easy compatibility workflows. - -In addition to returning the resolved file URL value, the resolve hook also -returns a `format` property specifying the module format of the resolved -module. This can be one of the following: - -| `format` | Description | -| --- | --- | -| `'module'` | Load a standard JavaScript module | -| `'commonjs'` | Load a Node.js CommonJS module | -| `'builtin'` | Load a Node.js builtin module | -| `'dynamic'` | Use a [dynamic instantiate hook][] | - -For example, a dummy loader to load JavaScript restricted to browser resolution -rules with only JS file extension and Node.js builtin modules support could -be written: - -```js -import path from 'path'; -import process from 'process'; -import Module from 'module'; - -const builtins = Module.builtinModules; -const JS_EXTENSIONS = new Set(['.js', '.mjs']); - -const baseURL = new URL('file://'); -baseURL.pathname = `${process.cwd()}/`; - -export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { - if (builtins.includes(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { - // For node_modules support: - // return defaultResolve(specifier, parentModuleURL); - throw new Error( - `imports must begin with '/', './', or '../'; '${specifier}' does not`); - } - const resolved = new URL(specifier, parentModuleURL); - const ext = path.extname(resolved.pathname); - if (!JS_EXTENSIONS.has(ext)) { - throw new Error( - `Cannot load file with non-JavaScript file extension ${ext}.`); - } - return { - url: resolved.href, - format: 'esm' - }; -} -``` - -With this loader, running: - -```console -NODE_OPTIONS='--experimental-modules --loader ./custom-loader.mjs' node x.js -``` - -would load the module `x.js` as an ES module with relative resolution support -(with `node_modules` loading skipped in this example). - -### Dynamic instantiate hook - -To create a custom dynamic module that doesn't correspond to one of the -existing `format` interpretations, the `dynamicInstantiate` hook can be used. -This hook is called only for modules that return `format: 'dynamic'` from -the `resolve` hook. - -```js -export async function dynamicInstantiate(url) { - return { - exports: ['customExportName'], - execute: (exports) => { - // Get and set functions provided for pre-allocated export names - exports.customExportName.set('value'); - } - }; -} -``` - -With the list of module exports provided upfront, the `execute` function will -then be called at the exact point of module evaluation order for that module -in the import tree. +
[Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [dynamic instantiate hook]: #esm_dynamic_instantiate_hook From fcc8c86a389f13d01f6fb85ffb577c23ddc3274a Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 7 Mar 2019 01:55:52 -0500 Subject: [PATCH 33/37] doc: improve createRequire example Original doesn't really make sense in retrospect --- doc/api/esm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 8480676a6a..81ccb43e4d 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -341,9 +341,9 @@ module.exports = 'cjs'; // esm.mjs import { createRequireFromPath as createRequire } from 'module'; -import { fileURLToPath as fromPath } from 'url'; +import { fileURLToPath as fromURL } from 'url'; -const require = createRequire(fromPath(import.meta.url)); +const require = createRequire(fromURL(import.meta.url)); const cjs = require('./cjs'); cjs === 'cjs'; // true From 8de9da05bcd1f6b91162cc00aa3a09020c883f76 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Fri, 8 Mar 2019 14:35:50 -0800 Subject: [PATCH 34/37] doc: updates from review Co-Authored-By: GeoffreyBooth --- doc/api/cli.md | 6 +++--- doc/api/esm.md | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 0e4f9280a8..4939574e88 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -513,13 +513,13 @@ Track heap object allocations for heap snapshots. ### `-m`, `--type=type` -With `--experimental-modules`, this tells Node.js whether to interpret the +Used with `--experimental-modules`, this configures Node.js to interpret the initial entry point as CommonJS or as an ES module. -Valid values are `"commonjs"` and `"module"`, where the default is to infer from +Valid values are `"commonjs"` and `"module"`. The default is to infer from the file extension and the `"type"` field in the nearest parent `package.json`. -Works with standard file input as well as `--eval`, `--print`, `STDIN`. +Works for executing a file as well as `--eval`, `--print`, `STDIN`. `-m` is an alias for `--type=module`. diff --git a/doc/api/esm.md b/doc/api/esm.md index 81ccb43e4d..03db0c441c 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -49,7 +49,7 @@ For completeness there is also `--type=commonjs`, for explicitly running a `.js` file as CommonJS. This is the default behavior if `--type` or `-m` is unspecified. -The `--type=module` or `-m` flags can also be used to tell Node.js to treat as +The `--type=module` or `-m` flags can also be used to configure Node.js to treat as an ES module input sent in via `--eval` or `--print` (or `-e` or `-p`) or piped to Node.js via `STDIN`. @@ -87,7 +87,7 @@ node --experimental-modules my-app.js # Runs as ES module If the nearest parent `package.json` lacks a `"type"` field, or contains `"type": "commonjs"`, extensionless and `.js` files are treated as CommonJS. If the volume root is reached and no `package.json` is found, -Node.js behaves the same as if it had found a `package.json` with no `"type"` +Node.js defers to the deafult, a `package.json` with no `"type"` field. ## Package Scope and File Extensions From f6e1a434b33cac99c18627137826f59e6b0afc0f Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 8 Mar 2019 15:09:21 -0800 Subject: [PATCH 35/37] doc: fixup describing "main" as package entry point --- doc/api/esm.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 03db0c441c..5a144b09a6 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -49,9 +49,9 @@ For completeness there is also `--type=commonjs`, for explicitly running a `.js` file as CommonJS. This is the default behavior if `--type` or `-m` is unspecified. -The `--type=module` or `-m` flags can also be used to configure Node.js to treat as -an ES module input sent in via `--eval` or `--print` (or `-e` or `-p`) or piped -to Node.js via `STDIN`. +The `--type=module` or `-m` flags can also be used to configure Node.js to treat +as an ES module input sent in via `--eval` or `--print` (or `-e` or `-p`) or +piped to Node.js via `STDIN`. ```sh node --experimental-modules --type=module --eval \ @@ -151,8 +151,9 @@ package scope: ## Package Entry Points -When a `package.json` contains `"type": "module"`, its `"main"` field defines -the ES module entry point for the package. +The `package.json` `"main"` field defines the entry point for a package, +whether the package is included into CommonJS via `require` or into an ES +module via `import`. ```js @@ -177,6 +178,9 @@ CommonJS. As with `import` statements, for ES module usage the value of `"main"` must be a full path including extension: `"./index.mjs"`, not `"./index"`. +If the `package.json` `"type"` field is omitted, a `.js` file in `"main"` will +be interpreted as CommonJS. + > Currently a package can define _either_ a CommonJS entry point **or** an ES > module entry point; there is no way to specify separate entry points for > CommonJS and ES module usage. This means that a package entry point can be From bec588fbd0e4f03a30870734da9af32bfd0184f1 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 5 Mar 2019 04:20:15 -0500 Subject: [PATCH 36/37] esm: add --es-module-specifier-resolution There are currently two supported values "none" and "node" --- doc/api/esm.md | 4 +- src/module_wrap.cc | 120 +++++++++++++----- src/node_options.cc | 5 + src/node_options.h | 1 + test/es-module/test-esm-package-scope.mjs | 12 -- test/es-module/test-esm-specifiers.mjs | 35 +++++ test/fixtures/es-module-specifiers/index.mjs | 10 ++ .../node_modules/explicit-main/entry.mjs | 1 + .../node_modules/explicit-main/package.json | 3 + .../implicit-main-type-commonjs/entry.mjs | 1 + .../implicit-main-type-commonjs/package.json | 4 + .../implicit-main-type-module/entry.js | 1 + .../implicit-main-type-module/entry.mjs | 1 + .../implicit-main-type-module/package.json | 4 + .../node_modules/implicit-main/entry.js | 1 + .../node_modules/implicit-main/entry.mjs | 1 + .../node_modules/implicit-main/package.json | 3 + .../package-type-commonjs}/a.js | 0 .../package-type-commonjs}/b.mjs | 0 .../package-type-commonjs}/c.cjs | 0 .../package-type-commonjs}/index.mjs | 4 +- .../package-type-commonjs/package.json | 3 + .../package-type-module}/a.js | 0 .../package-type-module}/b.mjs | 0 .../package-type-module}/c.cjs | 0 .../package-type-module}/index.js | 4 +- .../package-type-module/package.json | 3 + .../es-module-specifiers/package.json | 1 + .../legacy-loader/package.json | 13 -- .../esm-package-scope/new-loader/package.json | 13 -- 30 files changed, 172 insertions(+), 76 deletions(-) delete mode 100644 test/es-module/test-esm-package-scope.mjs create mode 100644 test/es-module/test-esm-specifiers.mjs create mode 100644 test/fixtures/es-module-specifiers/index.mjs create mode 100644 test/fixtures/es-module-specifiers/node_modules/explicit-main/entry.mjs create mode 100644 test/fixtures/es-module-specifiers/node_modules/explicit-main/package.json create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/entry.mjs create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/package.json create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.js create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.mjs create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/package.json create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.js create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.mjs create mode 100644 test/fixtures/es-module-specifiers/node_modules/implicit-main/package.json rename test/fixtures/{esm-package-scope/legacy-loader => es-module-specifiers/package-type-commonjs}/a.js (100%) rename test/fixtures/{esm-package-scope/legacy-loader => es-module-specifiers/package-type-commonjs}/b.mjs (100%) rename test/fixtures/{esm-package-scope/legacy-loader => es-module-specifiers/package-type-commonjs}/c.cjs (100%) rename test/fixtures/{esm-package-scope/legacy-loader => es-module-specifiers/package-type-commonjs}/index.mjs (82%) create mode 100644 test/fixtures/es-module-specifiers/package-type-commonjs/package.json rename test/fixtures/{esm-package-scope/new-loader => es-module-specifiers/package-type-module}/a.js (100%) rename test/fixtures/{esm-package-scope/new-loader => es-module-specifiers/package-type-module}/b.mjs (100%) rename test/fixtures/{esm-package-scope/new-loader => es-module-specifiers/package-type-module}/c.cjs (100%) rename test/fixtures/{esm-package-scope/new-loader => es-module-specifiers/package-type-module}/index.js (82%) create mode 100644 test/fixtures/es-module-specifiers/package-type-module/package.json create mode 100644 test/fixtures/es-module-specifiers/package.json delete mode 100644 test/fixtures/esm-package-scope/legacy-loader/package.json delete mode 100644 test/fixtures/esm-package-scope/new-loader/package.json diff --git a/doc/api/esm.md b/doc/api/esm.md index 5a144b09a6..5b40293830 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -664,6 +664,6 @@ READ_PACKAGE_JSON(_packageURL_) [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md [dynamic instantiate hook]: #esm_dynamic_instantiate_hook [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename -[`import.meta.url`]: esm.html#importmeta -[`import()`]: esm.html#import-expressions +[`import.meta.url`]: #esm_import_meta +[`import()`]: #esm_import-expressions [ecmascript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 507ae11a6b..f505c09cec 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -43,6 +43,14 @@ using v8::String; using v8::Undefined; using v8::Value; +static const char* const EXTENSIONS[] = { + ".mjs", + ".cjs", + ".js", + ".json", + ".node" +}; + ModuleWrap::ModuleWrap(Environment* env, Local object, Local module, @@ -667,13 +675,57 @@ Maybe LegacyMainResolve(const URL& pjson_url, return Nothing(); } +enum ResolveExtensionsOptions { + TRY_EXACT_NAME, + ONLY_VIA_EXTENSIONS +}; + +template +Maybe ResolveExtensions(const URL& search) { + if (options == TRY_EXACT_NAME) { + if (FileExists(search)) { + return Just(search); + } + } + + for (const char* extension : EXTENSIONS) { + URL guess(search.path() + extension, &search); + if (FileExists(guess)) { + return Just(guess); + } + } + + return Nothing(); +} + +inline Maybe ResolveIndex(const URL& search) { + return ResolveExtensions(URL("index", search)); +} + Maybe FinalizeResolution(Environment* env, - const URL& resolved, - const URL& base, - bool check_exists) { - const std::string& path = resolved.ToFilePath(); + const URL& resolved, + const URL& base) { + if (env->options()->es_module_specifier_resolution == "node") { + Maybe file = ResolveExtensions(resolved); + if (!file.IsNothing()) { + return file; + } + if (resolved.path().back() != '/') { + file = ResolveIndex(URL(resolved.path() + "/", &base)); + } else { + file = ResolveIndex(resolved); + } + if (!file.IsNothing()) { + return file; + } + std::string msg = "Cannot find module '" + resolved.path() + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + return Nothing(); + } - if (check_exists && CheckDescriptorAtPath(path) != FILE) { + const std::string& path = resolved.ToFilePath(); + if (CheckDescriptorAtPath(path) != FILE) { std::string msg = "Cannot find module '" + path + "' imported from " + base.ToFilePath(); node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); @@ -684,32 +736,36 @@ Maybe FinalizeResolution(Environment* env, } Maybe PackageMainResolve(Environment* env, - const URL& pjson_url, - const PackageConfig& pcfg, - const URL& base) { - if (pcfg.exists == Exists::No || ( - pcfg.esm == IsESM::Yes && pcfg.has_main == HasMain::No)) { - std::string msg = "Cannot find main entry point for '" + - URL(".", pjson_url).ToFilePath() + "' imported from " + - base.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); - return Nothing(); - } - if (pcfg.has_main == HasMain::Yes && - pcfg.main.substr(pcfg.main.length() - 4, 4) == ".mjs") { - return FinalizeResolution(env, URL(pcfg.main, pjson_url), base, true); - } - if (pcfg.esm == IsESM::Yes && - pcfg.main.substr(pcfg.main.length() - 3, 3) == ".js") { - return FinalizeResolution(env, URL(pcfg.main, pjson_url), base, true); - } - - Maybe resolved = LegacyMainResolve(pjson_url, pcfg); - // Legacy main resolution error - if (resolved.IsNothing()) { - return Nothing(); + const URL& pjson_url, + const PackageConfig& pcfg, + const URL& base) { + if (pcfg.exists == Exists::Yes) { + if (pcfg.has_main == HasMain::Yes) { + URL resolved(pcfg.main, pjson_url); + const std::string& path = resolved.ToFilePath(); + if (CheckDescriptorAtPath(path) == FILE) { + return Just(resolved); + } + } + if (env->options()->es_module_specifier_resolution == "node") { + if (pcfg.has_main == HasMain::Yes) { + return FinalizeResolution(env, URL(pcfg.main, pjson_url), base); + } else { + return FinalizeResolution(env, URL("index", pjson_url), base); + } + } + if (pcfg.esm == IsESM::No) { + Maybe resolved = LegacyMainResolve(pjson_url, pcfg); + if (!resolved.IsNothing()) { + return resolved; + } + } } - return resolved; + std::string msg = "Cannot find main entry point for '" + + URL(".", pjson_url).ToFilePath() + "' imported from " + + base.ToFilePath(); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + return Nothing(); } Maybe PackageResolve(Environment* env, @@ -759,7 +815,7 @@ Maybe PackageResolve(Environment* env, if (!pkg_subpath.length()) { return PackageMainResolve(env, pjson_url, *pcfg.FromJust(), base); } else { - return FinalizeResolution(env, URL(pkg_subpath, pjson_url), base, true); + return FinalizeResolution(env, URL(pkg_subpath, pjson_url), base); } CHECK(false); // Cross-platform root check. @@ -789,7 +845,7 @@ Maybe Resolve(Environment* env, return PackageResolve(env, specifier, base); } } - return FinalizeResolution(env, resolved, base, true); + return FinalizeResolution(env, resolved, base); } void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { diff --git a/src/node_options.cc b/src/node_options.cc index 413169fde4..dacba8210c 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -251,6 +251,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "custom loader", &EnvironmentOptions::userland_loader, kAllowedInEnvironment); + AddOption("--es-module-specifier-resolution", + "Select extension resolution algorithm for es modules; " + "either 'explicit' (default) or 'node'", + &EnvironmentOptions::es_module_specifier_resolution, + kAllowedInEnvironment); AddOption("--no-deprecation", "silence deprecation warnings", &EnvironmentOptions::no_deprecation, diff --git a/src/node_options.h b/src/node_options.h index 83c3be8674..862514f2a7 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -86,6 +86,7 @@ class EnvironmentOptions : public Options { public: bool abort_on_uncaught_exception = false; bool experimental_modules = false; + std::string es_module_specifier_resolution = "explicit"; std::string module_type; std::string experimental_policy; bool experimental_repl_await = false; diff --git a/test/es-module/test-esm-package-scope.mjs b/test/es-module/test-esm-package-scope.mjs deleted file mode 100644 index 6e07a307e0..0000000000 --- a/test/es-module/test-esm-package-scope.mjs +++ /dev/null @@ -1,12 +0,0 @@ -// Flags: --experimental-modules -/* eslint-disable node-core/required-modules */ - -import '../common/index.mjs'; -import assert from 'assert'; - -import legacyLoader from - '../fixtures/esm-package-scope/legacy-loader/index.mjs'; -import newLoader from '../fixtures/esm-package-scope/new-loader/index.js'; - -assert.strictEqual(legacyLoader, 'legacy-loader'); -assert.strictEqual(newLoader, 'new-loader'); diff --git a/test/es-module/test-esm-specifiers.mjs b/test/es-module/test-esm-specifiers.mjs new file mode 100644 index 0000000000..b386dcb8e9 --- /dev/null +++ b/test/es-module/test-esm-specifiers.mjs @@ -0,0 +1,35 @@ +// Flags: --experimental-modules --es-module-specifier-resolution=node +import { mustNotCall } from '../common'; +import assert from 'assert'; + +// commonJS index.js +import commonjs from '../fixtures/es-module-specifiers/package-type-commonjs'; +// esm index.js +import module from '../fixtures/es-module-specifiers/package-type-module'; +// notice the trailing slash +import success, { explicit, implicit, implicitModule, getImplicitCommonjs } + from '../fixtures/es-module-specifiers/'; + +assert.strictEqual(commonjs, 'commonjs'); +assert.strictEqual(module, 'module'); +assert.strictEqual(success, 'success'); +assert.strictEqual(explicit, 'esm'); +assert.strictEqual(implicit, 'esm'); +assert.strictEqual(implicitModule, 'esm'); + +async function main() { + try { + await import('../fixtures/es-module-specifiers/do-not-exist.js'); + } catch (e) { + // Files that do not exist should throw + assert.strictEqual(e.name, 'Error'); + } + try { + await getImplicitCommonjs(); + } catch (e) { + // Legacy loader cannot resolve .mjs automatically from main + assert.strictEqual(e.name, 'Error'); + } +} + +main().catch(mustNotCall); diff --git a/test/fixtures/es-module-specifiers/index.mjs b/test/fixtures/es-module-specifiers/index.mjs new file mode 100644 index 0000000000..2be7048513 --- /dev/null +++ b/test/fixtures/es-module-specifiers/index.mjs @@ -0,0 +1,10 @@ +import explicit from 'explicit-main'; +import implicit from 'implicit-main'; +import implicitModule from 'implicit-main-type-module'; + +function getImplicitCommonjs () { + return import('implicit-main-type-commonjs'); +} + +export {explicit, implicit, implicitModule, getImplicitCommonjs}; +export default 'success'; diff --git a/test/fixtures/es-module-specifiers/node_modules/explicit-main/entry.mjs b/test/fixtures/es-module-specifiers/node_modules/explicit-main/entry.mjs new file mode 100644 index 0000000000..914e3a97d5 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/explicit-main/entry.mjs @@ -0,0 +1 @@ +export default 'esm'; diff --git a/test/fixtures/es-module-specifiers/node_modules/explicit-main/package.json b/test/fixtures/es-module-specifiers/node_modules/explicit-main/package.json new file mode 100644 index 0000000000..e9457582ac --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/explicit-main/package.json @@ -0,0 +1,3 @@ +{ + "main": "entry.mjs" +} \ No newline at end of file diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/entry.mjs b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/entry.mjs new file mode 100644 index 0000000000..914e3a97d5 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/entry.mjs @@ -0,0 +1 @@ +export default 'esm'; diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/package.json b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/package.json new file mode 100644 index 0000000000..663dad4f46 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-commonjs/package.json @@ -0,0 +1,4 @@ +{ + "main": "entry", + "type": "commonjs" +} \ No newline at end of file diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.js b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.js new file mode 100644 index 0000000000..5d7af588fd --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.js @@ -0,0 +1 @@ +export default 'nope'; diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.mjs b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.mjs new file mode 100644 index 0000000000..914e3a97d5 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/entry.mjs @@ -0,0 +1 @@ +export default 'esm'; diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/package.json b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/package.json new file mode 100644 index 0000000000..c34ab42042 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main-type-module/package.json @@ -0,0 +1,4 @@ +{ + "main": "entry", + "type": "module" +} \ No newline at end of file diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.js b/test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.js new file mode 100644 index 0000000000..b2825bd3c9 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.js @@ -0,0 +1 @@ +module.exports = 'cjs'; diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.mjs b/test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.mjs new file mode 100644 index 0000000000..914e3a97d5 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main/entry.mjs @@ -0,0 +1 @@ +export default 'esm'; diff --git a/test/fixtures/es-module-specifiers/node_modules/implicit-main/package.json b/test/fixtures/es-module-specifiers/node_modules/implicit-main/package.json new file mode 100644 index 0000000000..bf2e35593b --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/implicit-main/package.json @@ -0,0 +1,3 @@ +{ + "main": "entry" +} \ No newline at end of file diff --git a/test/fixtures/esm-package-scope/legacy-loader/a.js b/test/fixtures/es-module-specifiers/package-type-commonjs/a.js similarity index 100% rename from test/fixtures/esm-package-scope/legacy-loader/a.js rename to test/fixtures/es-module-specifiers/package-type-commonjs/a.js diff --git a/test/fixtures/esm-package-scope/legacy-loader/b.mjs b/test/fixtures/es-module-specifiers/package-type-commonjs/b.mjs similarity index 100% rename from test/fixtures/esm-package-scope/legacy-loader/b.mjs rename to test/fixtures/es-module-specifiers/package-type-commonjs/b.mjs diff --git a/test/fixtures/esm-package-scope/legacy-loader/c.cjs b/test/fixtures/es-module-specifiers/package-type-commonjs/c.cjs similarity index 100% rename from test/fixtures/esm-package-scope/legacy-loader/c.cjs rename to test/fixtures/es-module-specifiers/package-type-commonjs/c.cjs diff --git a/test/fixtures/esm-package-scope/legacy-loader/index.mjs b/test/fixtures/es-module-specifiers/package-type-commonjs/index.mjs similarity index 82% rename from test/fixtures/esm-package-scope/legacy-loader/index.mjs rename to test/fixtures/es-module-specifiers/package-type-commonjs/index.mjs index 1c78c389a2..ef2b30b19b 100644 --- a/test/fixtures/esm-package-scope/legacy-loader/index.mjs +++ b/test/fixtures/es-module-specifiers/package-type-commonjs/index.mjs @@ -5,7 +5,7 @@ import {b} from './b.mjs'; // import 'c.cjs'; import cjs from './c.cjs'; // proves cross boundary fun bits -import jsAsEsm from '../new-loader/a.js'; +import jsAsEsm from '../package-type-module/a.js'; // named export from core import {strictEqual, deepStrictEqual} from 'assert'; @@ -18,4 +18,4 @@ deepStrictEqual(cjs, { three: 3 }); -export default 'legacy-loader'; +export default 'commonjs'; diff --git a/test/fixtures/es-module-specifiers/package-type-commonjs/package.json b/test/fixtures/es-module-specifiers/package-type-commonjs/package.json new file mode 100644 index 0000000000..5bbefffbab --- /dev/null +++ b/test/fixtures/es-module-specifiers/package-type-commonjs/package.json @@ -0,0 +1,3 @@ +{ + "type": "commonjs" +} diff --git a/test/fixtures/esm-package-scope/new-loader/a.js b/test/fixtures/es-module-specifiers/package-type-module/a.js similarity index 100% rename from test/fixtures/esm-package-scope/new-loader/a.js rename to test/fixtures/es-module-specifiers/package-type-module/a.js diff --git a/test/fixtures/esm-package-scope/new-loader/b.mjs b/test/fixtures/es-module-specifiers/package-type-module/b.mjs similarity index 100% rename from test/fixtures/esm-package-scope/new-loader/b.mjs rename to test/fixtures/es-module-specifiers/package-type-module/b.mjs diff --git a/test/fixtures/esm-package-scope/new-loader/c.cjs b/test/fixtures/es-module-specifiers/package-type-module/c.cjs similarity index 100% rename from test/fixtures/esm-package-scope/new-loader/c.cjs rename to test/fixtures/es-module-specifiers/package-type-module/c.cjs diff --git a/test/fixtures/esm-package-scope/new-loader/index.js b/test/fixtures/es-module-specifiers/package-type-module/index.js similarity index 82% rename from test/fixtures/esm-package-scope/new-loader/index.js rename to test/fixtures/es-module-specifiers/package-type-module/index.js index 98c536cc34..a8baacb7c9 100644 --- a/test/fixtures/esm-package-scope/new-loader/index.js +++ b/test/fixtures/es-module-specifiers/package-type-module/index.js @@ -5,7 +5,7 @@ import {b} from './b.mjs'; // import 'c.cjs'; import cjs from './c.cjs'; // import across boundaries -import jsAsCjs from '../legacy-loader/a.js' +import jsAsCjs from '../package-type-commonjs/a.js' // named export from core import {strictEqual, deepStrictEqual} from 'assert'; @@ -18,4 +18,4 @@ deepStrictEqual(cjs, { three: 3 }); -export default 'new-loader'; +export default 'module'; diff --git a/test/fixtures/es-module-specifiers/package-type-module/package.json b/test/fixtures/es-module-specifiers/package-type-module/package.json new file mode 100644 index 0000000000..3dbc1ca591 --- /dev/null +++ b/test/fixtures/es-module-specifiers/package-type-module/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/test/fixtures/es-module-specifiers/package.json b/test/fixtures/es-module-specifiers/package.json new file mode 100644 index 0000000000..9e26dfeeb6 --- /dev/null +++ b/test/fixtures/es-module-specifiers/package.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/test/fixtures/esm-package-scope/legacy-loader/package.json b/test/fixtures/esm-package-scope/legacy-loader/package.json deleted file mode 100644 index 215a962248..0000000000 --- a/test/fixtures/esm-package-scope/legacy-loader/package.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "name": "legacy-loader", - "version": "1.0.0", - "description": "", - "main": "index.js", - "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" - }, - "keywords": [], - "author": "Myles Borins ", - "license": "Apache-2.0", - "type": "commonjs" -} diff --git a/test/fixtures/esm-package-scope/new-loader/package.json b/test/fixtures/esm-package-scope/new-loader/package.json deleted file mode 100644 index 1f2c704322..0000000000 --- a/test/fixtures/esm-package-scope/new-loader/package.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "name": "new-loader", - "version": "1.0.0", - "description": "", - "main": "index.js", - "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" - }, - "keywords": [], - "author": "Myles Borins ", - "license": "Apache-2.0", - "type": "module" -} From 4a95e44baabb4dfefb1f557f72207dbb7270fa31 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 10 Mar 2019 18:00:26 +0200 Subject: [PATCH 37/37] esm: scoped --type, cpp refactoring --- doc/api/esm.md | 38 ++-- lib/internal/modules/cjs/loader.js | 20 +- lib/internal/modules/esm/default_resolve.js | 140 +++++-------- src/env.h | 12 +- src/module_wrap.cc | 197 ++++++++++++++++-- src/module_wrap.h | 1 + src/node_url.h | 8 + test/es-module/test-esm-type-flag-errors.js | 4 +- test/es-module/test-esm-type-scope.js | 11 + test/fixtures/es-modules/esm-dep.js | 1 + test/fixtures/es-modules/esm-main.js | 3 + .../es-modules/package-without-type/index.js | 3 +- 12 files changed, 289 insertions(+), 149 deletions(-) create mode 100644 test/es-module/test-esm-type-scope.js create mode 100644 test/fixtures/es-modules/esm-dep.js create mode 100644 test/fixtures/es-modules/esm-main.js diff --git a/doc/api/esm.md b/doc/api/esm.md index 5b40293830..4c494372c6 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -535,14 +535,13 @@ of these top-level routines. _isMain_ is **true** when resolving the Node.js application entry point. -If the top-level `--type` is _"commonjs"_, then the ESM resolver is skipped -entirely for the CommonJS loader. - -If the top-level `--type` is _"module"_, then the ESM resolver is used -as described here, with the conditional `--type` check in **ESM_FORMAT**. +When using the `--type` flag with an entry point, the algorithm below behaves +as if there is a package.json file in the folder of the entry point containing +a `type` property set to the flag value (provided there isn't already a +package.json in the entry folder with a conflicting type value).
-Resolver algorithm psuedocode +Resolver algorithm specification **ESM_RESOLVE(_specifier_, _parentURL_, _isMain_)** > 1. Let _resolvedURL_ be **undefined**. @@ -622,25 +621,20 @@ PACKAGE_MAIN_RESOLVE(_packageURL_, _pjson_) **ESM_FORMAT(_url_, _isMain_)** > 1. Assert: _url_ corresponds to an existing file. -> 1. If _isMain_ is **true** and the `--type` flag is _"module"_, then -> 1. If _url_ ends with _".cjs"_, then -> 1. Throw a _Type Mismatch_ error. -> 1. Return _"module"_. > 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_). -> 1. If _pjson_ is **null** and _isMain_ is **true**, then -> 1. If _url_ ends in _".mjs"_, then -> 1. Return _"module"_. -> 1. Return _"commonjs"_. -> 1. If _pjson.type_ exists and is _"module"_, then -> 1. If _url_ ends in _".cjs"_, then -> 1. Return _"commonjs"_. +> 1. If _url_ ends in _".mjs"_, then > 1. Return _"module"_. -> 1. Otherwise, -> 1. If _url_ ends in _".mjs"_, then -> 1. Return _"module"_. -> 1. If _url_ does not end in _".js"_, then -> 1. Throw an _Unsupported File Extension_ error. +> 1. If _url_ ends in _".cjs"_, then > 1. Return _"commonjs"_. +> 1. If _pjson?.type_ exists and is _"module"_, then +> 1. If _isMain_ is **true** or _url_ ends in _".js"_, then +> 1. Return _"module"_. +> 1. Throw an _Unsupported File Extension_ error. +> 1. Otherwise, +> 1. If _isMain_ is **true** or _url_ ends in _".js"_, _".json"_ or +> _".node"_, then +> 1. Return _"commonjs"_. +> 1. Throw an _Unsupported File Extension_ error. READ_PACKAGE_SCOPE(_url_) > 1. Let _scopeURL_ be _url_. diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 2f5e5b15e5..058bf7aa67 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -860,17 +860,15 @@ Module.runMain = function() { // Load the main module--the command line argument. if (experimentalModules) { if (asyncESM === undefined) lazyLoadESM(); - if (asyncESM.typeFlag !== 'commonjs') { - asyncESM.loaderPromise.then((loader) => { - return loader.import(pathToFileURL(process.argv[1]).pathname); - }) - .catch((e) => { - internalBinding('util').triggerFatalException(e); - }); - // Handle any nextTicks added in the first tick of the program - process._tickCallback(); - return; - } + asyncESM.loaderPromise.then((loader) => { + return loader.import(pathToFileURL(process.argv[1]).pathname); + }) + .catch((e) => { + internalBinding('util').triggerFatalException(e); + }); + // Handle any nextTicks added in the first tick of the program + process._tickCallback(); + return; } Module._load(process.argv[1], null, true); // Handle any nextTicks added in the first tick of the program diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 5471ee629a..89691cd7fc 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -1,22 +1,17 @@ 'use strict'; -const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); -const { extname } = require('path'); -const { realpathSync, readFileSync } = require('fs'); +const { ERR_TYPE_MISMATCH, + ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; +const { resolve: moduleWrapResolve, setScopeType } = + internalBinding('module_wrap'); +const { pathToFileURL, fileURLToPath } = require('internal/url'); +const asyncESM = require('internal/process/esm_loader'); const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); -const { ERR_INVALID_PACKAGE_CONFIG, - ERR_TYPE_MISMATCH, - ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; -const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); -const { pathToFileURL, fileURLToPath, URL } = require('internal/url'); -const asyncESM = require('internal/process/esm_loader'); - -const realpathCache = new Map(); -// TOOD(@guybedford): Shared cache with C++ -const pjsonCache = new Map(); +const { extname, resolve: pathResolve } = require('path'); +const { realpathSync } = require('fs'); const extensionFormatMap = { '__proto__': null, @@ -34,47 +29,56 @@ const legacyExtensionFormatMap = { '.node': 'commonjs' }; -function readPackageConfig(path, parentURL) { - const existing = pjsonCache.get(path); - if (existing !== undefined) - return existing; - try { - return JSON.parse(readFileSync(path).toString()); - } catch (e) { - if (e.code === 'ENOENT') { - pjsonCache.set(path, null); - return null; - } else if (e instanceof SyntaxError) { - throw new ERR_INVALID_PACKAGE_CONFIG(path, fileURLToPath(parentURL)); - } - throw e; +function resolve(specifier, parentURL) { + if (NativeModule.canBeRequiredByUsers(specifier)) { + return { + url: specifier, + format: 'builtin' + }; } -} -function getPackageBoundaryConfig(url, parentURL) { - let pjsonURL = new URL('package.json', url); - while (true) { - const pcfg = readPackageConfig(fileURLToPath(pjsonURL), parentURL); - if (pcfg) - return pcfg; + const isMain = parentURL === undefined; + if (isMain) { + parentURL = pathToFileURL(`${process.cwd()}/`).href; - const lastPjsonURL = pjsonURL; - pjsonURL = new URL('../package.json', pjsonURL); + if (asyncESM.typeFlag) { + let entryPath = pathResolve(specifier); + const ext = extname(entryPath); + + if (!preserveSymlinksMain) { + try { + entryPath = realpathSync(entryPath); + } catch (e) { + // Not found error will throw in main resolve. + if (e.code !== 'ENOENT') + throw e; + } + } + + // Conflict between explicit extension (.mjs, .cjs) and --type + if (ext === '.cjs' && asyncESM.typeFlag === 'module' || + ext === '.mjs' && asyncESM.typeFlag === 'commonjs') { + throw new ERR_TYPE_MISMATCH( + entryPath, ext, asyncESM.typeFlag, 'extension'); + } - // Terminates at root where ../package.json equals ../../package.json - // (can't just check "/package.json" for Windows support). - if (pjsonURL.pathname === lastPjsonURL.pathname) - return; - } -} + // apply the --type flag scope + const conflictErr = + setScopeType(entryPath, asyncESM.typeFlag === 'commonjs'); -function getModuleFormat(url, isMain, parentURL) { - const pcfg = getPackageBoundaryConfig(url, parentURL); + // Conflict between package scope type and --type + if (conflictErr) { + throw new ERR_TYPE_MISMATCH( + entryPath, ext, asyncESM.typeFlag, 'scope'); + } + } + } - const legacy = !pcfg || pcfg.type !== 'module'; + const { url, legacy } = + moduleWrapResolve(specifier, parentURL, + isMain ? !preserveSymlinksMain : !preserveSymlinks); const ext = extname(url.pathname); - let format = (legacy ? legacyExtensionFormatMap : extensionFormatMap)[ext]; if (!format) { @@ -85,50 +89,6 @@ function getModuleFormat(url, isMain, parentURL) { fileURLToPath(parentURL)); } - // Check for mismatch between --type and file extension, - // and between --type and the "type" field in package.json. - if (isMain && format !== 'module' && asyncESM.typeFlag === 'module') { - // Conflict between package scope type and --type - if (ext === '.js') { - if (pcfg && pcfg.type) - throw new ERR_TYPE_MISMATCH( - fileURLToPath(url), ext, asyncESM.typeFlag, 'scope'); - // Conflict between explicit extension (.mjs, .cjs) and --type - } else { - throw new ERR_TYPE_MISMATCH( - fileURLToPath(url), ext, asyncESM.typeFlag, 'extension'); - } - } - - return format; -} - -function resolve(specifier, parentURL) { - if (NativeModule.canBeRequiredByUsers(specifier)) { - return { - url: specifier, - format: 'builtin' - }; - } - - const isMain = parentURL === undefined; - if (isMain) - parentURL = pathToFileURL(`${process.cwd()}/`).href; - - let url = moduleWrapResolve(specifier, parentURL); - - if (isMain ? !preserveSymlinksMain : !preserveSymlinks) { - const real = realpathSync(fileURLToPath(url), { - [internalFS.realpathCacheKey]: realpathCache - }); - const old = url; - url = pathToFileURL(real); - url.search = old.search; - url.hash = old.hash; - } - - const format = getModuleFormat(url, isMain, parentURL); - return { url: `${url}`, format }; } diff --git a/src/env.h b/src/env.h index 7514b14249..318e2040fe 100644 --- a/src/env.h +++ b/src/env.h @@ -82,14 +82,17 @@ struct PackageConfig { struct HasMain { enum Bool { No, Yes }; }; - struct IsESM { + struct IsModule { enum Bool { No, Yes }; }; - const Exists::Bool exists; + struct PackageType { + enum Type { None, CommonJS, Module }; + }; + Exists::Bool exists; const IsValid::Bool is_valid; const HasMain::Bool has_main; const std::string main; - const IsESM::Bool esm; + PackageType::Type type; }; } // namespace loader @@ -149,6 +152,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(channel_string, "channel") \ V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite") \ V(code_string, "code") \ + V(commonjs_string, "commonjs") \ V(config_string, "config") \ V(constants_string, "constants") \ V(crypto_dsa_string, "dsa") \ @@ -793,7 +797,7 @@ class Environment { inline uint32_t get_next_script_id(); inline uint32_t get_next_function_id(); - std::unordered_map + std::unordered_map package_json_cache; inline double* heap_statistics_buffer() const; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index f505c09cec..4c569dac9f 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -19,6 +19,7 @@ using node::contextify::ContextifyContext; using node::url::URL; using node::url::URL_FLAGS_FAILED; using v8::Array; +using v8::Boolean; using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; @@ -474,6 +475,17 @@ std::string ReadFile(uv_file file) { return contents; } +Maybe RealPath(const std::string& path) { + uv_fs_t fs_req; + const int r = uv_fs_realpath(nullptr, &fs_req, path.c_str(), nullptr); + if (r < 0) { + return Nothing(); + } + std::string link_path = std::string(static_cast(fs_req.ptr)); + uv_fs_req_cleanup(&fs_req); + return Just(link_path); +} + enum DescriptorType { FILE, DIRECTORY, @@ -532,14 +544,21 @@ Maybe ReadIfFile(const std::string& path) { using Exists = PackageConfig::Exists; using IsValid = PackageConfig::IsValid; using HasMain = PackageConfig::HasMain; -using IsESM = PackageConfig::IsESM; +using PackageType = PackageConfig::PackageType; -Maybe GetPackageConfig(Environment* env, - const std::string& path, - const URL& base) { +Maybe GetPackageConfigMutable(Environment* env, + const std::string& path, + const URL& base) { auto existing = env->package_json_cache.find(path); if (existing != env->package_json_cache.end()) { - return Just(&existing->second); + PackageConfig* pcfg = &existing->second; + if (pcfg->is_valid == IsValid::No) { + std::string msg = "Invalid JSON in '" + path + + "' imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); + return Nothing(); + } + return Just(pcfg); } Maybe source = ReadIfFile(path); @@ -547,7 +566,7 @@ Maybe GetPackageConfig(Environment* env, if (source.IsNothing()) { auto entry = env->package_json_cache.emplace(path, PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "", - IsESM::No }); + PackageType::None }); return Just(&entry.first->second); } @@ -574,11 +593,11 @@ Maybe GetPackageConfig(Environment* env, if (!parsed) { (void)env->package_json_cache.emplace(path, PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "", - IsESM::No }); + PackageType::None }); std::string msg = "Invalid JSON in '" + path + "' imported from " + base.ToFilePath(); node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); - return Nothing(); + return Nothing(); } Local pkg_main; @@ -592,12 +611,15 @@ Maybe GetPackageConfig(Environment* env, main_std.assign(std::string(*main_utf8, main_utf8.length())); } - IsESM::Bool esm = IsESM::No; + PackageType::Type pkg_type = PackageType::None; Local type_v; if (pkg_json->Get(env->context(), env->type_string()).ToLocal(&type_v)) { if (type_v->StrictEquals(env->module_string())) { - esm = IsESM::Yes; + pkg_type = PackageType::Module; + } else if (type_v->StrictEquals(env->commonjs_string())) { + pkg_type = PackageType::CommonJS; } + // ignore unknown types for forwards compatibility } Local exports_v; @@ -606,21 +628,55 @@ Maybe GetPackageConfig(Environment* env, (exports_v->IsObject() || exports_v->IsString() || exports_v->IsBoolean())) { Persistent exports; - // esm = IsESM::Yes; exports.Reset(env->isolate(), exports_v); auto entry = env->package_json_cache.emplace(path, PackageConfig { Exists::Yes, IsValid::Yes, has_main, main_std, - esm }); + pkg_type }); return Just(&entry.first->second); } auto entry = env->package_json_cache.emplace(path, PackageConfig { Exists::Yes, IsValid::Yes, has_main, main_std, - esm }); + pkg_type }); return Just(&entry.first->second); } +Maybe GetPackageConfig(Environment* env, + const std::string& path, + const URL& base) { + Maybe pcfg_result = GetPackageConfigMutable(env, path, base); + if (pcfg_result.IsNothing()) + return Nothing(); + const PackageConfig* pcfg = pcfg_result.FromJust(); + return Just(pcfg); +} + +Maybe GetPackageScopeConfig(Environment* env, + const URL& resolved, + const URL& base) { + URL pjson_url("./package.json", &resolved); + while (true) { + Maybe pkg_cfg = + GetPackageConfig(env, pjson_url.ToFilePath(), base); + if (pkg_cfg.IsNothing()) return pkg_cfg; + if (pkg_cfg.FromJust()->exists == Exists::Yes) return pkg_cfg; + + URL last_pjson_url = pjson_url; + pjson_url = URL("../package.json", pjson_url); + + // Terminates at root where ../package.json equals ../../package.json + // (can't just check "/package.json" for Windows support). + if (pjson_url.path() == last_pjson_url.path()) { + auto entry = env->package_json_cache.emplace(pjson_url.ToFilePath(), + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "", + PackageType::None }); + const PackageConfig* pcfg = &entry.first->second; + return Just(pcfg); + } + } +} + /* * Legacy CommonJS main resolution: * 1. let M = pkg_url + (json main field) @@ -754,7 +810,7 @@ Maybe PackageMainResolve(Environment* env, return FinalizeResolution(env, URL("index", pjson_url), base); } } - if (pcfg.esm == IsESM::No) { + if (pcfg.type != PackageType::Module) { Maybe resolved = LegacyMainResolve(pjson_url, pcfg); if (!resolved.IsNothing()) { return resolved; @@ -830,8 +886,8 @@ Maybe PackageResolve(Environment* env, } // anonymous namespace Maybe Resolve(Environment* env, - const std::string& specifier, - const URL& base) { + const std::string& specifier, + const URL& base) { // Order swapped from spec for minor perf gain. // Ok since relative URLs cannot parse as URLs. URL resolved; @@ -848,12 +904,74 @@ Maybe Resolve(Environment* env, return FinalizeResolution(env, resolved, base); } -void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { +Maybe IsLegacyType(Environment* env, + const URL& resolved) { + Maybe pcfg = + GetPackageScopeConfig(env, resolved, resolved); + if (pcfg.IsNothing()) { + return Nothing(); + } + bool legacy = pcfg.FromJust()->exists == Exists::No || + pcfg.FromJust()->type != PackageType::Module; + return Just(legacy); +} + +void ModuleWrap::SetScopeType(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - // module.resolve(specifier, url) + // module.setScopeType(resolved, legacy) CHECK_EQ(args.Length(), 2); + CHECK(args[0]->IsString()); + Utf8Value path_utf8(env->isolate(), args[0]); + std::string path(*path_utf8, path_utf8.length()); + + TryCatchScope try_catch(env); + + const URL resolved = URL::FromFilePath(path); + + CHECK(args[1]->IsBoolean()); + bool legacy = args[1]->IsTrue(); + + URL pjson_url("./package.json", resolved); + + Maybe pcfg_result = + GetPackageConfigMutable(env, pjson_url.ToFilePath(), resolved); + + if (pcfg_result.IsNothing()) { + CHECK(try_catch.HasCaught()); + try_catch.ReThrow(); + return; + } + + PackageType::Type pkg_type = + legacy ? PackageType::CommonJS : PackageType::Module; + + PackageConfig* pcfg = pcfg_result.FromJust(); + + bool conflict_err = false; + + if (pcfg->exists == Exists::No) { + pcfg->exists = Exists::Yes; + pcfg->type = pkg_type; + } else { + if ((legacy && pcfg->type == PackageType::Module) || + (!legacy && pcfg->type == PackageType::CommonJS)) { + conflict_err = true; + } else { + pcfg->type = pkg_type; + } + } + + args.GetReturnValue().Set(Boolean::New(env->isolate(), conflict_err)); +} + +void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + // module.resolve(specifier, url, realpath) + CHECK_EQ(args.Length(), 3); + CHECK(args[0]->IsString()); Utf8Value specifier_utf8(env->isolate(), args[0]); std::string specifier_std(*specifier_utf8, specifier_utf8.length()); @@ -862,6 +980,9 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { Utf8Value url_utf8(env->isolate(), args[1]); URL url(*url_utf8, url_utf8.length()); + CHECK(args[2]->IsBoolean()); + bool realpath = args[2]->IsTrue(); + if (url.flags() & URL_FLAGS_FAILED) { return node::THROW_ERR_INVALID_ARG_TYPE( env, "second argument is not a URL string"); @@ -882,7 +1003,44 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { URL resolution = result.FromJust(); CHECK(!(resolution.flags() & URL_FLAGS_FAILED)); - Local resolved = resolution.ToObject(env).ToLocalChecked(); + if (realpath) { + Maybe real_resolution = + node::loader::RealPath(resolution.ToFilePath()); + // Note: Ensure module resolve FS error handling consistency + if (real_resolution.IsNothing()) { + std::string msg = "realpath error resolving " + resolution.ToFilePath(); + env->ThrowError(msg.c_str()); + try_catch.ReThrow(); + return; + } + std::string fragment = resolution.fragment(); + std::string query = resolution.query(); + resolution = URL::FromFilePath(real_resolution.FromJust()); + resolution.set_fragment(fragment); + resolution.set_query(query); + } + + Maybe legacy = node::loader::IsLegacyType(env, resolution); + if (result.IsNothing()) { + CHECK(try_catch.HasCaught()); + try_catch.ReThrow(); + return; + } + CHECK(!try_catch.HasCaught()); + + Local resolved = Object::New(env->isolate()); + + resolved->DefineOwnProperty( + env->context(), + env->legacy_string(), + Boolean::New(env->isolate(), legacy.FromJust()), + v8::ReadOnly).FromJust(); + + resolved->DefineOwnProperty( + env->context(), + env->url_string(), + resolution.ToObject(env).ToLocalChecked(), + v8::ReadOnly).FromJust(); args.GetReturnValue().Set(resolved); } @@ -1020,6 +1178,7 @@ void ModuleWrap::Initialize(Local target, target->Set(env->context(), FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"), tpl->GetFunction(context).ToLocalChecked()).FromJust(); env->SetMethod(target, "resolve", Resolve); + env->SetMethod(target, "setScopeType", SetScopeType); env->SetMethod(target, "setImportModuleDynamicallyCallback", SetImportModuleDynamicallyCallback); diff --git a/src/module_wrap.h b/src/module_wrap.h index 9ddb99d174..7570d748a4 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -65,6 +65,7 @@ class ModuleWrap : public BaseObject { const v8::FunctionCallbackInfo& args); static void Resolve(const v8::FunctionCallbackInfo& args); + static void SetScopeType(const v8::FunctionCallbackInfo& args); static void SetImportModuleDynamicallyCallback( const v8::FunctionCallbackInfo& args); static void SetInitializeImportMetaObjectCallback( diff --git a/src/node_url.h b/src/node_url.h index e85b14e2bd..06fd9e41eb 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -153,6 +153,14 @@ class URL { return context_.fragment; } + void set_fragment(const std::string& fragment) { + context_.fragment = fragment; + } + + void set_query(const std::string& query) { + context_.query = query; + } + std::string path() const { std::string ret; for (const std::string& element : context_.path) { diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js index 84d67a8681..b493430e7f 100644 --- a/test/es-module/test-esm-type-flag-errors.js +++ b/test/es-module/test-esm-type-flag-errors.js @@ -25,11 +25,11 @@ expect('--type=module', packageWithoutTypeMain, 'package-without-type'); expect('-m', packageWithoutTypeMain, 'package-without-type'); // Check that running with conflicting --type flags throws errors -expect('--type=commonjs', mjsFile, 'ERR_REQUIRE_ESM', true); +expect('--type=commonjs', mjsFile, 'ERR_TYPE_MISMATCH', true); expect('--type=module', cjsFile, 'ERR_TYPE_MISMATCH', true); expect('-m', cjsFile, 'ERR_TYPE_MISMATCH', true); expect('--type=commonjs', packageTypeModuleMain, - 'SyntaxError', true); + 'ERR_TYPE_MISMATCH', true); expect('--type=module', packageTypeCommonJsMain, 'ERR_TYPE_MISMATCH', true); expect('-m', packageTypeCommonJsMain, diff --git a/test/es-module/test-esm-type-scope.js b/test/es-module/test-esm-type-scope.js new file mode 100644 index 0000000000..122f2338a6 --- /dev/null +++ b/test/es-module/test-esm-type-scope.js @@ -0,0 +1,11 @@ +'use strict'; +require('../common'); +const fixtures = require('../common/fixtures'); +const { spawn } = require('child_process'); +const assert = require('assert'); + +spawn(process.execPath, ['--experimental-modules', '-m', + fixtures.path('es-modules/esm-main.js')], + { stdio: 'inherit' }).on('exit', (code) => { + assert.strictEqual(code, 0); +}); diff --git a/test/fixtures/es-modules/esm-dep.js b/test/fixtures/es-modules/esm-dep.js new file mode 100644 index 0000000000..a632d766d1 --- /dev/null +++ b/test/fixtures/es-modules/esm-dep.js @@ -0,0 +1 @@ +export const asdf = 'asdf'; diff --git a/test/fixtures/es-modules/esm-main.js b/test/fixtures/es-modules/esm-main.js new file mode 100644 index 0000000000..ca1e48aad6 --- /dev/null +++ b/test/fixtures/es-modules/esm-main.js @@ -0,0 +1,3 @@ +// test we can run esm .js files with --type=module +// including a sub-dependency +import { asdf } from'./esm-dep.js'; diff --git a/test/fixtures/es-modules/package-without-type/index.js b/test/fixtures/es-modules/package-without-type/index.js index a547216cb0..a1c7b8a487 100644 --- a/test/fixtures/es-modules/package-without-type/index.js +++ b/test/fixtures/es-modules/package-without-type/index.js @@ -1,3 +1,4 @@ const identifier = 'package-without-type'; console.log(identifier); -module.exports = identifier; +if (typeof module !== 'undefined') + module.exports = identifier;