From f14420e11b26de9d900384c07e91800a5e405554 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 12 Feb 2018 13:02:42 +0200 Subject: [PATCH] esm: support main without extension and pjson cache This adds support for ensuring that the top-level main into Node is supported loading when it has no extension for backwards-compat with NodeJS bin workflows. In addition package.json caching is implemented in the module lookup process. --- doc/api/esm.md | 12 +- lib/internal/loader/DefaultResolve.js | 16 ++- lib/internal/loader/Loader.js | 58 ++------- lib/internal/loader/Translators.js | 6 +- lib/internal/process/modules.js | 41 +++++- lib/module.js | 29 ++--- src/module_wrap.cc | 119 ++++++++++-------- src/module_wrap.h | 6 + .../es-module-loaders/example-loader.mjs | 5 +- test/fixtures/es-module-loaders/js-loader.mjs | 6 +- test/fixtures/es-modules/noext | 1 + .../test-module-main-extension-lookup.js | 2 + 12 files changed, 166 insertions(+), 135 deletions(-) create mode 100644 test/fixtures/es-modules/noext diff --git a/doc/api/esm.md b/doc/api/esm.md index 2cfa5a9f29c753..529f6e422b294c 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -117,16 +117,19 @@ The resolve hook returns the resolved file URL and module format for a given module specifier and parent file URL: ```js -import url from 'url'; +const baseURL = new URL('file://'); +baseURL.pathname = process.cwd() + '/'; export async function resolve(specifier, parentModuleURL, defaultResolver) { return { - url: new URL(specifier, parentModuleURL).href, + url: new URL(specifier, parentModuleURL || baseURL).href, format: 'esm' }; } ``` +The parentURL is provided as `undefined` when performing main NodeJS load itself. + The default NodeJS ES module resolution function is provided as a third argument to the resolver for easy compatibility workflows. @@ -155,7 +158,10 @@ import Module from 'module'; const builtins = Module.builtinModules; const JS_EXTENSIONS = new Set(['.js', '.mjs']); -export function resolve(specifier, parentModuleURL/*, defaultResolve */) { +const baseURL = new URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { if (builtins.includes(specifier)) { return { url: specifier, diff --git a/lib/internal/loader/DefaultResolve.js b/lib/internal/loader/DefaultResolve.js index 69dd9537c18c2d..0d61b5b48be632 100644 --- a/lib/internal/loader/DefaultResolve.js +++ b/lib/internal/loader/DefaultResolve.js @@ -2,7 +2,6 @@ const { URL } = require('url'); const CJSmodule = require('module'); -const internalURLModule = require('internal/url'); const internalFS = require('internal/fs'); const NativeModule = require('native_module'); const { extname } = require('path'); @@ -11,6 +10,7 @@ const preserveSymlinks = !!process.binding('config').preserveSymlinks; const errors = require('internal/errors'); const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); const StringStartsWith = Function.call.bind(String.prototype.startsWith); +const { getURLFromFilePath, getPathFromURL } = require('internal/url'); const realpathCache = new Map(); @@ -57,7 +57,8 @@ function resolve(specifier, parentURL) { let url; try { - url = search(specifier, parentURL); + url = search(specifier, + parentURL || getURLFromFilePath(`${process.cwd()}/`).href); } catch (e) { if (typeof e.message === 'string' && StringStartsWith(e.message, 'Cannot find module')) @@ -66,17 +67,22 @@ function resolve(specifier, parentURL) { } if (!preserveSymlinks) { - const real = realpathSync(internalURLModule.getPathFromURL(url), { + const real = realpathSync(getPathFromURL(url), { [internalFS.realpathCacheKey]: realpathCache }); const old = url; - url = internalURLModule.getURLFromFilePath(real); + url = getURLFromFilePath(real); url.search = old.search; url.hash = old.hash; } const ext = extname(url.pathname); - return { url: `${url}`, format: extensionFormatMap[ext] || ext }; + + const format = extensionFormatMap[ext] || parentURL === undefined && 'cjs'; + if (!format) + throw new errors.Error('ERR_UNKNOWN_FILE_EXTENSION', url.pathname); + + return { url: `${url}`, format }; } module.exports = resolve; diff --git a/lib/internal/loader/Loader.js b/lib/internal/loader/Loader.js index eda42645f170f6..ba8555cfe63721 100644 --- a/lib/internal/loader/Loader.js +++ b/lib/internal/loader/Loader.js @@ -1,51 +1,21 @@ 'use strict'; -const path = require('path'); -const { getURLFromFilePath, URL } = require('internal/url'); const errors = require('internal/errors'); - const ModuleMap = require('internal/loader/ModuleMap'); const ModuleJob = require('internal/loader/ModuleJob'); const defaultResolve = require('internal/loader/DefaultResolve'); const createDynamicModule = require('internal/loader/CreateDynamicModule'); const translators = require('internal/loader/Translators'); -const { setImportModuleDynamicallyCallback } = internalBinding('module_wrap'); + const FunctionBind = Function.call.bind(Function.prototype.bind); const debug = require('util').debuglog('esm'); -// Returns a file URL for the current working directory. -function getURLStringForCwd() { - try { - return getURLFromFilePath(`${process.cwd()}/`).href; - } catch (e) { - e.stack; - // If the current working directory no longer exists. - if (e.code === 'ENOENT') { - return undefined; - } - throw e; - } -} - -function normalizeReferrerURL(referrer) { - if (typeof referrer === 'string' && path.isAbsolute(referrer)) { - return getURLFromFilePath(referrer).href; - } - return new URL(referrer).href; -} - /* A Loader instance is used as the main entry point for loading ES modules. * Currently, this is a singleton -- there is only one used for loading * the main module and everything in its dependency graph. */ class Loader { - constructor(base = getURLStringForCwd()) { - if (typeof base !== 'string') - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'base', 'string'); - - this.base = base; - this.isMain = true; - + constructor() { // methods which translate input code or other information // into es modules this.translators = translators; @@ -71,8 +41,8 @@ class Loader { this._dynamicInstantiate = undefined; } - async resolve(specifier, parentURL = this.base) { - if (typeof parentURL !== 'string') + async resolve(specifier, parentURL) { + if (parentURL !== undefined && typeof parentURL !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'parentURL', 'string'); const { url, format } = @@ -93,7 +63,7 @@ class Loader { return { url, format }; } - async import(specifier, parent = this.base) { + async import(specifier, parent) { const job = await this.getModuleJob(specifier, parent); const module = await job.run(); return module.namespace(); @@ -107,7 +77,7 @@ class Loader { this._dynamicInstantiate = FunctionBind(dynamicInstantiate, null); } - async getModuleJob(specifier, parentURL = this.base) { + async getModuleJob(specifier, parentURL) { const { url, format } = await this.resolve(specifier, parentURL); let job = this.moduleMap.get(url); if (job !== undefined) @@ -134,24 +104,16 @@ class Loader { } let inspectBrk = false; - if (this.isMain) { - if (process._breakFirstLine) { - delete process._breakFirstLine; - inspectBrk = true; - } - this.isMain = false; + if (process._breakFirstLine) { + delete process._breakFirstLine; + inspectBrk = true; } job = new ModuleJob(this, url, loaderInstance, inspectBrk); this.moduleMap.set(url, job); return job; } - - static registerImportDynamicallyCallback(loader) { - setImportModuleDynamicallyCallback(async (referrer, specifier) => { - return loader.import(specifier, normalizeReferrerURL(referrer)); - }); - } } Object.setPrototypeOf(Loader.prototype, null); + module.exports = Loader; diff --git a/lib/internal/loader/Translators.js b/lib/internal/loader/Translators.js index d2f28774177fd6..18b1b12fd15854 100644 --- a/lib/internal/loader/Translators.js +++ b/lib/internal/loader/Translators.js @@ -19,7 +19,7 @@ const JsonParse = JSON.parse; const translators = new SafeMap(); module.exports = translators; -// Stragety for loading a standard JavaScript module +// Strategy for loading a standard JavaScript module translators.set('esm', async (url) => { const source = `${await readFileAsync(new URL(url))}`; debug(`Translating StandardModule ${url}`); @@ -62,7 +62,7 @@ translators.set('builtin', async (url) => { }); }); -// Stragety for loading a node native module +// Strategy for loading a node native module translators.set('addon', async (url) => { debug(`Translating NativeModule ${url}`); return createDynamicModule(['default'], url, (reflect) => { @@ -74,7 +74,7 @@ translators.set('addon', async (url) => { }); }); -// Stragety for loading a JSON file +// Strategy for loading a JSON file translators.set('json', async (url) => { debug(`Translating JSONModule ${url}`); return createDynamicModule(['default'], url, (reflect) => { diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index eda47f80cddeb4..40588b3396d088 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -1,17 +1,52 @@ 'use strict'; const { + setImportModuleDynamicallyCallback, setInitializeImportMetaObjectCallback } = internalBinding('module_wrap'); +const { getURLFromFilePath } = require('internal/url'); +const Loader = require('internal/loader/Loader'); +const path = require('path'); +const { URL } = require('url'); + +function normalizeReferrerURL(referrer) { + if (typeof referrer === 'string' && path.isAbsolute(referrer)) { + return getURLFromFilePath(referrer).href; + } + return new URL(referrer).href; +} + function initializeImportMetaObject(wrap, meta) { meta.url = wrap.url; } function setupModules() { setInitializeImportMetaObjectCallback(initializeImportMetaObject); + + let ESMLoader = new Loader(); + const loaderPromise = (async () => { + const userLoader = process.binding('config').userLoader; + if (userLoader) { + const hooks = await ESMLoader.import( + userLoader, getURLFromFilePath(`${process.cwd()}/`).href); + ESMLoader = new Loader(); + ESMLoader.hook(hooks); + exports.ESMLoader = ESMLoader; + } + return ESMLoader; + })(); + loaderPromise.catch(() => {}); + + setImportModuleDynamicallyCallback(async (referrer, specifier) => { + const loader = await loaderPromise; + return loader.import(specifier, normalizeReferrerURL(referrer)); + }); + + exports.loaderPromise = loaderPromise; + exports.ESMLoader = ESMLoader; } -module.exports = { - setup: setupModules -}; +exports.setup = setupModules; +exports.ESMLoader = undefined; +exports.loaderPromise = undefined; diff --git a/lib/module.js b/lib/module.js index 5ee537f157289b..c3250608ebe0f1 100644 --- a/lib/module.js +++ b/lib/module.js @@ -24,7 +24,6 @@ const NativeModule = require('native_module'); const util = require('util'); const { decorateErrorStack } = require('internal/util'); -const internalModule = require('internal/module'); const { getURLFromFilePath } = require('internal/url'); const vm = require('vm'); const assert = require('assert').ok; @@ -35,6 +34,7 @@ const { internalModuleReadJSON, internalModuleStat } = process.binding('fs'); +const internalModule = require('internal/module'); const preserveSymlinks = !!process.binding('config').preserveSymlinks; const experimentalModules = !!process.binding('config').experimentalModules; @@ -43,10 +43,9 @@ const errors = require('internal/errors'); module.exports = Module; // these are below module.exports for the circular reference -const Loader = require('internal/loader/Loader'); +const internalESModule = require('internal/process/modules'); const ModuleJob = require('internal/loader/ModuleJob'); const createDynamicModule = require('internal/loader/CreateDynamicModule'); -let ESMLoader; function stat(filename) { filename = path.toNamespacedPath(filename); @@ -447,7 +446,6 @@ Module._resolveLookupPaths = function(request, parent, newReturn) { return (newReturn ? parentDir : [id, parentDir]); }; - // Check the cache for the requested file. // 1. If a module already exists in the cache: return its exports object. // 2. If the module is native: call `NativeModule.require()` with the @@ -460,22 +458,10 @@ Module._load = function(request, parent, isMain) { debug('Module._load REQUEST %s parent: %s', request, parent.id); } - if (isMain && experimentalModules) { - (async () => { - // loader setup - if (!ESMLoader) { - ESMLoader = new Loader(); - const userLoader = process.binding('config').userLoader; - if (userLoader) { - ESMLoader.isMain = false; - const hooks = await ESMLoader.import(userLoader); - ESMLoader = new Loader(); - ESMLoader.hook(hooks); - } - } - Loader.registerImportDynamicallyCallback(ESMLoader); - await ESMLoader.import(getURLFromFilePath(request).pathname); - })() + if (experimentalModules && isMain) { + internalESModule.loaderPromise.then((loader) => { + return loader.import(getURLFromFilePath(request).pathname); + }) .catch((e) => { decorateErrorStack(e); console.error(e); @@ -578,7 +564,8 @@ Module.prototype.load = function(filename) { Module._extensions[extension](this, filename); this.loaded = true; - if (ESMLoader) { + if (experimentalModules) { + const ESMLoader = internalESModule.ESMLoader; const url = getURLFromFilePath(filename); const urlString = `${url}`; const exports = this.exports; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 4a9c847a6a0836..ce942db94612d9 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -461,10 +461,9 @@ enum CheckFileOptions { CLOSE_AFTER_CHECK }; -Maybe CheckFile(const URL& search, +Maybe CheckFile(const std::string& path, CheckFileOptions opt = CLOSE_AFTER_CHECK) { uv_fs_t fs_req; - std::string path = search.ToFilePath(); if (path.empty()) { return Nothing(); } @@ -494,15 +493,65 @@ Maybe CheckFile(const URL& search, return Just(fd); } +PackageConfig emptyPackage = { false, false, "" }; +std::unordered_map pjson_cache_; + +PackageConfig& GetPackageConfig(Environment* env, const std::string path) { + auto existing = pjson_cache_.find(path); + if (existing != pjson_cache_.end()) { + return existing->second; + } + Maybe check = CheckFile(path, LEAVE_OPEN_AFTER_CHECK); + if (check.IsNothing()) { + return pjson_cache_[path] = emptyPackage; + } + + Isolate* isolate = env->isolate(); + Local context = isolate->GetCurrentContext(); + std::string pkg_src = ReadFile(check.FromJust()); + uv_fs_t fs_req; + uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr); + uv_fs_req_cleanup(&fs_req); + + // It's not okay for the called of this method to not be able to tell + // whether an exception is pending or not. + TryCatch try_catch(isolate); + + Local src; + if (!String::NewFromUtf8(isolate, + pkg_src.c_str(), + v8::NewStringType::kNormal, + pkg_src.length()).ToLocal(&src)) { + return pjson_cache_[path] = emptyPackage; + } + + Local pkg_json; + if (!JSON::Parse(context, src).ToLocal(&pkg_json) || !pkg_json->IsObject()) + return (pjson_cache_[path] = emptyPackage); + Local pkg_main; + bool has_main = false; + std::string main_std; + if (pkg_json.As()->Get(context, env->main_string()) + .ToLocal(&pkg_main) && pkg_main->IsString()) { + has_main = true; + Utf8Value main_utf8(isolate, pkg_main.As()); + main_std = std::string(*main_utf8, main_utf8.length()); + } + + PackageConfig pjson = { true, has_main, main_std }; + return pjson_cache_[path] = pjson; +} + enum ResolveExtensionsOptions { TRY_EXACT_NAME, ONLY_VIA_EXTENSIONS }; template -Maybe ResolveExtensions(const URL& search) { +Maybe ResolveExtensions(Environment* env, const URL& search) { if (options == TRY_EXACT_NAME) { - Maybe check = CheckFile(search); + std::string filePath = search.ToFilePath(); + Maybe check = CheckFile(filePath); if (!check.IsNothing()) { return Just(search); } @@ -510,7 +559,7 @@ Maybe ResolveExtensions(const URL& search) { for (const char* extension : EXTENSIONS) { URL guess(search.path() + extension, &search); - Maybe check = CheckFile(guess); + Maybe check = CheckFile(guess.ToFilePath()); if (!check.IsNothing()) { return Just(guess); } @@ -519,50 +568,21 @@ Maybe ResolveExtensions(const URL& search) { return Nothing(); } -inline Maybe ResolveIndex(const URL& search) { - return ResolveExtensions(URL("index", search)); +inline Maybe ResolveIndex(Environment* env, const URL& search) { + return ResolveExtensions(env, URL("index", search)); } Maybe ResolveMain(Environment* env, const URL& search) { URL pkg("package.json", &search); - Maybe check = CheckFile(pkg, LEAVE_OPEN_AFTER_CHECK); - if (check.IsNothing()) { - return Nothing(); - } - - Isolate* isolate = env->isolate(); - Local context = isolate->GetCurrentContext(); - std::string pkg_src = ReadFile(check.FromJust()); - uv_fs_t fs_req; - uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr); - uv_fs_req_cleanup(&fs_req); - - // It's not okay for the called of this method to not be able to tell - // whether an exception is pending or not. - TryCatch try_catch(isolate); - - Local src; - if (!String::NewFromUtf8(isolate, - pkg_src.c_str(), - v8::NewStringType::kNormal, - pkg_src.length()).ToLocal(&src)) { - return Nothing(); - } - Local pkg_json; - if (!JSON::Parse(context, src).ToLocal(&pkg_json) || !pkg_json->IsObject()) - return Nothing(); - Local pkg_main; - if (!pkg_json.As()->Get(context, env->main_string()) - .ToLocal(&pkg_main) || !pkg_main->IsString()) { + PackageConfig pjson = GetPackageConfig(env, pkg.ToFilePath()); + if (!pjson.exists || !pjson.has_main) { return Nothing(); } - Utf8Value main_utf8(isolate, pkg_main.As()); - std::string main_std(*main_utf8, main_utf8.length()); - if (!ShouldBeTreatedAsRelativeOrAbsolutePath(main_std)) { - main_std.insert(0, "./"); + if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { + return Resolve(env, "./" + pjson.main, search); } - return Resolve(env, main_std, search); + return Resolve(env, pjson.main, search); } Maybe ResolveModule(Environment* env, @@ -594,26 +614,25 @@ Maybe ResolveModule(Environment* env, Maybe ResolveDirectory(Environment* env, const URL& search, - bool read_pkg_json) { - if (read_pkg_json) { + bool check_pjson_main) { + if (check_pjson_main) { Maybe main = ResolveMain(env, search); if (!main.IsNothing()) return main; } - return ResolveIndex(search); + return ResolveIndex(env, search); } } // anonymous namespace - Maybe Resolve(Environment* env, const std::string& specifier, const URL& base, - bool read_pkg_json) { + bool check_pjson_main) { URL pure_url(specifier); if (!(pure_url.flags() & URL_FLAGS_FAILED)) { // just check existence, without altering - Maybe check = CheckFile(pure_url); + Maybe check = CheckFile(pure_url.ToFilePath()); if (check.IsNothing()) { return Nothing(); } @@ -624,13 +643,13 @@ Maybe Resolve(Environment* env, } if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { URL resolved(specifier, base); - Maybe file = ResolveExtensions(resolved); + Maybe file = ResolveExtensions(env, resolved); if (!file.IsNothing()) return file; if (specifier.back() != '/') { resolved = URL(specifier + "/", base); } - return ResolveDirectory(env, resolved, read_pkg_json); + return ResolveDirectory(env, resolved, check_pjson_main); } else { return ResolveModule(env, specifier, base); } @@ -667,7 +686,7 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { return; } - Maybe result = node::loader::Resolve(env, specifier_std, url, true); + 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; env->ThrowError(msg.c_str()); diff --git a/src/module_wrap.h b/src/module_wrap.h index 5950c5a1be0177..95e16abf6db85a 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -12,6 +12,12 @@ namespace node { namespace loader { +struct PackageConfig { + bool exists; + bool has_main; + std::string main; +}; + v8::Maybe Resolve(Environment* env, const std::string& specifier, const url::URL& base, diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs index 771273a8d865c1..acb4486edc1288 100644 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -8,7 +8,10 @@ const builtins = new Set( ); const JS_EXTENSIONS = new Set(['.js', '.mjs']); -export function resolve(specifier, parentModuleURL/*, defaultResolve */) { +const baseURL = new url.URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve(specifier, parentModuleURL = baseURL /*, defaultResolve */) { if (builtins.has(specifier)) { return { url: specifier, diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs index 79d9774c1d787e..2173b0b503ef45 100644 --- a/test/fixtures/es-module-loaders/js-loader.mjs +++ b/test/fixtures/es-module-loaders/js-loader.mjs @@ -3,7 +3,11 @@ const builtins = new Set( Object.keys(process.binding('natives')).filter(str => /^(?!(?:internal|node|v8)\/)/.test(str)) ) -export function resolve (specifier, base) { + +const baseURL = new _url.URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve (specifier, base = baseURL) { if (builtins.has(specifier)) { return { url: specifier, diff --git a/test/fixtures/es-modules/noext b/test/fixtures/es-modules/noext new file mode 100644 index 00000000000000..f21c9bee6df46a --- /dev/null +++ b/test/fixtures/es-modules/noext @@ -0,0 +1 @@ +exports.cjs = true; \ No newline at end of file diff --git a/test/parallel/test-module-main-extension-lookup.js b/test/parallel/test-module-main-extension-lookup.js index 0a8cc47c77b2ed..6f7bc2eb1db6b2 100644 --- a/test/parallel/test-module-main-extension-lookup.js +++ b/test/parallel/test-module-main-extension-lookup.js @@ -5,3 +5,5 @@ const { execFileSync } = require('child_process'); const node = process.argv[0]; execFileSync(node, ['--experimental-modules', 'test/es-module/test-esm-ok']); +execFileSync(node, ['--experimental-modules', + 'test/fixtures/es-modules/noext']);