From ea06ef2b1dddcdfb087279cc1eed4e471afe0d74 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 5 Mar 2019 04:20:15 -0500 Subject: [PATCH] esm: add --es-module-specifier-resolution There are currently two supported values "explicit" and "node" --- 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 -- 29 files changed, 170 insertions(+), 74 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/src/module_wrap.cc b/src/module_wrap.cc index f8e8c894d3..c507eb1205 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -45,6 +45,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, @@ -669,13 +677,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()); @@ -686,32 +738,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, @@ -761,7 +817,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. @@ -791,7 +847,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 485ae02ffc..e966b024fe 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -253,6 +253,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 d8a8440c1c..f0973679c6 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -92,6 +92,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" -}