From b0e1d7ed3fdceae8513214d88370da9abcacfae5 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Wed, 27 Jun 2018 22:51:38 -0400 Subject: [PATCH 1/4] modules: remove file extension and directory resolving for esm this build on top of the limitations of the package name map proposal. It removes file extensions and directory resolution in the resolver. This is only currently limited for local development. When resolving dependencies file extension and directory resolution will still work. Refs: https://github.com/domenic/package-name-maps --- doc/api/esm.md | 12 +++++- lib/internal/modules/cjs/loader.js | 7 +++- src/module_wrap.cc | 41 ++++++++++++++----- src/module_wrap.h | 6 ++- test/es-module/test-esm-basic-imports.mjs | 3 +- .../test-esm-cyclic-dynamic-import.mjs | 5 ++- test/es-module/test-esm-double-encoding.mjs | 5 ++- 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-json.mjs | 3 +- test/es-module/test-esm-live-binding.mjs | 3 +- .../test-esm-loader-invalid-format.mjs | 2 +- .../es-module/test-esm-loader-invalid-url.mjs | 2 +- ...oader-missing-dynamic-instantiate-hook.mjs | 2 +- test/es-module/test-esm-main-lookup.mjs | 6 --- 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 | 3 +- test/es-module/test-esm-shebang.mjs | 3 +- test/es-module/test-esm-snapshot.mjs | 7 ++-- 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 | 2 +- .../es-module-loaders/syntax-error-import.mjs | 2 +- test/fixtures/es-modules/loop.mjs | 2 +- test/fixtures/es-modules/pjson-main/main.js | 1 - .../es-modules/pjson-main/package.json | 3 -- .../test-esm-double-encoding-native%20.mjs | 6 +++ .../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, 115 insertions(+), 68 deletions(-) delete mode 100644 test/es-module/test-esm-main-lookup.mjs delete mode 100644 test/fixtures/es-modules/pjson-main/main.js delete mode 100644 test/fixtures/es-modules/pjson-main/package.json create mode 100644 test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index 459f87771815..5586f4018c2e 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -54,6 +54,14 @@ property: ## Notable differences between `import` and `require` +### 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 @@ -78,8 +86,8 @@ 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. diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index c218dc2cac2a..f6106da03078 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -42,6 +42,7 @@ const { const preserveSymlinks = !!process.binding('config').preserveSymlinks; const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain; const experimentalModules = !!process.binding('config').experimentalModules; +const hasLoader = !!process.binding('config').userLoader; const { ERR_INVALID_ARG_TYPE, @@ -728,7 +729,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(getURLFromFilePath(process.argv[1]).pathname); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 63e886ab0d7a..ba7eb4c29b06 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -552,6 +552,12 @@ enum ResolveExtensionsOptions { 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) { @@ -590,9 +596,9 @@ Maybe ResolveMain(Environment* env, const URL& search) { return Nothing(); } if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { - return Resolve(env, "./" + pjson.main, search, IgnoreMain); + return ResolveRecurse(env, "./" + pjson.main, search, IgnoreMain); } - return Resolve(env, pjson.main, search, IgnoreMain); + return ResolveRecurse(env, pjson.main, search, IgnoreMain); } Maybe ResolveModule(Environment* env, @@ -603,7 +609,7 @@ Maybe ResolveModule(Environment* env, do { dir = parent; Maybe check = - Resolve(env, "./node_modules/" + specifier, dir, CheckMain); + ResolveRecurse(env, "./node_modules/" + specifier, dir, CheckMain); if (!check.IsNothing()) { const size_t limit = specifier.find('/'); const size_t spec_len = @@ -636,10 +642,27 @@ Maybe ResolveDirectory(Environment* env, } // anonymous namespace -Maybe Resolve(Environment* env, +Maybe ResolveRecurse(Environment* env, const std::string& specifier, const URL& base, PackageMainCheck check_pjson_main) { + 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); + } else { + return ResolveModule(env, specifier, base); + } +} + +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 @@ -654,13 +677,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 3e19b6c9eb3e..16bdc7f6e322 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -17,11 +17,15 @@ enum PackageMainCheck : bool { IgnoreMain = false }; -v8::Maybe Resolve(Environment* env, +v8::Maybe ResolveRecurse(Environment* env, const std::string& specifier, const url::URL& base, PackageMainCheck read_pkg_json = CheckMain); +v8::Maybe Resolve(Environment* env, + const std::string& specifier, + const url::URL& base); + class ModuleWrap : public BaseObject { public: static const std::string EXTENSIONS[]; diff --git a/test/es-module/test-esm-basic-imports.mjs b/test/es-module/test-esm-basic-imports.mjs index 78a4106f9449..d9bb22be0a5f 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 c8dfff919c2f..a207efc73ecb 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 c81d0530d332..9366d4bd6bcc 100644 --- a/test/es-module/test-esm-double-encoding.mjs +++ b/test/es-module/test-esm-double-encoding.mjs @@ -1,6 +1,7 @@ // Flags: --experimental-modules -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; // 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-encoded-path.mjs b/test/es-module/test-esm-encoded-path.mjs index 365a425afa50..2cabfdacff39 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 4e777412a346..cf110ff2900e 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 c17e0e20d49b..4c34b337fb89 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-json.mjs b/test/es-module/test-esm-json.mjs index a7146d19a9ad..059470d63624 100644 --- a/test/es-module/test-esm-json.mjs +++ b/test/es-module/test-esm-json.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 json from '../fixtures/es-modules/json.json'; diff --git a/test/es-module/test-esm-live-binding.mjs b/test/es-module/test-esm-live-binding.mjs index d151e004dff4..880a6c389b42 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 f8714d4aa11d..e4e4e30f5cc2 100644 --- a/test/es-module/test-esm-loader-invalid-format.mjs +++ b/test/es-module/test-esm-loader-invalid-format.mjs @@ -1,5 +1,5 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs -import { expectsError, mustCall } from '../common'; +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 43971a2e6e3b..44bacf9347c8 100644 --- a/test/es-module/test-esm-loader-invalid-url.mjs +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -1,5 +1,5 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs -import { expectsError, mustCall } from '../common'; +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 f2b37f7e8a4d..a650bb1c31a1 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,6 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs -import { expectsError } from '../common'; +import { expectsError } from '../common.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 deleted file mode 100644 index ca313a1d2644..000000000000 --- a/test/es-module/test-esm-main-lookup.mjs +++ /dev/null @@ -1,6 +0,0 @@ -// Flags: --experimental-modules -import '../common'; -import assert from 'assert'; -import main from '../fixtures/es-modules/pjson-main'; - -assert.strictEqual(main, 'main'); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs index 3aae9230de36..e235f598cb1d 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 dcd159f6c872..6b58e074ff21 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 5119957bae7c..b5be2d7e63b5 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 ff32cde36ff2..fc7617930cf9 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 '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; import '../fixtures/es-module-require-cache/preload.js'; import '../fixtures/es-module-require-cache/counter.js'; import assert from 'assert'; diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs index 5c274d835c8a..879db4431622 100644 --- a/test/es-module/test-esm-shared-loader-dep.mjs +++ b/test/es-module/test-esm-shared-loader-dep.mjs @@ -1,5 +1,6 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs -import '../common'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; import assert from 'assert'; import '../fixtures/es-modules/test-esm-ok.mjs'; import dep from '../fixtures/es-module-loaders/loader-dep.js'; diff --git a/test/es-module/test-esm-shebang.mjs b/test/es-module/test-esm-shebang.mjs index d5faace4792f..486e04dadece 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-snapshot.mjs b/test/es-module/test-esm-snapshot.mjs index 3d4b44bbdd5b..3997e24ed703 100644 --- a/test/es-module/test-esm-snapshot.mjs +++ b/test/es-module/test-esm-snapshot.mjs @@ -1,7 +1,8 @@ // Flags: --experimental-modules -import '../common'; -import '../fixtures/es-modules/esm-snapshot-mutator'; -import one from '../fixtures/es-modules/esm-snapshot'; +/* eslint-disable node-core/required-modules */ +import '../common/index.mjs'; +import '../fixtures/es-modules/esm-snapshot-mutator.js'; +import one from '../fixtures/es-modules/esm-snapshot.js'; import assert from 'assert'; assert.strictEqual(one, 1); diff --git a/test/es-module/test-esm-symlink-main.js b/test/es-module/test-esm-symlink-main.js index f7631ef2e5bd..871180f5ccf4 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 232925a52e3f..9b9eb98cd984 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 541127eee591..34f843cd7d70 100644 --- a/test/es-module/test-esm-throw-undefined.mjs +++ b/test/es-module/test-esm-throw-undefined.mjs @@ -5,7 +5,7 @@ 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/syntax-error-import.mjs b/test/fixtures/es-module-loaders/syntax-error-import.mjs index 9cad68c7ce2a..3a6bc5effc19 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-modules/loop.mjs b/test/fixtures/es-modules/loop.mjs index 1b5cab10edc7..3d2ddd2eb730 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.js b/test/fixtures/es-modules/pjson-main/main.js deleted file mode 100644 index dfdd47b87731..000000000000 --- 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/package.json b/test/fixtures/es-modules/pjson-main/package.json deleted file mode 100644 index c13b8cf6acfd..000000000000 --- a/test/fixtures/es-modules/pjson-main/package.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "main": "main.js" -} diff --git a/test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs b/test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs new file mode 100644 index 000000000000..a3bfe972e5a8 --- /dev/null +++ b/test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs @@ -0,0 +1,6 @@ +'use strict'; + +// Trivial test to assert we can load files with `%` in their pathname. +// Imported by `test-esm-double-encoding.mjs`. + +export default 42; diff --git a/test/message/esm_display_syntax_error_import.mjs b/test/message/esm_display_syntax_error_import.mjs index 87cedf1d4edd..12d10270e968 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 31ee2b6f4b11..48f2e2fb7465 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 32c0edb35068..a53bbbcd1924 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 b067a77942fa..3e1024db8aa0 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 e74b70bec8cc..5905d2a95478 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 3d20316647c6..9e7eab295e87 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 cc6140e181ad16646f1ad2bce1b6941130c4638e Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Thu, 12 Jul 2018 10:40:40 +0200 Subject: [PATCH 2/4] refactor ResolveRecurse into ResolveMain --- src/module_wrap.cc | 47 ++++++++++++++-------------------------------- src/module_wrap.h | 10 ---------- 2 files changed, 14 insertions(+), 43 deletions(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index ba7eb4c29b06..cd7ef5a44b8d 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -595,10 +595,20 @@ Maybe ResolveMain(Environment* env, const URL& search) { pjson.has_main == HasMain::No) { return Nothing(); } - if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { - return ResolveRecurse(env, "./" + pjson.main, search, IgnoreMain); + + URL resolved; + if (ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { + resolved = URL(pjson.main, search); + } else { + resolved = URL("./" + pjson.main, search); } - return ResolveRecurse(env, pjson.main, search, IgnoreMain); + Maybe file = ResolveExtensions(resolved); + if (!file.IsNothing()) + return file; + if (specifier.back() != '/') { + resolved = URL(specifier + "/", search); + } + return ResolveIndex(resolved); } Maybe ResolveModule(Environment* env, @@ -609,7 +619,7 @@ Maybe ResolveModule(Environment* env, do { dir = parent; Maybe check = - ResolveRecurse(env, "./node_modules/" + specifier, dir, CheckMain); + Resolve(env, "./node_modules/" + specifier, dir, CheckMain); if (!check.IsNothing()) { const size_t limit = specifier.find('/'); const size_t spec_len = @@ -629,37 +639,8 @@ 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 ResolveRecurse(Environment* env, - const std::string& specifier, - const URL& base, - PackageMainCheck check_pjson_main) { - 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); - } else { - return ResolveModule(env, specifier, base); - } -} - Maybe Resolve(Environment* env, const std::string& specifier, const URL& base) { diff --git a/src/module_wrap.h b/src/module_wrap.h index 16bdc7f6e322..c0be3d125bd2 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -12,16 +12,6 @@ namespace node { namespace loader { -enum PackageMainCheck : bool { - CheckMain = true, - IgnoreMain = false -}; - -v8::Maybe ResolveRecurse(Environment* env, - const std::string& specifier, - const url::URL& base, - PackageMainCheck read_pkg_json = CheckMain); - v8::Maybe Resolve(Environment* env, const std::string& specifier, const url::URL& base); From ba1aefd245ee0544f95115b70d3f0ab36655340e Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 15 Jul 2018 12:58:43 +0200 Subject: [PATCH 3/4] build fixes --- src/module_wrap.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index cd7ef5a44b8d..38effa2df696 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -605,8 +605,8 @@ Maybe ResolveMain(Environment* env, const URL& search) { Maybe file = ResolveExtensions(resolved); if (!file.IsNothing()) return file; - if (specifier.back() != '/') { - resolved = URL(specifier + "/", search); + if (resolved.back() != '/') { + resolved = URL(resolved + "/", search); } return ResolveIndex(resolved); } @@ -619,7 +619,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 = From 3e16cf5c12ed74850fc33dc77c39d7a9bb08757a Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 9 Aug 2018 16:28:34 -0400 Subject: [PATCH 4/4] fixup: more build fixes --- src/module_wrap.cc | 4 ++-- .../test-esm-loader-missing-dynamic-instantiate-hook.mjs | 2 +- test/es-module/test-esm-throw-undefined.mjs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 38effa2df696..0fee562e982c 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -605,8 +605,8 @@ Maybe ResolveMain(Environment* env, const URL& search) { Maybe file = ResolveExtensions(resolved); if (!file.IsNothing()) return file; - if (resolved.back() != '/') { - resolved = URL(resolved + "/", search); + if (pjson.main.back() != '/') { + resolved = URL(pjson.main + "/", search); } return ResolveIndex(resolved); } 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 a650bb1c31a1..c91836312239 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,6 @@ // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs -import { expectsError } from '../common.mjs'; +import { expectsError } from '../common/index.mjs'; import('test').catch(expectsError({ code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK', diff --git a/test/es-module/test-esm-throw-undefined.mjs b/test/es-module/test-esm-throw-undefined.mjs index 34f843cd7d70..4c091e615326 100644 --- a/test/es-module/test-esm-throw-undefined.mjs +++ b/test/es-module/test-esm-throw-undefined.mjs @@ -1,5 +1,5 @@ // Flags: --experimental-modules -import '../common'; +import '../common/index.mjs'; import assert from 'assert'; async function doTest() {