diff --git a/doc/api/esm.md b/doc/api/esm.md index a584e3ce25..4120de4c80 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -541,14 +541,11 @@ 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, it overrides the ESM_FORMAT result while +providing errors in the case of explicit conflicts.
-Resolver algorithm psuedocode +Resolver algorithm specification **ESM_RESOLVE(_specifier_, _parentURL_, _isMain_)** > 1. Let _resolvedURL_ be **undefined**. @@ -628,25 +625,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 533e65385a..06062fac32 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -866,17 +866,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('task_queue').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('task_queue').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 ef1e1a54fe..be7d9d1a3e 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -1,23 +1,16 @@ 'use strict'; -const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); -const { extname } = require('path'); -const { realpathSync, readFileSync } = require('fs'); -const { getOptionValue } = require('internal/options'); -const preserveSymlinks = getOptionValue('--preserve-symlinks'); -const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); -const { ERR_INVALID_PACKAGE_CONFIG, - ERR_TYPE_MISMATCH, +const { ERR_TYPE_MISMATCH, ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; +const { getOptionValue } = require('internal/options'); const experimentalJsonModules = getOptionValue('--experimental-json-modules'); const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); -const { pathToFileURL, fileURLToPath, URL } = require('internal/url'); +const { pathToFileURL, fileURLToPath } = 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 preserveSymlinks = getOptionValue('--preserve-symlinks'); +const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); +const { extname } = require('path'); const extensionFormatMap = { '__proto__': null, @@ -45,75 +38,6 @@ if (experimentalJsonModules) { }); } -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 getPackageBoundaryConfig(url, parentURL) { - let pjsonURL = new URL('package.json', url); - while (true) { - const pcfg = readPackageConfig(fileURLToPath(pjsonURL), parentURL); - if (pcfg) - return pcfg; - - const lastPjsonURL = pjsonURL; - pjsonURL = new URL('../package.json', pjsonURL); - - // Terminates at root where ../package.json equals ../../package.json - // (can't just check "/package.json" for Windows support). - if (pjsonURL.pathname === lastPjsonURL.pathname) - return; - } -} - -function getModuleFormat(url, isMain, parentURL) { - const pcfg = getPackageBoundaryConfig(url, parentURL); - - const legacy = !pcfg || pcfg.type !== 'module'; - - const ext = extname(url.pathname); - - let format = (legacy ? legacyExtensionFormatMap : extensionFormatMap)[ext]; - - if (!format) { - if (isMain) - format = legacy ? 'commonjs' : 'module'; - else - throw new ERR_UNKNOWN_FILE_EXTENSION(fileURLToPath(url), - 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 { @@ -126,19 +50,39 @@ function resolve(specifier, parentURL) { if (isMain) parentURL = pathToFileURL(`${process.cwd()}/`).href; - let url = moduleWrapResolve(specifier, parentURL); + const { url, type } = + moduleWrapResolve(specifier, parentURL, + isMain ? !preserveSymlinksMain : !preserveSymlinks); + + const ext = extname(url.pathname); + let format = + (type !== 2 ? legacyExtensionFormatMap : extensionFormatMap)[ext]; + + if (isMain && asyncESM.typeFlag) { + // Conflict between explicit extension (.mjs, .cjs) and --type + if (ext === '.cjs' && asyncESM.typeFlag === 'module' || + ext === '.mjs' && asyncESM.typeFlag === 'commonjs') { + throw new ERR_TYPE_MISMATCH( + fileURLToPath(url), ext, asyncESM.typeFlag, 'extension'); + } - 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; + // Conflict between package scope type and --type + if (ext === '.js') { + if (type === 2 && asyncESM.typeFlag === 'commonjs' || + type === 1 && asyncESM.typeFlag === 'module') { + throw new ERR_TYPE_MISMATCH( + fileURLToPath(url), ext, asyncESM.typeFlag, 'scope'); + } + } } - const format = getModuleFormat(url, isMain, parentURL); + if (!format) { + if (isMain) + format = type === 2 ? 'module' : 'commonjs'; + else + throw new ERR_UNKNOWN_FILE_EXTENSION(fileURLToPath(url), + fileURLToPath(parentURL)); + } return { url: `${url}`, format }; } diff --git a/src/env.h b/src/env.h index e351afd9a4..03f798dba5 100644 --- a/src/env.h +++ b/src/env.h @@ -82,14 +82,14 @@ struct PackageConfig { struct HasMain { enum Bool { No, Yes }; }; - struct IsESM { - enum Bool { No, Yes }; + struct PackageType { + enum Type : uint32_t { None, CommonJS, Module }; }; const Exists::Bool exists; const IsValid::Bool is_valid; const HasMain::Bool has_main; const std::string main; - const IsESM::Bool esm; + const PackageType::Type type; }; } // namespace loader @@ -149,6 +149,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") \ @@ -218,7 +219,6 @@ 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 074d39ee38..4b771e4a2a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -476,6 +476,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, @@ -534,14 +545,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) { auto existing = env->package_json_cache.find(path); if (existing != env->package_json_cache.end()) { - return Just(&existing->second); + const 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); @@ -549,7 +567,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); } @@ -576,7 +594,7 @@ 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()); @@ -594,12 +612,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; @@ -608,21 +629,45 @@ 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 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) @@ -756,7 +801,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; @@ -832,8 +877,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; @@ -850,11 +895,21 @@ Maybe Resolve(Environment* env, return FinalizeResolution(env, resolved, base); } +Maybe GetPackageType(Environment* env, + const URL& resolved) { + Maybe pcfg = + GetPackageScopeConfig(env, resolved, resolved); + if (pcfg.IsNothing()) { + return Nothing(); + } + return Just(pcfg.FromJust()->type); +} + void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - // module.resolve(specifier, url) - CHECK_EQ(args.Length(), 2); + // module.resolve(specifier, url, realpath) + CHECK_EQ(args.Length(), 3); CHECK(args[0]->IsString()); Utf8Value specifier_utf8(env->isolate(), args[0]); @@ -864,6 +919,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"); @@ -884,7 +942,45 @@ 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 pkg_type = + node::loader::GetPackageType(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->type_string(), + Integer::New(env->isolate(), pkg_type.FromJust()), + v8::ReadOnly).FromJust(); + + resolved->DefineOwnProperty( + env->context(), + env->url_string(), + resolution.ToObject(env).ToLocalChecked(), + v8::ReadOnly).FromJust(); args.GetReturnValue().Set(resolved); } 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-symlink-type.js b/test/es-module/test-esm-symlink-type.js index 6159ebecd1..47d7917a74 100644 --- a/test/es-module/test-esm-symlink-type.js +++ b/test/es-module/test-esm-symlink-type.js @@ -64,7 +64,7 @@ symlinks.forEach((symlink) => { stdout.includes(symlink.prints)) return; assert.fail(`For ${JSON.stringify(symlink)}, ${ (symlink.errorsWithPreserveSymlinksMain) ? - 'failed to error' : 'errored unexpectedly' + 'failed to error' : `errored unexpectedly\n${err}` } with --preserve-symlinks-main`); } else { if (stdout.includes(symlink.prints)) return; 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,