From 1a021ecbf06c226f9a20d93d292d369873eaa1d3 Mon Sep 17 00:00:00 2001 From: guybedford Date: Tue, 6 Nov 2018 02:31:15 +0200 Subject: [PATCH] linting fixes --- doc/api/esm.md | 81 ++++++++++++++------- lib/internal/modules/esm/default_resolve.js | 3 +- src/module_wrap.cc | 15 ++-- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 7accafb01a..51feedfec7 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -142,10 +142,6 @@ fs.readFileSync = () => Buffer.from('Hello, ESM'); fs.readFileSync === readFileSync; ``` -[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 - ## Resolution Algorithm ### Features @@ -157,41 +153,57 @@ The resolver has the following properties: * Relative and absolute URL resolution * No default extensions * Bare specifier package resolution lookup through node_modules -* Avoids the package fallback which breaks package isolation. This is the problem that an import to `x/y.js` will keep checking subsequent node_modules even if a package "x" is found without a "y.js". This is done through package name format detection. +* Avoids the package fallback which breaks package isolation. This is the + problem that an import to `x/y.js` will keep checking subsequent node_modules + even if a package "x" is found without a "y.js". This is done through package + name format detection. -The ESM resolver differs from the CommonJS resolver in detecting a package name based on it being either `package` or `@scoped/package`. This allows the resolver to know when a package has been matched, and then avoid a fallback for missing modules. +The ESM resolver differs from the CommonJS resolver in detecting a package name +based on it being either `package` or `@scoped/package`. This allows the +resolver to know when a package has been matched, and then avoid a fallback for +missing modules. -The main lookup is then provided by _CHECK_PACKAGE_MAIN_ which is still an experimental approach under discussion. +The main lookup is then provided by _CHECK_PACKAGE_MAIN_ which is still an +experimental approach under discussion. Currently, the main lookup will only check for _index.mjs_. ### Examples -Relative and absolute resolution without directory or extension resolution (URL resolution): +Relative and absolute resolution without directory or extension resolution +(URL resolution): -* ESM_RESOLVE(_"./a.asdf"_, _"file:///parent/path"_) -> _file:///parent/a.asdf_ -* ESM_RESOLVE(_"/a"_, _"file:///parent/path"_) -> _file:///a_ -* ESM_RESOLVE(_"file:///a"_, _"file:///parent/path"_) -> _file:///a_ +* ESM_RESOLVE("./a.asdf", "file:///parent/path") -> "file:///parent/a.asdf" +* ESMRESOLVE("/a", "file:///parent/path") -> "file:///a" +* ESMRESOLVE("file:///a", "file:///parent/path") -> "file:///a" Package resolution: -1. ESM_RESOLVE(_"pkg/x"_, _"file:///path/to/project"_) -> _file:///path/to/node_modules/pkg/x_ (if it exists) -1. ESM_RESOLVE(_"pkg/x"_, _"file:///path/to/project"_) -> _Not Found_ otherwise, even if _file:///path/node_modules/pkg/x_ exists. +1. ESMRESOLVE("pkg/x", "file:///path/to/project") -> + "file:///path/to/nodemodules/pkg/x" (if it exists) +1. ESMRESOLVE("pkg/x", "file:///path/to/project") -> + Not Found otherwise, even if "file:///path/nodemodules/pkg/x" exists. Main resolution: -1. ESM_RESOLVE(_"pkg"_, _"file:///path/to/project"_) -> _file:///path/to/project/node_modules/pkg/index.mjs_ (if it exists) -1. ESM_RESOLVE(_"pkg"_, _"file:///path/to/project"_) -> _Not Found_ otherwise, even if _file:///path/to/node_modules/pkg/index.mjs_ exists. -1. ESM_RESOLVE(_"file:///path/to/project"_) -> _file:///path/to/project/index.mjs_ (if it exists) -1. ESM_RESOLVE(_"file:///path/to/project"_) -> _Not Found_, if _file:///path/to/project/index.mjs_ does not exist. +1. ESMRESOLVE("pkg", "file:///path/to/project") -> + "file:///path/to/project/nodemodules/pkg/index.mjs" (if it exists) +1. ESMRESOLVE("pkg", "file:///path/to/project") -> Not Found otherwise, + even if "file:///path/to/nodemodules/pkg/index.mjs" exists. +1. ESMRESOLVE("file:///path/to/project") -> + "file:///path/to/project/index.mjs" (if it exists) +1. ESMRESOLVE("file:///path/to/project") -> + Not Found, if "file:///path/to/project/index.mjs" does not exist. ### Resolver Algorithm -The algorithm to resolve an ES module specifier is provided through _ESM_RESOLVE_: +The algorithm to resolve an ES module specifier is provided through +_ESM_RESOLVE_: **ESM_RESOLVE**(_specifier_, _parentURL_) > 1. If _specifier_ is a valid URL then, -> 1. Let _resolvedURL_ be the result of parsing and reserializing _specifier_ as a URL. +> 1. Let _resolvedURL_ be the result of parsing and reserializing +> _specifier_ as a URL. > 1. Return the result of **PATH_RESOLVE**(_resolvedURL_). > 1. If _specifier_ starts with _"/"_, _"./"_ or _"../"_ then, > 1. Let _resolvedURL_ be the URL resolution of _specifier_ to _parentURL_. @@ -202,7 +214,8 @@ The algorithm to resolve an ES module specifier is provided through _ESM_RESOLVE **PATH_RESOLVE**(_resolvedURL_) > 1. If the file at _resolvedURL_ exists then, > 1. Return _resolvedURL_. -> 1. Let _packageMainURL_ be the result of **PACKAGE_MAIN_RESOLVE**(_resolvedURL_). +> 1. Let _packageMainURL_ be the result of +> **PACKAGE_MAIN_RESOLVE**(_resolvedURL_). > 1. If _packageMainURL_ is not _undefined_ then, > 1. Return _packageMainURL_. > 1. Throw a _Module Not Found_ error. @@ -214,22 +227,29 @@ The algorithm to resolve an ES module specifier is provided through _ESM_RESOLVE > 1. Let _packageName_ be _undefined_. > 1. Let _packagePath_ be _undefined_. > 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. Set _packageName_ to the substring of _packageSpecifier_ until the +> first _"/"_ separator or the end of the string. > 1. If _packageSpecifier_ starts with _"@"_ then, > 1. If _packageSpecifier_ does not contain a _"/"_ separator then, > 1. Throw a _Invalid Package Name_ error. -> 1. Set _packageName_ to the substring of _packageSpecifier_ until the second _"/"_ separator or the end of the string. -> 1. Let _packagePath_ be the substring of _packageSpecifier_ from the position at the length of _packageName_ plus one, if any. +> 1. Set _packageName_ to the substring of _packageSpecifier_ +> until the second _"/"_ separator or the end of the string. +> 1. Let _packagePath_ 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: _packagePath_ is either empty, or a path without a leading separator. +> 1. Assert: _packagePath_ is either empty, or a path without a leading +> separator. > 1. Note: Further package name encoding validations can be added here. -> 1. If _packagePath_ is empty and _packageName_ is a Node.js builtin module then, +> 1. If _packagePath_ is empty and _packageName_ is a Node.js builtin +> module then, > 1. Return _"node:${packageName}"_. > 1. Set _parentURL_ to the parent folder URL of _parentURL_. > 1. While _parentURL_ contains a non-empty _pathname_, -> 1. Let _packageURL_ be the URL resolution of _"${parentURL}/node_modules/${packageName}"_. +> 1. Let _packageURL_ be the URL resolution of +> _"${parentURL}/node_modules/${packageName}"_. > 1. If the folder at _packageURL_ does not exist then, -> 1. Note: This check can be optimized out where possible in implementation. +> 1. Note: This check can be optimized out where possible in +> implementation. > 1. Set _parentURL_ to the parent URL path of _parentURL_. > 1. Continue the next loop iteration. > 1. If _packagePath_ is empty then, @@ -237,7 +257,8 @@ The algorithm to resolve an ES module specifier is provided through _ESM_RESOLVE > 1. If _url_ is not _undefined_ then, > 1. Return _url_. > 1. Otherwise, -> 1. Let _url_ be equal to the URL resolution of _packagePath_ in _packageURL_. +> 1. Let _url_ be equal to the URL resolution of _packagePath_ in +> _packageURL_. > 1. If the file at _url_ exists then, > 1. Return _url_. > 1. Otherwise, if _url_ is a directory then, @@ -252,3 +273,7 @@ The algorithm to resolve an ES module specifier is provided through _ESM_RESOLVE > 1. Return _"${packageURL}/index.mjs"_. > 1. Return _undefined_. + +[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/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 60bb97301e..2ce9453680 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -34,7 +34,8 @@ 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, fileURLToPath(base), found); + error = new ERR_MODULE_RESOLUTION_LEGACY(target, + fileURLToPath(base), found); } catch { // ignore } diff --git a/src/module_wrap.cc b/src/module_wrap.cc index fe7c2beb88..c05dfcb34d 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -508,7 +508,7 @@ Maybe PackageResolve(Environment* env, } sep_index = specifier.find('/', sep_index + 1); } - std::string pkg_name = specifier.substr(0, + std::string pkg_name = specifier.substr(0, sep_index == std::string::npos ? std::string::npos : sep_index); std::string pkg_path; if (sep_index == std::string::npos || @@ -554,9 +554,9 @@ Maybe PackageResolve(Environment* env, return Nothing(); } -Maybe PathResolve (Environment* env, - const URL& url, - const URL& base) { +Maybe PathResolve(Environment* env, + const URL& url, + const URL& base) { std::string path = url.ToFilePath(); DescriptorType check = CheckDescriptor(path); if (check == FILE) return Just(url); @@ -606,14 +606,15 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { return node::THROW_ERR_INVALID_ARG_TYPE( env, "second argument is not a URL string"); } - + TryCatch try_catch(env->isolate()); 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 + + } 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_MISSING_MODULE(env, msg.c_str()); try_catch.ReThrow();