From c0e6e60cb18878745c433a712443f01b6c2a716d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 14 Mar 2020 16:33:17 -0700 Subject: [PATCH 01/12] tools: update minimist@1.2.5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update minimist used by lint-md.js to 1.2.5. Refs: https://app.snyk.io/vuln/SNYK-JS-MINIMIST-559764 Signed-off-by: Rich Trott PR-URL: https://github.com/nodejs/node/pull/32274 Reviewed-By: Michaël Zasso Reviewed-By: Tobias Nießen Reviewed-By: James M Snell --- tools/lint-md.js | 22 ++++++++++---- .../node-lint-md-cli-rollup/package-lock.json | 30 +++++++++---------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/tools/lint-md.js b/tools/lint-md.js index e6eecf265c5173..35df2b11fcb936 100644 --- a/tools/lint-md.js +++ b/tools/lint-md.js @@ -5962,12 +5962,21 @@ var minimist = function (args, opts) { function setKey (obj, keys, value) { var o = obj; - keys.slice(0,-1).forEach(function (key) { + for (var i = 0; i < keys.length-1; i++) { + var key = keys[i]; + if (key === '__proto__') return; if (o[key] === undefined) o[key] = {}; + if (o[key] === Object.prototype || o[key] === Number.prototype + || o[key] === String.prototype) o[key] = {}; + if (o[key] === Array.prototype) o[key] = []; o = o[key]; - }); + } var key = keys[keys.length - 1]; + if (key === '__proto__') return; + if (o === Object.prototype || o === Number.prototype + || o === String.prototype) o = {}; + if (o === Array.prototype) o = []; if (o[key] === undefined || flags.bools[key] || typeof o[key] === 'boolean') { o[key] = value; } @@ -6065,7 +6074,7 @@ var minimist = function (args, opts) { setArg(key, args[i+1], arg); i++; } - else if (args[i+1] && /true|false/.test(args[i+1])) { + else if (args[i+1] && /^(true|false)$/.test(args[i+1])) { setArg(key, args[i+1] === 'true', arg); i++; } @@ -11282,6 +11291,7 @@ function writeSync(description, options) { file.contents || '', options ); + return file } var sync$1 = { @@ -11368,7 +11378,7 @@ function write$1(description, options, callback) { if (error) { reject(error); } else { - resolve(); + resolve(file); } } } @@ -13910,7 +13920,7 @@ var chars = windows$1 ? {error: '×', warning: '‼'} : {error: '✖', warning: var trailing = /\s*$/; // Default filename. -var DEFAULT = ''; +var defaultName = ''; var noop = {open: '', close: ''}; @@ -14014,7 +14024,7 @@ function parse$4(files, options) { type: 'header', origin: origin, destination: destination, - name: origin || options.defaultName || DEFAULT, + name: origin || options.defaultName || defaultName, stored: Boolean(file.stored), moved: Boolean(file.stored && destination !== origin), stats: vfileStatistics(messages) diff --git a/tools/node-lint-md-cli-rollup/package-lock.json b/tools/node-lint-md-cli-rollup/package-lock.json index f619276709e70a..5a8728bc98a7cc 100644 --- a/tools/node-lint-md-cli-rollup/package-lock.json +++ b/tools/node-lint-md-cli-rollup/package-lock.json @@ -613,9 +613,9 @@ } }, "minimist": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", - "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=" + "version": "1.2.5", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", + "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==" }, "ms": { "version": "2.1.2", @@ -1516,9 +1516,9 @@ } }, "to-vfile": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/to-vfile/-/to-vfile-6.0.0.tgz", - "integrity": "sha512-i9fwXXSsHLu7mzgixc1WjgnqSe6pGpjnzCYoFmrASvEueLfyKf09QAe+XQYu8OAJ62aFqHpe2EKXojeRVvEzqA==", + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/to-vfile/-/to-vfile-6.1.0.tgz", + "integrity": "sha512-BxX8EkCxOAZe+D/ToHdDsJcVI4HqQfmw0tCkp31zf3dNP/XWIAjU4CmeuSwsSoOzOTqHPOL0KUzyZqJplkD0Qw==", "requires": { "is-buffer": "^2.0.0", "vfile": "^4.0.0" @@ -1720,9 +1720,9 @@ } }, "vfile-reporter": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/vfile-reporter/-/vfile-reporter-6.0.0.tgz", - "integrity": "sha512-8Is0XxFxWJUhPJdOg3CyZTqd3ICCWg6r304PuBl818ZG91h4FMS3Q+lrOPS+cs5/DZK3H0+AkJdH0J8JEwKtDA==", + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/vfile-reporter/-/vfile-reporter-6.0.1.tgz", + "integrity": "sha512-0OppK9mo8G2XUpv+hIKLVSDsoxJrXnOy73+vIm0jQUOUFYRduqpFHX+QqAQfvRHyX9B0UFiRuNJnBOjQCIsw1g==", "requires": { "repeat-string": "^1.5.0", "string-width": "^4.0.0", @@ -1743,14 +1743,14 @@ } }, "vfile-sort": { - "version": "2.2.1", - "resolved": "https://registry.npmjs.org/vfile-sort/-/vfile-sort-2.2.1.tgz", - "integrity": "sha512-5dt7xEhC44h0uRQKhbM2JAe0z/naHphIZlMOygtMBM9Nn0pZdaX5fshhwWit9wvsuP8t/wp43nTDRRErO1WK8g==" + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/vfile-sort/-/vfile-sort-2.2.2.tgz", + "integrity": "sha512-tAyUqD2R1l/7Rn7ixdGkhXLD3zsg+XLAeUDUhXearjfIcpL1Hcsj5hHpCoy/gvfK/Ws61+e972fm0F7up7hfYA==" }, "vfile-statistics": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/vfile-statistics/-/vfile-statistics-1.1.3.tgz", - "integrity": "sha512-CstaK/ebTz1W3Qp41Bt9Lj/2DmumFsCwC2sKahDNSPh0mPh7/UyMLCoU8ZBX34CRU0d61B4W41yIFsV0NKMZeA==" + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/vfile-statistics/-/vfile-statistics-1.1.4.tgz", + "integrity": "sha512-lXhElVO0Rq3frgPvFBwahmed3X03vjPF8OcjKMy8+F1xU/3Q3QU3tKEDp743SFtb74PdF0UWpxPvtOP0GCLheA==" }, "wrapped": { "version": "1.0.1", From a1a282ff0a1849a476947de708f54c138d04a71c Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Thu, 19 Dec 2019 16:25:52 -0500 Subject: [PATCH 02/12] esm: import.meta.resolve with nodejs: builtins PR-URL: https://github.com/nodejs/node/pull/31032 Reviewed-By: Jan Krems Reviewed-By: Myles Borins --- doc/api/cli.md | 8 +++ doc/api/esm.md | 50 ++++++++++++++++--- doc/node.1 | 3 ++ lib/internal/modules/esm/get_format.js | 4 +- lib/internal/modules/esm/loader.js | 10 ++-- lib/internal/modules/esm/resolve.js | 10 ++-- lib/internal/modules/esm/translators.js | 33 ++++++++---- src/module_wrap.cc | 15 ++++-- src/node_options.cc | 8 +++ src/node_options.h | 1 + test/es-module/test-esm-dynamic-import.js | 5 +- .../test-esm-import-meta-resolve.mjs | 24 +++++++++ .../es-module-loaders/example-loader.mjs | 4 +- .../loader-unknown-builtin-module.mjs | 4 +- .../not-found-assert-loader.mjs | 3 +- 15 files changed, 145 insertions(+), 37 deletions(-) create mode 100644 test/es-module/test-esm-import-meta-resolve.mjs diff --git a/doc/api/cli.md b/doc/api/cli.md index 6ba36e1dfc3a45..5b48e2c5e5a2d3 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -156,6 +156,13 @@ Enable experimental Source Map V3 support for stack traces. Currently, overriding `Error.prepareStackTrace` is ignored when the `--enable-source-maps` flag is set. +### `--experimental-import-meta-resolve` + + +Enable experimental `import.meta.resolve()` support. + ### `--experimental-json-modules` * `--enable-fips` * `--enable-source-maps` +* `--experimental-import-meta-resolve` * `--experimental-json-modules` * `--experimental-loader` * `--experimental-modules` diff --git a/doc/api/esm.md b/doc/api/esm.md index 5728b359a4b143..f85cff8b8855eb 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -837,6 +837,32 @@ const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); ``` +### No `require.resolve` + +Former use cases relying on `require.resolve` to determine the resolved path +of a module can be supported via `import.meta.resolve`, which is experimental +and supported via the `--experimental-import-meta-resolve` flag: + +```js +(async () => { + const dependencyAsset = await import.meta.resolve('component-lib/asset.css'); +})(); +``` + +`import.meta.resolve` also accepts a second argument which is the parent module +from which to resolve from: + +```js +(async () => { + // Equivalent to import.meta.resolve('./dep') + await import.meta.resolve('./dep', import.meta.url); +})(); +``` + +This function is asynchronous since the ES module resolver in Node.js is +asynchronous. With the introduction of [Top-Level Await][], these use cases +will be easier as they won't require an async function wrapper. + ### No `require.extensions` `require.extensions` is not used by `import`. The expectation is that loader @@ -1405,13 +1431,14 @@ The resolver has the following properties: The algorithm to load an ES module specifier is given through the **ESM_RESOLVE** method below. It returns the resolved URL for a -module specifier relative to a parentURL, in addition to the unique module -format for that resolved URL given by the **ESM_FORMAT** routine. +module specifier relative to a parentURL. -The _"module"_ format is returned for an ECMAScript Module, while the -_"commonjs"_ format is used to indicate loading through the legacy -CommonJS loader. Additional formats such as _"addon"_ can be extended in future -updates. +The algorithm to determine the module format of a resolved URL is +provided by **ESM_FORMAT**, which returns the unique module +format for any file. The _"module"_ format is returned for an ECMAScript +Module, while the _"commonjs"_ format is used to indicate loading through the +legacy CommonJS loader. Additional formats such as _"addon"_ can be extended in +future updates. In the following algorithms, all subroutine errors are propagated as errors of these top-level routines unless stated otherwise. @@ -1440,11 +1467,13 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If _resolvedURL_ contains any percent encodings of _"/"_ or _"\\"_ (_"%2f"_ > and _"%5C"_ respectively), then > 1. Throw an _Invalid Specifier_ error. -> 1. If the file at _resolvedURL_ does not exist, then +> 1. If _resolvedURL_ does not end with a trailing _"/"_ and the file at +> _resolvedURL_ does not exist, then > 1. Throw a _Module Not Found_ error. > 1. Set _resolvedURL_ to the real path of _resolvedURL_. > 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_). > 1. Load _resolvedURL_ as module format, _format_. +> 1. Return _resolvedURL_. **PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_) @@ -1472,7 +1501,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If _selfUrl_ isn't empty, return _selfUrl_. > 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin > module, then -> 1. Return the string _"node:"_ concatenated with _packageSpecifier_. +> 1. Return the string _"nodejs:"_ concatenated with _packageSpecifier_. > 1. While _parentURL_ is not the file system root, > 1. Let _packageURL_ be the URL resolution of _"node_modules/"_ > concatenated with _packageSpecifier_, relative to _parentURL_. @@ -1481,6 +1510,8 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Set _parentURL_ to the parent URL path of _parentURL_. > 1. Continue the next loop iteration. > 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_). +> 1. If _packageSubpath_ is equal to _"./"_, then +> 1. Return _packageURL_ + _"/"_. > 1. If _packageSubpath_ is _undefined__, then > 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, > _pjson_). @@ -1502,6 +1533,8 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If _pjson_ does not include an _"exports"_ property, then > 1. Return **undefined**. > 1. If _pjson.name_ is equal to _packageName_, then +> 1. If _packageSubpath_ is equal to _"./"_, then +> 1. Return _packageURL_ + _"/"_. > 1. If _packageSubpath_ is _undefined_, then > 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_). > 1. Otherwise, @@ -1680,3 +1713,4 @@ success! [the official standard format]: https://tc39.github.io/ecma262/#sec-modules [transpiler loader example]: #esm_transpiler_loader [6.1.7 Array Index]: https://tc39.es/ecma262/#integer-index +[Top-Level Await]: https://github.com/tc39/proposal-top-level-await diff --git a/doc/node.1 b/doc/node.1 index 46eddf69e876a7..ad82f466f86d45 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -113,6 +113,9 @@ Requires Node.js to be built with .It Fl -enable-source-maps Enable experimental Source Map V3 support for stack traces. . +.It Fl -experimental-import-meta-resolve +Enable experimental ES modules support for import.meta.resolve(). +. .It Fl -experimental-json-modules Enable experimental JSON interop support for the ES Module loader. . diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index 69ba2398129908..9815077c3a6dcb 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -1,6 +1,5 @@ 'use strict'; -const { NativeModule } = require('internal/bootstrap/loaders'); const { extname } = require('path'); const { getOptionValue } = require('internal/options'); @@ -39,7 +38,7 @@ if (experimentalJsonModules) extensionFormatMap['.json'] = legacyExtensionFormatMap['.json'] = 'json'; function defaultGetFormat(url, context, defaultGetFormat) { - if (NativeModule.canBeRequiredByUsers(url)) { + if (url.startsWith('nodejs:')) { return { format: 'builtin' }; } const parsed = new URL(url); @@ -73,5 +72,6 @@ function defaultGetFormat(url, context, defaultGetFormat) { } return { format: format || null }; } + return { format: null }; } exports.defaultGetFormat = defaultGetFormat; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 6d9b267ffe5d67..5a27f4be7c9d1a 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -94,7 +94,10 @@ class Loader { throw new ERR_INVALID_RETURN_PROPERTY_VALUE( 'string', 'loader resolve', 'url', url); } + return url; + } + async getFormat(url) { const getFormatResponse = await this._getFormat( url, {}, defaultGetFormat); if (typeof getFormatResponse !== 'object') { @@ -109,7 +112,7 @@ class Loader { } if (format === 'builtin') { - return { url: `node:${url}`, format }; + return format; } if (this._resolve !== defaultResolve) { @@ -132,7 +135,7 @@ class Loader { ); } - return { url, format }; + return format; } async eval( @@ -185,7 +188,8 @@ class Loader { } async getModuleJob(specifier, parentURL) { - const { url, format } = await this.resolve(specifier, parentURL); + const url = await this.resolve(specifier, parentURL); + const format = await this.getFormat(url); let job = this.moduleMap.get(url); // CommonJS will set functions for lazy job evaluation. if (typeof job === 'function') diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index f1045871dddb72..ec2e681e621d0d 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -8,6 +8,7 @@ const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { realpathSync } = require('fs'); const { getOptionValue } = require('internal/options'); +const { sep } = require('path'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); @@ -29,11 +30,13 @@ function defaultResolve(specifier, { parentURL } = {}, defaultResolve) { }; } } catch {} + if (parsed && parsed.protocol === 'nodejs:') + return { url: specifier }; if (parsed && parsed.protocol !== 'file:' && parsed.protocol !== 'data:') throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(); if (NativeModule.canBeRequiredByUsers(specifier)) { return { - url: specifier + url: 'nodejs:' + specifier }; } if (parentURL && parentURL.startsWith('data:')) { @@ -58,11 +61,12 @@ function defaultResolve(specifier, { parentURL } = {}, defaultResolve) { let url = moduleWrapResolve(specifier, parentURL); if (isMain ? !preserveSymlinksMain : !preserveSymlinks) { - const real = realpathSync(fileURLToPath(url), { + const urlPath = fileURLToPath(url); + const real = realpathSync(urlPath, { [internalFS.realpathCacheKey]: realpathCache }); const old = url; - url = pathToFileURL(real); + url = pathToFileURL(real + (urlPath.endsWith(sep) ? '/' : '')); url.search = old.search; url.hash = old.hash; } diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 9f3bcfb8e7db9d..497f90ed94475b 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -28,6 +28,9 @@ const { ERR_UNKNOWN_BUILTIN_MODULE } = require('internal/errors').codes; const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache'); const moduleWrap = internalBinding('module_wrap'); const { ModuleWrap } = moduleWrap; +const { getOptionValue } = require('internal/options'); +const experimentalImportMetaResolve = + getOptionValue('--experimental-import-meta-resolve'); const debug = debuglog('esm'); @@ -42,16 +45,28 @@ function errPath(url) { return url; } -function initializeImportMeta(meta, { url }) { - meta.url = url; -} - let esmLoader; async function importModuleDynamically(specifier, { url }) { if (!esmLoader) { - esmLoader = require('internal/process/esm_loader'); + esmLoader = require('internal/process/esm_loader').ESMLoader; } - return esmLoader.ESMLoader.import(specifier, url); + return esmLoader.import(specifier, url); +} + +function createImportMetaResolve(defaultParentUrl) { + return async function resolve(specifier, parentUrl = defaultParentUrl) { + if (!esmLoader) { + esmLoader = require('internal/process/esm_loader').ESMLoader; + } + return esmLoader.resolve(specifier, parentUrl); + }; +} + +function initializeImportMeta(meta, { url }) { + // Alphabetical + if (experimentalImportMetaResolve) + meta.resolve = createImportMetaResolve(url); + meta.url = url; } // Strategy for loading a standard JavaScript module @@ -104,10 +119,10 @@ translators.set('commonjs', function commonjsStrategy(url, isMain) { // through normal resolution translators.set('builtin', async function builtinStrategy(url) { debug(`Translating BuiltinModule ${url}`); - // Slice 'node:' scheme - const id = url.slice(5); + // Slice 'nodejs:' scheme + const id = url.slice(7); const module = loadNativeModule(id, url, true); - if (!module) { + if (!url.startsWith('nodejs:') || !module) { throw new ERR_UNKNOWN_BUILTIN_MODULE(id); } debug(`Loading BuiltinModule ${url}`); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 8fb04431a7ef4c..304d0d04e6095b 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -812,6 +812,10 @@ Maybe FinalizeResolution(Environment* env, return Nothing(); } + if (resolved.path().back() == '/') { + return Just(resolved); + } + const std::string& path = resolved.ToFilePath(); if (CheckDescriptorAtPath(path) != FILE) { std::string msg = "Cannot find module " + @@ -1197,7 +1201,9 @@ Maybe ResolveSelf(Environment* env, } if (!found_pjson || pcfg->name != pkg_name) return Nothing(); if (pcfg->exports.IsEmpty()) return Nothing(); - if (!pkg_subpath.length()) { + if (pkg_subpath == "./") { + return Just(URL("./", pjson_url)); + } else if (!pkg_subpath.length()) { return PackageMainResolve(env, pjson_url, *pcfg, base); } else { return PackageExportsResolve(env, pjson_url, pkg_subpath, *pcfg, base); @@ -1241,8 +1247,7 @@ Maybe PackageResolve(Environment* env, return Nothing(); } std::string pkg_subpath; - if ((sep_index == std::string::npos || - sep_index == specifier.length() - 1)) { + if (sep_index == std::string::npos) { pkg_subpath = ""; } else { pkg_subpath = "." + specifier.substr(sep_index); @@ -1273,7 +1278,9 @@ Maybe PackageResolve(Environment* env, Maybe pcfg = GetPackageConfig(env, pjson_path, base); // Invalid package configuration error. if (pcfg.IsNothing()) return Nothing(); - if (!pkg_subpath.length()) { + if (pkg_subpath == "./") { + return Just(URL("./", pjson_url)); + } else if (!pkg_subpath.length()) { return PackageMainResolve(env, pjson_url, *pcfg.FromJust(), base); } else { if (!pcfg.FromJust()->exports.IsEmpty()) { diff --git a/src/node_options.cc b/src/node_options.cc index 2a4c20d19da463..695935e2774a91 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -116,6 +116,10 @@ void PerIsolateOptions::CheckOptions(std::vector* errors) { } void EnvironmentOptions::CheckOptions(std::vector* errors) { + if (experimental_import_meta_resolve && !experimental_modules) { + errors->push_back("--experimental-meta-resolve requires " + "--experimental-modules be enabled"); + } if (!userland_loader.empty() && !experimental_modules) { errors->push_back("--experimental-loader requires " "--experimental-modules be enabled"); @@ -360,6 +364,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "experimental ES Module support for webassembly modules", &EnvironmentOptions::experimental_wasm_modules, kAllowedInEnvironment); + AddOption("--experimental-import-meta-resolve", + "experimental ES Module import.meta.resolve() support", + &EnvironmentOptions::experimental_import_meta_resolve, + kAllowedInEnvironment); AddOption("--experimental-policy", "use the specified file as a " "security policy", diff --git a/src/node_options.h b/src/node_options.h index 13f636e35742f6..571ac305f9990c 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -106,6 +106,7 @@ class EnvironmentOptions : public Options { std::string experimental_specifier_resolution; std::string es_module_specifier_resolution; bool experimental_wasm_modules = false; + bool experimental_import_meta_resolve = false; std::string module_type; std::string experimental_policy; std::string experimental_policy_integrity; diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js index d80daf878411fd..a08e66dad3f820 100644 --- a/test/es-module/test-esm-dynamic-import.js +++ b/test/es-module/test-esm-dynamic-import.js @@ -54,11 +54,12 @@ function expectFsNamespace(result) { expectFsNamespace(import('fs')); expectFsNamespace(eval('import("fs")')); expectFsNamespace(eval('import("fs")')); + expectFsNamespace(import('nodejs:fs')); + expectModuleError(import('nodejs:unknown'), + 'ERR_UNKNOWN_BUILTIN_MODULE'); expectModuleError(import('./not-an-existing-module.mjs'), 'ERR_MODULE_NOT_FOUND'); - expectModuleError(import('node:fs'), - 'ERR_UNSUPPORTED_ESM_URL_SCHEME'); expectModuleError(import('http://example.com/foo.js'), 'ERR_UNSUPPORTED_ESM_URL_SCHEME'); })(); diff --git a/test/es-module/test-esm-import-meta-resolve.mjs b/test/es-module/test-esm-import-meta-resolve.mjs new file mode 100644 index 00000000000000..3d9dae48676c50 --- /dev/null +++ b/test/es-module/test-esm-import-meta-resolve.mjs @@ -0,0 +1,24 @@ +// Flags: --experimental-modules --experimental-import-meta-resolve +import '../common/index.mjs'; +import assert from 'assert'; + +const dirname = import.meta.url.slice(0, import.meta.url.lastIndexOf('/') + 1); +const fixtures = dirname.slice(0, dirname.lastIndexOf('/', dirname.length - 2) + + 1) + 'fixtures/'; + +(async () => { + assert.strictEqual(await import.meta.resolve('./test-esm-import-meta.mjs'), + dirname + 'test-esm-import-meta.mjs'); + try { + await import.meta.resolve('./notfound.mjs'); + assert.fail(); + } catch (e) { + assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND'); + } + assert.strictEqual( + await import.meta.resolve('../fixtures/empty-with-bom.txt'), + fixtures + 'empty-with-bom.txt'); + assert.strictEqual(await import.meta.resolve('../fixtures/'), fixtures); + assert.strictEqual(await import.meta.resolve('baz/', fixtures), + fixtures + 'node_modules/baz/'); +})(); diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs index 70f9f28f08e742..1ed18bda51070d 100644 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -11,7 +11,7 @@ baseURL.pathname = process.cwd() + '/'; export function resolve(specifier, { parentURL = baseURL }, defaultResolve) { if (builtinModules.includes(specifier)) { return { - url: specifier + url: 'nodejs:' + specifier }; } if (/^\.{1,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) { @@ -27,7 +27,7 @@ export function resolve(specifier, { parentURL = baseURL }, defaultResolve) { } export function getFormat(url, context, defaultGetFormat) { - if (builtinModules.includes(url)) { + if (url.startsWith('nodejs:') && builtinModules.includes(url.slice(7))) { return { format: 'builtin' }; diff --git a/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs b/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs index 1a48231966ce5b..e976343e47e9bc 100644 --- a/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs +++ b/test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs @@ -1,14 +1,14 @@ export async function resolve(specifier, { parentURL }, defaultResolve) { if (specifier === 'unknown-builtin-module') { return { - url: 'unknown-builtin-module' + url: 'nodejs:unknown-builtin-module' }; } return defaultResolve(specifier, {parentURL}, defaultResolve); } export async function getFormat(url, context, defaultGetFormat) { - if (url === 'unknown-builtin-module') { + if (url === 'nodejs:unknown-builtin-module') { return { format: 'builtin' }; diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs index 7b1d176e4537f6..2130bad5f52698 100644 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -14,8 +14,7 @@ export async function resolve(specifier, { parentURL }, defaultResolve) { catch (e) { assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND'); return { - format: 'builtin', - url: 'fs' + url: 'nodejs:fs' }; } assert.fail(`Module resolution for ${specifier} should be throw ERR_MODULE_NOT_FOUND`); From 37aee30e3c4822b2dcd1d751183052f3ffe62f42 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 3 Feb 2020 13:31:12 +0200 Subject: [PATCH 03/12] module: package "exports" error refinements PR-URL: https://github.com/nodejs/node/pull/31625 Reviewed-By: Jan Krems --- doc/api/errors.md | 20 ++ doc/api/esm.md | 75 ++--- lib/internal/errors.js | 29 +- lib/internal/modules/cjs/loader.js | 104 ++++--- src/module_wrap.cc | 258 ++++++++++-------- src/node_errors.h | 4 +- test/es-module/test-esm-exports.mjs | 54 ++-- .../node_modules/pkgexports/package.json | 4 + .../pkgexports/resolve-self-invalid.js | 1 + .../pkgexports/resolve-self-invalid.mjs | 1 + 10 files changed, 331 insertions(+), 219 deletions(-) create mode 100644 test/fixtures/node_modules/pkgexports/resolve-self-invalid.js create mode 100644 test/fixtures/node_modules/pkgexports/resolve-self-invalid.mjs diff --git a/doc/api/errors.md b/doc/api/errors.md index 8de1b49e260ea1..ebc5a2e51d5b1b 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1301,6 +1301,12 @@ An invalid HTTP token was supplied. An IP address is not valid. + +### `ERR_INVALID_MODULE_SPECIFIER` + +The imported module string is an invalid URL, package name, or package subpath +specifier. + ### `ERR_INVALID_OPT_VALUE` @@ -1316,6 +1322,12 @@ An invalid or unknown file encoding was passed. An invalid `package.json` file was found which failed parsing. + +### `ERR_INVALID_PACKAGE_TARGET` + +The `package.json` [exports][] field contains an invalid target mapping value +for the attempted module resolution. + ### `ERR_INVALID_PERFORMANCE_MARK` @@ -1616,6 +1628,13 @@ A non-context-aware native addon was loaded in a process that disallows them. A given value is out of the accepted range. + +### `ERR_PACKAGE_PATH_NOT_EXPORTED` + +The `package.json` [exports][] field does not export the requested subpath. +Because exports are encapsulated, private internal modules that are not exported +cannot be imported through the package resolution, unless using an absolute URL. + ### `ERR_REQUIRE_ESM` @@ -2457,6 +2476,7 @@ such as `process.stdout.on('data')`. [crypto digest algorithm]: crypto.html#crypto_crypto_gethashes [domains]: domain.html [event emitter-based]: events.html#events_class_eventemitter +[exports]: esm.html#esm_package_exports [file descriptors]: https://en.wikipedia.org/wiki/File_descriptor [policy]: policy.html [stream-based]: stream.html diff --git a/doc/api/esm.md b/doc/api/esm.md index f85cff8b8855eb..0ca5dae5eadbff 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1446,6 +1446,17 @@ of these top-level routines unless stated otherwise. _defaultEnv_ is the conditional environment name priority array, `["node", "import"]`. +The resolver can throw the following errors: +* _Invalid Module Specifier_: Module specifier is an invalid URL, package name + or package subpath specifier. +* _Invalid Package Configuration_: package.json configuration is invalid or + contains an invalid configuration. +* _Invalid Package Target_: Package exports define a target module within the + package that is an invalid type or string target. +* _Package Path Not Exported_: Package exports do not define or permit a target + subpath in the package for the given module. +* _Module Not Found_: The package or module requested does not exist. +
Resolver algorithm specification @@ -1456,7 +1467,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Set _resolvedURL_ to the result of parsing and reserializing > _specifier_ as a URL. > 1. Otherwise, if _specifier_ starts with _"/"_, then -> 1. Throw an _Invalid Specifier_ error. +> 1. Throw an _Invalid Module Specifier_ error. > 1. Otherwise, if _specifier_ starts with _"./"_ or _"../"_, then > 1. Set _resolvedURL_ to the URL resolution of _specifier_ relative to > _parentURL_. @@ -1466,7 +1477,7 @@ _defaultEnv_ is the conditional environment name priority array, > **PACKAGE_RESOLVE**(_specifier_, _parentURL_). > 1. If _resolvedURL_ contains any percent encodings of _"/"_ or _"\\"_ (_"%2f"_ > and _"%5C"_ respectively), then -> 1. Throw an _Invalid Specifier_ error. +> 1. Throw an _Invalid Module Specifier_ error. > 1. If _resolvedURL_ does not end with a trailing _"/"_ and the file at > _resolvedURL_ does not exist, then > 1. Throw a _Module Not Found_ error. @@ -1480,14 +1491,14 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Let _packageName_ be *undefined*. > 1. Let _packageSubpath_ be *undefined*. > 1. If _packageSpecifier_ is an empty string, then -> 1. Throw an _Invalid Specifier_ error. +> 1. Throw an _Invalid Module Specifier_ error. > 1. Otherwise, > 1. If _packageSpecifier_ does not contain a _"/"_ separator, then -> 1. Throw an _Invalid Specifier_ error. +> 1. Throw an _Invalid Module Specifier_ error. > 1. Set _packageName_ to the substring of _packageSpecifier_ > until the second _"/"_ separator or the end of the string. > 1. If _packageName_ starts with _"."_ or contains _"\\"_ or _"%"_, then -> 1. Throw an _Invalid Specifier_ error. +> 1. Throw an _Invalid Module Specifier_ error. > 1. Let _packageSubpath_ be _undefined_. > 1. If the length of _packageSpecifier_ is greater than the length of > _packageName_, then @@ -1495,7 +1506,7 @@ _defaultEnv_ is the conditional environment name priority array, > _packageSpecifier_ from the position at the length of _packageName_. > 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent > encoded strings for _"/"_ or _"\\"_, then -> 1. Throw an _Invalid Specifier_ error. +> 1. Throw an _Invalid Module Specifier_ error. > 1. Set _selfUrl_ to the result of > **SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_). > 1. If _selfUrl_ isn't empty, return _selfUrl_. @@ -1552,7 +1563,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Throw a _Module Not Found_ error. > 1. If _pjson.exports_ is not **null** or **undefined**, then > 1. If _exports_ is an Object with both a key starting with _"."_ and a key -> not starting with _"."_, throw an "Invalid Package Configuration" error. +> not starting with _"."_, throw an _Invalid Package Configuration_ error. > 1. If _pjson.exports_ is a String or Array, or an Object containing no > keys starting with _"."_, then > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, @@ -1561,6 +1572,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Let _mainExport_ be the _"."_ property in _pjson.exports_. > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, > _mainExport_, _""_). +> 1. Throw a _Package Path Not Exported_ error. > 1. If _pjson.main_ is a String, then > 1. Let _resolvedMain_ be the URL resolution of _packageURL_, "/", and > _pjson.main_. @@ -1575,7 +1587,7 @@ _defaultEnv_ is the conditional environment name priority array, **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _packagePath_, _exports_) > 1. If _exports_ is an Object with both a key starting with _"."_ and a key not -> starting with _"."_, throw an "Invalid Package Configuration" error. +> starting with _"."_, throw an _Invalid Package Configuration_ error. > 1. If _exports_ is an Object and all keys of _exports_ start with _"."_, then > 1. Set _packagePath_ to _"./"_ concatenated with _packagePath_. > 1. If _packagePath_ is a key of _exports_, then @@ -1591,43 +1603,44 @@ _defaultEnv_ is the conditional environment name priority array, > of the length of _directory_. > 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, > _subpath_, _defaultEnv_). -> 1. Throw a _Module Not Found_ error. +> 1. Throw a _Package Path Not Exported_ error. **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_, _env_) -> 1. If _target_ is a String, then -> 1. If _target_ does not start with _"./"_, throw a _Module Not Found_ -> error. -> 1. If _subpath_ has non-zero length and _target_ does not end with _"/"_, -> throw a _Module Not Found_ error. -> 1. If _target_ or _subpath_ contain any _"node_modules"_ segments including -> _"node_modules"_ percent-encoding, throw a _Module Not Found_ error. +> 1.If _target_ is a String, then +> 1. If _target_ does not start with _"./"_ or contains any _"node_modules"_ +> segments including _"node_modules"_ percent-encoding, throw an +> _Invalid Package Target_ error. > 1. Let _resolvedTarget_ be the URL resolution of the concatenation of > _packageURL_ and _target_. -> 1. If _resolvedTarget_ is contained in _packageURL_, then -> 1. Let _resolved_ be the URL resolution of the concatenation of -> _subpath_ and _resolvedTarget_. -> 1. If _resolved_ is contained in _resolvedTarget_, then -> 1. Return _resolved_. +> 1. If _resolvedTarget_ is not contained in _packageURL_, throw an +> _Invalid Package Target_ error. +> 1. If _subpath_ has non-zero length and _target_ does not end with _"/"_, +> throw an _Invalid Module Specifier_ error. +> 1. Let _resolved_ be the URL resolution of the concatenation of +> _subpath_ and _resolvedTarget_. +> 1. If _resolved_ is not contained in _resolvedTarget_, throw an +> _Invalid Module Specifier_ error. +> 1. Return _resolved_. > 1. Otherwise, if _target_ is a non-null Object, then > 1. If _exports_ contains any index property keys, as defined in ECMA-262 > [6.1.7 Array Index][], throw an _Invalid Package Configuration_ error. > 1. For each property _p_ of _target_, in object insertion order as, > 1. If _env_ contains an entry for _p_, then > 1. Let _targetValue_ be the value of the _p_ property in _target_. -> 1. Let _resolved_ be the result of **PACKAGE_EXPORTS_TARGET_RESOLVE** -> (_packageURL_, _targetValue_, _subpath_, _env_). -> 1. Assert: _resolved_ is a String. -> 1. Return _resolved_. +> 1. Return the result of **PACKAGE_EXPORTS_TARGET_RESOLVE**( +> _packageURL_, _targetValue_, _subpath_, _env_), continuing the +> loop on any _Package Path Not Exported_ error. +> 1. Throw a _Package Path Not Exported_ error. > 1. Otherwise, if _target_ is an Array, then +> 1. If _target.length is zero, throw an _Invalid Package Target_ error. > 1. For each item _targetValue_ in _target_, do > 1. If _targetValue_ is an Array, continue the loop. -> 1. Let _resolved_ be the result of -> **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _targetValue_, -> _subpath_, _env_), continuing the loop on abrupt completion. -> 1. Assert: _resolved_ is a String. -> 1. Return _resolved_. -> 1. Throw a _Module Not Found_ error. +> 1. Return the result of **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, +> _targetValue_, _subpath_, _env_), continuing the loop on any +> _Package Path Not Exported_ or _Invalid Package Target_ error. +> 1. Throw the last fallback resolution error. +> 1. Otherwise throw an _Invalid Package Target_ error. **ESM_FORMAT**(_url_) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7611e461ad69f7..b69ab5658d27f6 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -13,16 +13,20 @@ const { ArrayIsArray, Error, + JSONStringify, Map, MathAbs, NumberIsInteger, ObjectDefineProperty, ObjectKeys, + StringPrototypeSlice, Symbol, SymbolFor, WeakMap, } = primordials; +const sep = process.platform === 'win32' ? '\\' : '/'; + const messages = new Map(); const codes = {}; @@ -1065,6 +1069,11 @@ E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s', TypeError); E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent', TypeError); E('ERR_INVALID_HTTP_TOKEN', '%s must be a valid HTTP token ["%s"]', TypeError); E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s', TypeError); +E('ERR_INVALID_MODULE_SPECIFIER', (pkgPath, subpath) => { + assert(subpath !== '.'); + return `Package subpath '${subpath}' is not a valid module request for the ` + + `"exports" resolution of ${pkgPath}${sep}package.json`; +}, TypeError); E('ERR_INVALID_OPT_VALUE', (name, value) => `The value "${String(value)}" is invalid for option "${name}"`, TypeError, @@ -1072,7 +1081,17 @@ E('ERR_INVALID_OPT_VALUE', (name, value) => E('ERR_INVALID_OPT_VALUE_ENCODING', 'The value "%s" is invalid for option "encoding"', TypeError); E('ERR_INVALID_PACKAGE_CONFIG', - 'Invalid package config for \'%s\', %s', Error); + `Invalid package config %s${sep}package.json, %s`, Error); +E('ERR_INVALID_PACKAGE_TARGET', (pkgPath, key, subpath, target) => { + if (key === '.') { + return `Invalid "exports" main target ${JSONStringify(target)} defined ` + + `in the package config ${pkgPath}${sep}package.json`; + } else { + return `Invalid "exports" target ${JSONStringify(target)} defined for '${ + StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + + `package config ${pkgPath}${sep}package.json`; + } +}, Error); E('ERR_INVALID_PERFORMANCE_MARK', 'The "%s" performance mark has not been set', Error); E('ERR_INVALID_PROTOCOL', @@ -1216,6 +1235,14 @@ E('ERR_OUT_OF_RANGE', msg += ` It must be ${range}. Received ${received}`; return msg; }, RangeError); +E('ERR_PACKAGE_PATH_NOT_EXPORTED', (pkgPath, subpath) => { + if (subpath === '.') { + return `No "exports" main resolved in ${pkgPath}${sep}package.json`; + } else { + return `Package subpath '${subpath}' is not defined by "exports" in ${ + pkgPath}${sep}package.json`; + } +}, Error); E('ERR_REQUIRE_ESM', (filename, parentPath = null, packageJsonPath = null) => { let msg = `Must use import to load ES Module: ${filename}`; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index ed433e1b8744ea..734dffa343a522 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -83,6 +83,9 @@ const { ERR_INVALID_ARG_VALUE, ERR_INVALID_OPT_VALUE, ERR_INVALID_PACKAGE_CONFIG, + ERR_INVALID_PACKAGE_TARGET, + ERR_INVALID_MODULE_SPECIFIER, + ERR_PACKAGE_PATH_NOT_EXPORTED, ERR_REQUIRE_ESM } = require('internal/errors').codes; const { validateString } = require('internal/validators'); @@ -500,13 +503,9 @@ function applyExports(basePath, expansion) { if (ObjectPrototypeHasOwnProperty(pkgExports, mappingKey)) { const mapping = pkgExports[mappingKey]; return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, '', - basePath, mappingKey); + mappingKey); } - // Fallback to CJS main lookup when no main export is defined - if (mappingKey === '.') - return basePath; - let dirMatch = ''; for (const candidateKey of ObjectKeys(pkgExports)) { if (candidateKey[candidateKey.length - 1] !== '/') continue; @@ -520,18 +519,11 @@ function applyExports(basePath, expansion) { const mapping = pkgExports[dirMatch]; const subpath = StringPrototypeSlice(mappingKey, dirMatch.length); return resolveExportsTarget(pathToFileURL(basePath + '/'), mapping, - subpath, basePath, mappingKey); + subpath, mappingKey); } } - // Fallback to CJS main lookup when no main export is defined - if (mappingKey === '.') - return basePath; - // eslint-disable-next-line no-restricted-syntax - const e = new Error(`Package exports for '${basePath}' do not define ` + - `a '${mappingKey}' subpath`); - e.code = 'MODULE_NOT_FOUND'; - throw e; + throw new ERR_PACKAGE_PATH_NOT_EXPORTED(basePath, mappingKey); } // This only applies to requests of a specific form: @@ -566,39 +558,53 @@ function isArrayIndex(p) { return n >= 0 && n < (2 ** 32) - 1; } -function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { +function resolveExportsTarget(baseUrl, target, subpath, mappingKey) { if (typeof target === 'string') { - if (target.startsWith('./') && - (subpath.length === 0 || target.endsWith('/'))) { - const resolvedTarget = new URL(target, pkgPath); - const pkgPathPath = pkgPath.pathname; - const resolvedTargetPath = resolvedTarget.pathname; - if (StringPrototypeStartsWith(resolvedTargetPath, pkgPathPath) && + let resolvedTarget, resolvedTargetPath; + const pkgPathPath = baseUrl.pathname; + if (StringPrototypeStartsWith(target, './')) { + resolvedTarget = new URL(target, baseUrl); + resolvedTargetPath = resolvedTarget.pathname; + if (!StringPrototypeStartsWith(resolvedTargetPath, pkgPathPath) || StringPrototypeIndexOf(resolvedTargetPath, '/node_modules/', - pkgPathPath.length - 1) === -1) { - const resolved = new URL(subpath, resolvedTarget); - const resolvedPath = resolved.pathname; - if (StringPrototypeStartsWith(resolvedPath, resolvedTargetPath) && - StringPrototypeIndexOf(resolvedPath, '/node_modules/', - pkgPathPath.length - 1) === -1) { - return fileURLToPath(resolved); - } - } + pkgPathPath.length - 1) !== -1) + resolvedTarget = undefined; } + if (subpath.length > 0 && target[target.length - 1] !== '/') + resolvedTarget = undefined; + if (resolvedTarget === undefined) + throw new ERR_INVALID_PACKAGE_TARGET(StringPrototypeSlice(baseUrl.pathname + , 0, -1), mappingKey, subpath, target); + const resolved = new URL(subpath, resolvedTarget); + const resolvedPath = resolved.pathname; + if (StringPrototypeStartsWith(resolvedPath, resolvedTargetPath) && + StringPrototypeIndexOf(resolvedPath, '/node_modules/', + pkgPathPath.length - 1) === -1) { + return fileURLToPath(resolved); + } + throw new ERR_INVALID_MODULE_SPECIFIER(StringPrototypeSlice(baseUrl.pathname + , 0, -1), mappingKey); } else if (ArrayIsArray(target)) { + if (target.length === 0) + throw new ERR_INVALID_PACKAGE_TARGET(StringPrototypeSlice(baseUrl.pathname + , 0, -1), mappingKey, subpath, target); for (const targetValue of target) { - if (ArrayIsArray(targetValue)) continue; try { - return resolveExportsTarget(pkgPath, targetValue, subpath, basePath, - mappingKey); + return resolveExportsTarget(baseUrl, targetValue, subpath, mappingKey); } catch (e) { - if (e.code !== 'MODULE_NOT_FOUND') throw e; + if (e.code !== 'ERR_PACKAGE_PATH_NOT_EXPORTED' && + e.code !== 'ERR_INVALID_PACKAGE_TARGET') + throw e; } } + // Throw last fallback error + resolveExportsTarget(baseUrl, target[target.length - 1], subpath, + mappingKey); + assert(false); } else if (typeof target === 'object' && target !== null) { const keys = ObjectKeys(target); if (keys.some(isArrayIndex)) { - throw new ERR_INVALID_PACKAGE_CONFIG(basePath, '"exports" cannot ' + + throw new ERR_INVALID_PACKAGE_CONFIG(baseUrl, '"exports" cannot ' + 'contain numeric property keys.'); } for (const p of keys) { @@ -607,34 +613,26 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) { case 'require': try { emitExperimentalWarning('Conditional exports'); - const result = resolveExportsTarget(pkgPath, target[p], subpath, - basePath, mappingKey); - return result; + return resolveExportsTarget(baseUrl, target[p], subpath, + mappingKey); } catch (e) { - if (e.code !== 'MODULE_NOT_FOUND') throw e; + if (e.code !== 'ERR_PACKAGE_PATH_NOT_EXPORTED') throw e; } break; case 'default': try { - return resolveExportsTarget(pkgPath, target.default, subpath, - basePath, mappingKey); + return resolveExportsTarget(baseUrl, target.default, subpath, + mappingKey); } catch (e) { - if (e.code !== 'MODULE_NOT_FOUND') throw e; + if (e.code !== 'ERR_PACKAGE_PATH_NOT_EXPORTED') throw e; } } } + throw new ERR_PACKAGE_PATH_NOT_EXPORTED( + StringPrototypeSlice(baseUrl.pathname, 0, -1), mappingKey + subpath); } - let e; - if (mappingKey !== '.') { - // eslint-disable-next-line no-restricted-syntax - e = new Error(`Package exports for '${basePath}' do not define a ` + - `valid '${mappingKey}' target${subpath ? ' for ' + subpath : ''}`); - } else { - // eslint-disable-next-line no-restricted-syntax - e = new Error(`No valid exports main found for '${basePath}'`); - } - e.code = 'MODULE_NOT_FOUND'; - throw e; + throw new ERR_INVALID_PACKAGE_TARGET( + StringPrototypeSlice(baseUrl.pathname, 0, -1), mappingKey, subpath, target); } Module._findPath = function(request, paths, isMain) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 304d0d04e6095b..c25edd9a28a964 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -832,10 +832,20 @@ void ThrowExportsNotFound(Environment* env, const std::string& subpath, const URL& pjson_url, const URL& base) { - const std::string msg = "Package exports for " + - pjson_url.ToFilePath() + " do not define a '" + subpath + - "' subpath, imported from " + base.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + const std::string msg = "Package subpath '" + subpath + "' is not defined" + + " by \"exports\" in " + pjson_url.ToFilePath() + " imported from " + + base.ToFilePath(); + node::THROW_ERR_PACKAGE_PATH_NOT_EXPORTED(env, msg.c_str()); +} + +void ThrowSubpathInvalid(Environment* env, + const std::string& subpath, + const URL& pjson_url, + const URL& base) { + const std::string msg = "Package subpath '" + subpath + "' is not a valid " + + "module request for the \"exports\" resolution of " + + pjson_url.ToFilePath() + " imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_MODULE_SPECIFIER(env, msg.c_str()); } void ThrowExportsInvalid(Environment* env, @@ -844,14 +854,15 @@ void ThrowExportsInvalid(Environment* env, const URL& pjson_url, const URL& base) { if (subpath.length()) { - const std::string msg = "Cannot resolve package exports target '" + target + - "' matched for '" + subpath + "' in " + pjson_url.ToFilePath() + - ", imported from " + base.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + const std::string msg = "Invalid \"exports\" target \"" + target + + "\" defined for '" + subpath + "' in the package config " + + pjson_url.ToFilePath() + " imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_PACKAGE_TARGET(env, msg.c_str()); } else { - const std::string msg = "Cannot resolve package main '" + target + "' in" + - pjson_url.ToFilePath() + ", imported from " + base.ToFilePath(); - node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); + const std::string msg = "Invalid \"exports\" main target " + target + + " defined in the package config " + pjson_url.ToFilePath() + + " imported from " + base.ToFilePath(); + node::THROW_ERR_INVALID_PACKAGE_TARGET(env, msg.c_str()); } } @@ -861,14 +872,20 @@ void ThrowExportsInvalid(Environment* env, const URL& pjson_url, const URL& base) { Local target_string; - if (target->ToString(env->context()).ToLocal(&target_string)) { - Utf8Value target_utf8(env->isolate(), target_string); - std::string target_str(*target_utf8, target_utf8.length()); - if (target->IsArray()) { - target_str = '[' + target_str + ']'; - } - ThrowExportsInvalid(env, subpath, target_str, pjson_url, base); + if (target->IsObject()) { + if (!v8::JSON::Stringify(env->context(), target.As(), + v8::String::Empty(env->isolate())).ToLocal(&target_string)) + return; + } else { + if (!target->ToString(env->context()).ToLocal(&target_string)) + return; + } + Utf8Value target_utf8(env->isolate(), target_string); + std::string target_str(*target_utf8, target_utf8.length()); + if (target->IsArray()) { + target_str = '[' + target_str + ']'; } + ThrowExportsInvalid(env, subpath, target_str, pjson_url, base); } Maybe ResolveExportsTargetString(Environment* env, @@ -876,18 +893,13 @@ Maybe ResolveExportsTargetString(Environment* env, const std::string& subpath, const std::string& match, const URL& pjson_url, - const URL& base, - bool throw_invalid = true) { + const URL& base) { if (target.substr(0, 2) != "./") { - if (throw_invalid) { - ThrowExportsInvalid(env, match, target, pjson_url, base); - } + ThrowExportsInvalid(env, match, target, pjson_url, base); return Nothing(); } if (subpath.length() > 0 && target.back() != '/') { - if (throw_invalid) { - ThrowExportsInvalid(env, match, target, pjson_url, base); - } + ThrowExportsInvalid(env, match, target, pjson_url, base); return Nothing(); } URL resolved(target, pjson_url); @@ -896,9 +908,7 @@ Maybe ResolveExportsTargetString(Environment* env, if (resolved_path.find(pkg_path) != 0 || resolved_path.find("/node_modules/", pkg_path.length() - 1) != std::string::npos) { - if (throw_invalid) { - ThrowExportsInvalid(env, match, target, pjson_url, base); - } + ThrowExportsInvalid(env, match, target, pjson_url, base); return Nothing(); } if (subpath.length() == 0) return Just(resolved); @@ -907,9 +917,7 @@ Maybe ResolveExportsTargetString(Environment* env, if (subpath_resolved_path.find(resolved_path) != 0 || subpath_resolved_path.find("/node_modules/", pkg_path.length() - 1) != std::string::npos) { - if (throw_invalid) { - ThrowExportsInvalid(env, match, target + subpath, pjson_url, base); - } + ThrowSubpathInvalid(env, match + subpath, pjson_url, base); return Nothing(); } return Just(subpath_resolved); @@ -939,15 +947,14 @@ Maybe ResolveExportsTarget(Environment* env, Local target, const std::string& subpath, const std::string& pkg_subpath, - const URL& base, - bool throw_invalid = true) { + const URL& base) { Isolate* isolate = env->isolate(); Local context = env->context(); if (target->IsString()) { Utf8Value target_utf8(isolate, target.As()); std::string target_str(*target_utf8, target_utf8.length()); Maybe resolved = ResolveExportsTargetString(env, target_str, subpath, - pkg_subpath, pjson_url, base, throw_invalid); + pkg_subpath, pjson_url, base); if (resolved.IsNothing()) { return Nothing(); } @@ -956,40 +963,56 @@ Maybe ResolveExportsTarget(Environment* env, Local target_arr = target.As(); const uint32_t length = target_arr->Length(); if (length == 0) { - if (throw_invalid) { - ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); - } + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); return Nothing(); } for (uint32_t i = 0; i < length; i++) { auto target_item = target_arr->Get(context, i).ToLocalChecked(); - if (!target_item->IsArray()) { + { + TryCatchScope try_catch(env); Maybe resolved = ResolveExportsTarget(env, pjson_url, - target_item, subpath, pkg_subpath, base, false); - if (resolved.IsNothing()) continue; + target_item, subpath, pkg_subpath, base); + if (resolved.IsNothing()) { + CHECK(try_catch.HasCaught()); + if (try_catch.Exception().IsEmpty()) return Nothing(); + Local e; + if (!try_catch.Exception()->ToObject(context).ToLocal(&e)) + return Nothing(); + Local code; + if (!e->Get(context, env->code_string()).ToLocal(&code)) + return Nothing(); + Local code_string; + if (!code->ToString(context).ToLocal(&code_string)) + return Nothing(); + Utf8Value code_utf8(env->isolate(), code_string); + if (strcmp(*code_utf8, "ERR_PACKAGE_PATH_NOT_EXPORTED") == 0 || + strcmp(*code_utf8, "ERR_INVALID_PACKAGE_TARGET") == 0) { + continue; + } + try_catch.ReThrow(); + return Nothing(); + } + CHECK(!try_catch.HasCaught()); return FinalizeResolution(env, resolved.FromJust(), base); } } - if (throw_invalid) { - auto invalid = target_arr->Get(context, length - 1).ToLocalChecked(); - Maybe resolved = ResolveExportsTarget(env, pjson_url, invalid, - subpath, pkg_subpath, base, true); - CHECK(resolved.IsNothing()); - } + auto invalid = target_arr->Get(context, length - 1).ToLocalChecked(); + Maybe resolved = ResolveExportsTarget(env, pjson_url, invalid, + subpath, pkg_subpath, base); + CHECK(resolved.IsNothing()); return Nothing(); } else if (target->IsObject()) { Local target_obj = target.As(); Local target_obj_keys = target_obj->GetOwnPropertyNames(context).ToLocalChecked(); Local conditionalTarget; - bool matched = false; for (uint32_t i = 0; i < target_obj_keys->Length(); ++i) { Local key = target_obj_keys->Get(context, i).ToLocalChecked(); if (IsArrayIndex(env, key)) { - const std::string msg = "Invalid package config for " + - pjson_url.ToFilePath() + ", \"exports\" cannot contain numeric " + - "property keys."; + const std::string msg = "Invalid package config " + + pjson_url.ToFilePath() + " imported from " + base.ToFilePath() + + ". \"exports\" cannot contain numeric property keys."; node::THROW_ERR_INVALID_PACKAGE_CONFIG(env, msg.c_str()); return Nothing(); } @@ -1000,35 +1023,60 @@ Maybe ResolveExportsTarget(Environment* env, key->ToString(context).ToLocalChecked()); std::string key_str(*key_utf8, key_utf8.length()); if (key_str == "node" || key_str == "import") { - matched = true; conditionalTarget = target_obj->Get(context, key).ToLocalChecked(); - Maybe resolved = ResolveExportsTarget(env, pjson_url, - conditionalTarget, subpath, pkg_subpath, base, false); - if (!resolved.IsNothing()) { + { + TryCatchScope try_catch(env); + Maybe resolved = ResolveExportsTarget(env, pjson_url, + conditionalTarget, subpath, pkg_subpath, base); + if (resolved.IsNothing()) { + CHECK(try_catch.HasCaught()); + if (try_catch.Exception().IsEmpty()) return Nothing(); + Local e; + if (!try_catch.Exception()->ToObject(context).ToLocal(&e)) + return Nothing(); + Local code; + if (!e->Get(context, env->code_string()).ToLocal(&code)) + return Nothing(); + Local code_string; + if (!code->ToString(context).ToLocal(&code_string)) + return Nothing(); + Utf8Value code_utf8(env->isolate(), code_string); + if (strcmp(*code_utf8, "ERR_PACKAGE_PATH_NOT_EXPORTED") == 0) + continue; + try_catch.ReThrow(); + return Nothing(); + } + CHECK(!try_catch.HasCaught()); ProcessEmitExperimentalWarning(env, "Conditional exports"); return resolved; } } else if (key_str == "default") { - matched = true; conditionalTarget = target_obj->Get(context, key).ToLocalChecked(); - Maybe resolved = ResolveExportsTarget(env, pjson_url, - conditionalTarget, subpath, pkg_subpath, base, false); - if (!resolved.IsNothing()) { + { + TryCatchScope try_catch(env); + Maybe resolved = ResolveExportsTarget(env, pjson_url, + conditionalTarget, subpath, pkg_subpath, base); + if (resolved.IsNothing()) { + CHECK(try_catch.HasCaught() && !try_catch.Exception().IsEmpty()); + auto e = try_catch.Exception()->ToObject(context).ToLocalChecked(); + auto code = e->Get(context, env->code_string()).ToLocalChecked(); + Utf8Value code_utf8(env->isolate(), + code->ToString(context).ToLocalChecked()); + std::string code_str(*code_utf8, code_utf8.length()); + if (code_str == "ERR_PACKAGE_PATH_NOT_EXPORTED") continue; + try_catch.ReThrow(); + return Nothing(); + } + CHECK(!try_catch.HasCaught()); ProcessEmitExperimentalWarning(env, "Conditional exports"); return resolved; } } } - if (matched && throw_invalid) { - Maybe resolved = ResolveExportsTarget(env, pjson_url, - conditionalTarget, subpath, pkg_subpath, base, true); - CHECK(resolved.IsNothing()); - return Nothing(); - } - } - if (throw_invalid) { - ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); + ThrowExportsNotFound(env, pkg_subpath, pjson_url, base); + return Nothing(); } + ThrowExportsInvalid(env, pkg_subpath, target, pjson_url, base); return Nothing(); } @@ -1050,8 +1098,8 @@ Maybe IsConditionalExportsMainSugar(Environment* env, if (i == 0) { isConditionalSugar = curIsConditionalSugar; } else if (isConditionalSugar != curIsConditionalSugar) { - const std::string msg = "Cannot resolve package exports in " + - pjson_url.ToFilePath() + ", imported from " + base.ToFilePath() + ". " + + const std::string msg = "Invalid package config " + pjson_url.ToFilePath() + + " imported from " + base.ToFilePath() + ". " + "\"exports\" cannot contain some keys starting with '.' and some not." + " The exports object must either be an object of package subpath keys" + " or an object of main entry condition name keys only."; @@ -1076,8 +1124,7 @@ Maybe PackageMainResolve(Environment* env, if (isConditionalExportsMainSugar.IsNothing()) return Nothing(); if (isConditionalExportsMainSugar.FromJust()) { - return ResolveExportsTarget(env, pjson_url, exports, "", "", base, - true); + return ResolveExportsTarget(env, pjson_url, exports, "", "", base); } else if (exports->IsObject()) { Local exports_obj = exports.As(); if (exports_obj->HasOwnProperty(env->context(), env->dot_string()) @@ -1085,10 +1132,12 @@ Maybe PackageMainResolve(Environment* env, Local target = exports_obj->Get(env->context(), env->dot_string()) .ToLocalChecked(); - return ResolveExportsTarget(env, pjson_url, target, "", "", base, - true); + return ResolveExportsTarget(env, pjson_url, target, "", "", base); } } + std::string msg = "No \"exports\" main resolved in " + + pjson_url.ToFilePath(); + node::THROW_ERR_PACKAGE_PATH_NOT_EXPORTED(env, msg.c_str()); } if (pcfg.has_main == HasMain::Yes) { URL resolved(pcfg.main, pjson_url); @@ -1180,39 +1229,6 @@ Maybe PackageExportsResolve(Environment* env, return Nothing(); } -Maybe ResolveSelf(Environment* env, - const std::string& pkg_name, - const std::string& pkg_subpath, - const URL& base) { - const PackageConfig* pcfg; - if (GetPackageScopeConfig(env, base, base).To(&pcfg) && - pcfg->exists == Exists::Yes) { - // TODO(jkrems): Find a way to forward the pair/iterator already generated - // while executing GetPackageScopeConfig - URL pjson_url(""); - bool found_pjson = false; - for (auto it = env->package_json_cache.begin(); - it != env->package_json_cache.end(); - ++it) { - if (&it->second == pcfg) { - pjson_url = URL::FromFilePath(it->first); - found_pjson = true; - } - } - if (!found_pjson || pcfg->name != pkg_name) return Nothing(); - if (pcfg->exports.IsEmpty()) return Nothing(); - if (pkg_subpath == "./") { - return Just(URL("./", pjson_url)); - } else if (!pkg_subpath.length()) { - return PackageMainResolve(env, pjson_url, *pcfg, base); - } else { - return PackageExportsResolve(env, pjson_url, pkg_subpath, *pcfg, base); - } - } - - return Nothing(); -} - Maybe PackageResolve(Environment* env, const std::string& specifier, const URL& base) { @@ -1253,10 +1269,30 @@ Maybe PackageResolve(Environment* env, pkg_subpath = "." + specifier.substr(sep_index); } - Maybe self_url = ResolveSelf(env, pkg_name, pkg_subpath, base); - if (self_url.IsJust()) { - ProcessEmitExperimentalWarning(env, "Package name self resolution"); - return self_url; + // ResolveSelf + const PackageConfig* pcfg; + if (GetPackageScopeConfig(env, base, base).To(&pcfg) && + pcfg->exists == Exists::Yes) { + // TODO(jkrems): Find a way to forward the pair/iterator already generated + // while executing GetPackageScopeConfig + URL pjson_url(""); + bool found_pjson = false; + for (const auto& it : env->package_json_cache) { + if (&it.second == pcfg) { + pjson_url = URL::FromFilePath(it.first); + found_pjson = true; + } + } + if (found_pjson && pcfg->name == pkg_name && !pcfg->exports.IsEmpty()) { + ProcessEmitExperimentalWarning(env, "Package name self resolution"); + if (pkg_subpath == "./") { + return Just(URL("./", pjson_url)); + } else if (!pkg_subpath.length()) { + return PackageMainResolve(env, pjson_url, *pcfg, base); + } else { + return PackageExportsResolve(env, pjson_url, pkg_subpath, *pcfg, base); + } + } } URL pjson_url("./node_modules/" + pkg_name + "/package.json", &base); diff --git a/src/node_errors.h b/src/node_errors.h index e0bb77e25c6f81..2da952ad29d499 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -41,7 +41,8 @@ void OnFatalError(const char* location, const char* message); V(ERR_OSSL_EVP_INVALID_DIGEST, Error) \ V(ERR_INVALID_ARG_TYPE, TypeError) \ V(ERR_INVALID_MODULE_SPECIFIER, TypeError) \ - V(ERR_INVALID_PACKAGE_CONFIG, SyntaxError) \ + V(ERR_INVALID_PACKAGE_CONFIG, Error) \ + V(ERR_INVALID_PACKAGE_TARGET, Error) \ V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \ V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MISSING_ARGS, TypeError) \ @@ -51,6 +52,7 @@ void OnFatalError(const char* location, const char* message); V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \ V(ERR_MODULE_NOT_FOUND, Error) \ V(ERR_OUT_OF_RANGE, RangeError) \ + V(ERR_PACKAGE_PATH_NOT_EXPORTED, Error) \ V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \ V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \ V(ERR_STRING_TOO_LONG, Error) \ diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index 6ac06ee3ea0ba2..cc576e083a236d 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -33,7 +33,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/resolve-self', isRequire ? { default: 'self-cjs' } : { default: 'self-mjs' }], // Resolve self sugar - ['pkgexports-sugar', { default: 'main' }] + ['pkgexports-sugar', { default: 'main' }], ]); for (const [validSpecifier, expected] of validSpecifiers) { @@ -54,48 +54,59 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; // Sugar cases still encapsulate ['pkgexports-sugar/not-exported.js', './not-exported.js'], ['pkgexports-sugar2/not-exported.js', './not-exported.js'], + // Conditional exports with no match are "not exported" errors + ['pkgexports/invalid1', './invalid1'], + ['pkgexports/invalid4', './invalid4'], ]); const invalidExports = new Map([ - // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" - // variants do not to prevent confusion and accidental loopholes. - ['pkgexports/sub/./../asdf.js', './sub/./../asdf.js'], + // Directory mappings require a trailing / to work + ['pkgexports/missingtrailer/x', './missingtrailer/'], // This path steps back inside the package but goes through an exports // target that escapes the package, so we still catch that as invalid - ['pkgexports/belowdir/pkgexports/asdf.js', './belowdir/pkgexports/asdf.js'], + ['pkgexports/belowdir/pkgexports/asdf.js', './belowdir/'], // This target file steps below the package ['pkgexports/belowfile', './belowfile'], - // Directory mappings require a trailing / to work - ['pkgexports/missingtrailer/x', './missingtrailer/x'], // Invalid target handling ['pkgexports/null', './null'], - ['pkgexports/invalid1', './invalid1'], ['pkgexports/invalid2', './invalid2'], ['pkgexports/invalid3', './invalid3'], - ['pkgexports/invalid4', './invalid4'], // Missing / invalid fallbacks ['pkgexports/nofallback1', './nofallback1'], ['pkgexports/nofallback2', './nofallback2'], // Reaching into nested node_modules ['pkgexports/nodemodules', './nodemodules'], + // Self resolve invalid + ['pkgexports/resolve-self-invalid', './invalid2'], + ]); + + const invalidSpecifiers = new Map([ + // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" + // variants do not to prevent confusion and accidental loopholes. + ['pkgexports/sub/./../asdf.js', './sub/./../asdf.js'], ]); for (const [specifier, subpath] of undefinedExports) { loadFixture(specifier).catch(mustCall((err) => { - strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, 'Package exports'); - assertIncludes(err.message, `do not define a '${subpath}' subpath`); + strictEqual(err.code, 'ERR_PACKAGE_PATH_NOT_EXPORTED'); + assertStartsWith(err.message, 'Package subpath '); + assertIncludes(err.message, subpath); })); } for (const [specifier, subpath] of invalidExports) { loadFixture(specifier).catch(mustCall((err) => { - strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, (isRequire ? 'Package exports' : - 'Cannot resolve')); - assertIncludes(err.message, isRequire ? - `do not define a valid '${subpath}' target` : - `matched for '${subpath}'`); + strictEqual(err.code, 'ERR_INVALID_PACKAGE_TARGET'); + assertStartsWith(err.message, 'Invalid "exports"'); + assertIncludes(err.message, subpath); + })); + } + + for (const [specifier, subpath] of invalidSpecifiers) { + loadFixture(specifier).catch(mustCall((err) => { + strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER'); + assertStartsWith(err.message, 'Package subpath '); + assertIncludes(err.message, subpath); })); } @@ -103,8 +114,8 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; // of falling back to main if (isRequire) { loadFixture('pkgexports-main').catch(mustCall((err) => { - strictEqual(err.code, 'MODULE_NOT_FOUND'); - assertStartsWith(err.message, 'No valid export'); + strictEqual(err.code, 'ERR_PACKAGE_PATH_NOT_EXPORTED'); + assertStartsWith(err.message, 'No "exports" main '); })); } @@ -131,8 +142,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; // Sugar conditional exports main mixed failure case loadFixture('pkgexports-sugar-fail').catch(mustCall((err) => { strictEqual(err.code, 'ERR_INVALID_PACKAGE_CONFIG'); - assertStartsWith(err.message, (isRequire ? 'Invalid package' : - 'Cannot resolve')); + assertStartsWith(err.message, 'Invalid package'); assertIncludes(err.message, '"exports" cannot contain some keys starting ' + 'with \'.\' and some not. The exports object must either be an object of ' + 'package subpath keys or an object of main entry condition name keys ' + diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index 02e06f0ebe5f3c..7f417ad5457bfc 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -35,6 +35,10 @@ "./resolve-self": { "require": "./resolve-self.js", "import": "./resolve-self.mjs" + }, + "./resolve-self-invalid": { + "require": "./resolve-self-invalid.js", + "import": "./resolve-self-invalid.mjs" } } } diff --git a/test/fixtures/node_modules/pkgexports/resolve-self-invalid.js b/test/fixtures/node_modules/pkgexports/resolve-self-invalid.js new file mode 100644 index 00000000000000..c3ebf76fc1b2f3 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/resolve-self-invalid.js @@ -0,0 +1 @@ +require('pkg-exports/invalid2'); diff --git a/test/fixtures/node_modules/pkgexports/resolve-self-invalid.mjs b/test/fixtures/node_modules/pkgexports/resolve-self-invalid.mjs new file mode 100644 index 00000000000000..1edbf62c4b0114 --- /dev/null +++ b/test/fixtures/node_modules/pkgexports/resolve-self-invalid.mjs @@ -0,0 +1 @@ +import 'pkg-exports/invalid2'; From e147f61624d1b5e7d926b8bdc771b913729ecc23 Mon Sep 17 00:00:00 2001 From: himself65 Date: Sun, 8 Mar 2020 15:36:35 +0800 Subject: [PATCH 04/12] esm: remove unused parameter on module.instantiate PR-URL: https://github.com/nodejs/node/pull/32147 Reviewed-By: Gus Caplan Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- lib/internal/modules/esm/module_job.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 01a574044b4f4a..08ba06101ca92c 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -89,7 +89,7 @@ class ModuleJob { const initWrapper = internalBinding('inspector').callAndPauseOnStart; initWrapper(this.module.instantiate, this.module); } else { - this.module.instantiate(true); + this.module.instantiate(); } } catch (e) { decorateErrorStack(e); From 07b77a366e35004a735f4299f863fecaff59e6b0 Mon Sep 17 00:00:00 2001 From: Jan Krems Date: Tue, 3 Mar 2020 09:03:04 -0800 Subject: [PATCH 05/12] module: add hook for global preload code PR-URL: https://github.com/nodejs/node/pull/32068 Reviewed-By: Bradley Farias Reviewed-By: Geoffrey Booth --- doc/api/esm.md | 33 ++++++++++++ lib/internal/modules/esm/loader.js | 51 ++++++++++++++++++- lib/internal/process/esm_loader.js | 1 + .../es-module/test-esm-loader-side-effect.mjs | 32 ++++++++++++ .../loader-side-effect-require-preload.js | 6 +++ .../es-module-loaders/loader-side-effect.mjs | 32 ++++++++++++ 6 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 test/es-module/test-esm-loader-side-effect.mjs create mode 100644 test/fixtures/es-module-loaders/loader-side-effect-require-preload.js create mode 100644 test/fixtures/es-module-loaders/loader-side-effect.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index 0ca5dae5eadbff..206103e8408f78 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1209,6 +1209,39 @@ export async function transformSource(source, } ``` +#### getGlobalPreloadCode hook + +> Note: The loaders API is being redesigned. This hook may disappear or its +> signature may change. Do not rely on the API described below. + +Sometimes it can be necessary to run some code inside of the same global scope +that the application will run in. This hook allows to return a string that will +be ran as sloppy-mode script on startup. + +Similar to how CommonJS wrappers work, the code runs in an implicit function +scope. The only argument is a `require`-like function that can be used to load +builtins like "fs": `getBuiltin(request: string)`. + +If the code needs more advanced `require` features, it will have to construct +its own `require` using `module.createRequire()`. + +```js +/** + * @returns {string} Code to run before application startup + */ +export function getGlobalPreloadCode() { + return `\ +globalThis.someInjectedProperty = 42; +console.log('I just set some globals!'); + +const { createRequire } = getBuiltin('module'); + +const require = createRequire(process.cwd + '/'); +// [...] +`; +} +``` + #### dynamicInstantiate hook > Note: The loaders API is being redesigned. This hook may disappear or its diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 5a27f4be7c9d1a..285f656fa99b11 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -7,6 +7,7 @@ const { } = primordials; const { + ERR_INVALID_ARG_VALUE, ERR_INVALID_RETURN_PROPERTY, ERR_INVALID_RETURN_PROPERTY_VALUE, ERR_INVALID_RETURN_VALUE, @@ -47,6 +48,14 @@ class Loader { // Map of already-loaded CJS modules to use this.cjsCache = new SafeMap(); + // This hook is called before the first root module is imported. It's a + // function that returns a piece of code that runs as a sloppy-mode script. + // The script may evaluate to a function that can be called with a + // `getBuiltin` helper that can be used to retrieve builtins. + // If the hook returns `null` instead of a source string, it opts out of + // running any preload code. + // The preload code runs as soon as the hook module has finished evaluating. + this._getGlobalPreloadCode = null; // The resolver has the signature // (specifier : string, parentURL : string, defaultResolve) // -> Promise<{ url : string }> @@ -168,7 +177,16 @@ class Loader { return module.getNamespace(); } - hook({ resolve, dynamicInstantiate, getFormat, getSource, transformSource }) { + hook(hooks) { + const { + resolve, + dynamicInstantiate, + getFormat, + getSource, + transformSource, + getGlobalPreloadCode, + } = hooks; + // Use .bind() to avoid giving access to the Loader instance when called. if (resolve !== undefined) this._resolve = FunctionPrototypeBind(resolve, null); @@ -185,6 +203,37 @@ class Loader { if (transformSource !== undefined) { this._transformSource = FunctionPrototypeBind(transformSource, null); } + if (getGlobalPreloadCode !== undefined) { + this._getGlobalPreloadCode = + FunctionPrototypeBind(getGlobalPreloadCode, null); + } + } + + runGlobalPreloadCode() { + if (!this._getGlobalPreloadCode) { + return; + } + const preloadCode = this._getGlobalPreloadCode(); + if (preloadCode === null) { + return; + } + + if (typeof preloadCode !== 'string') { + throw new ERR_INVALID_RETURN_VALUE( + 'string', 'loader getGlobalPreloadCode', preloadCode); + } + const { compileFunction } = require('vm'); + const preloadInit = compileFunction(preloadCode, ['getBuiltin'], { + filename: '', + }); + const { NativeModule } = require('internal/bootstrap/loaders'); + + preloadInit.call(globalThis, (builtinName) => { + if (NativeModule.canBeRequiredByUsers(builtinName)) { + return require(builtinName); + } + throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName); + }); } async getModuleJob(specifier, parentURL) { diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 49463e284c541f..ce6a7e756263c1 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -60,6 +60,7 @@ async function initializeLoader() { await ESMLoader.import(userLoader, pathToFileURL(cwd).href); ESMLoader = new Loader(); ESMLoader.hook(hooks); + ESMLoader.runGlobalPreloadCode(); return exports.ESMLoader = ESMLoader; })(); } diff --git a/test/es-module/test-esm-loader-side-effect.mjs b/test/es-module/test-esm-loader-side-effect.mjs new file mode 100644 index 00000000000000..3ac71448252e97 --- /dev/null +++ b/test/es-module/test-esm-loader-side-effect.mjs @@ -0,0 +1,32 @@ +// Flags: --experimental-modules --experimental-loader ./test/fixtures/es-module-loaders/loader-side-effect.mjs --require ./test/fixtures/es-module-loaders/loader-side-effect-require-preload.js +import { allowGlobals, mustCall } from '../common/index.mjs'; +import assert from 'assert'; +import { fileURLToPath } from 'url'; +import { Worker, isMainThread, parentPort } from 'worker_threads'; + +/* global implicitGlobalProperty */ +assert.strictEqual(globalThis.implicitGlobalProperty, 42); +allowGlobals(implicitGlobalProperty); + +/* global implicitGlobalConst */ +assert.strictEqual(implicitGlobalConst, 42 * 42); +allowGlobals(implicitGlobalConst); + +/* global explicitGlobalProperty */ +assert.strictEqual(globalThis.explicitGlobalProperty, 42 * 42 * 42); +allowGlobals(explicitGlobalProperty); + +/* global preloadOrder */ +assert.deepStrictEqual(globalThis.preloadOrder, ['--require', 'loader']); +allowGlobals(preloadOrder); + +if (isMainThread) { + const worker = new Worker(fileURLToPath(import.meta.url)); + const promise = new Promise((resolve, reject) => { + worker.on('message', resolve); + worker.on('error', reject); + }); + promise.then(mustCall()); +} else { + parentPort.postMessage('worker done'); +} diff --git a/test/fixtures/es-module-loaders/loader-side-effect-require-preload.js b/test/fixtures/es-module-loaders/loader-side-effect-require-preload.js new file mode 100644 index 00000000000000..98820b9379748e --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-side-effect-require-preload.js @@ -0,0 +1,6 @@ +/** + * This file is combined with `loader-side-effect.mjs` via `--require`. Its + * purpose is to test execution order of the two kinds of preload code. + */ + +(globalThis.preloadOrder || (globalThis.preloadOrder = [])).push('--require'); diff --git a/test/fixtures/es-module-loaders/loader-side-effect.mjs b/test/fixtures/es-module-loaders/loader-side-effect.mjs new file mode 100644 index 00000000000000..5c80724fbb95f6 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-side-effect.mjs @@ -0,0 +1,32 @@ +// Arrow function so it closes over the this-value of the preload scope. +const globalPreload = () => { + /* global getBuiltin */ + const assert = getBuiltin('assert'); + const vm = getBuiltin('vm'); + + assert.strictEqual(typeof require, 'undefined'); + assert.strictEqual(typeof module, 'undefined'); + assert.strictEqual(typeof exports, 'undefined'); + assert.strictEqual(typeof __filename, 'undefined'); + assert.strictEqual(typeof __dirname, 'undefined'); + + assert.strictEqual(this, globalThis); + (globalThis.preloadOrder || (globalThis.preloadOrder = [])).push('loader'); + + vm.runInThisContext(`\ +var implicitGlobalProperty = 42; +const implicitGlobalConst = 42 * 42; +`); + + assert.strictEqual(globalThis.implicitGlobalProperty, 42); + (implicitGlobalProperty).foo = 'bar'; // assert: not strict mode + + globalThis.explicitGlobalProperty = 42 * 42 * 42; +} + +export function getGlobalPreloadCode() { + return `\ + +(${globalPreload.toString()})(); +`; +} From 9da5813fe8de35b926ba5ea14c6591413e6a6689 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Wed, 18 Mar 2020 12:25:57 -0500 Subject: [PATCH 06/12] doc: import clarifications with links to MDN PR-URL: https://github.com/nodejs/node/pull/31479 Reviewed-By: James M Snell Reviewed-By: Geoffrey Booth Reviewed-By: Myles Borins --- doc/api/esm.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 206103e8408f78..ba78d9e6ef1e82 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -929,17 +929,14 @@ import packageMain from 'commonjs-package'; // Works import { method } from 'commonjs-package'; // Errors ``` +It is also possible to +[import an ES or CommonJS module for its side effects only][]. + ### `import()` expressions -Dynamic `import()` is supported in both CommonJS and ES modules. It can be used +[Dynamic `import()`][] is supported in both CommonJS and ES modules. It can be used to include ES module files from CommonJS code. -```js -(async () => { - await import('./my-app.mjs'); -})(); -``` - ## CommonJS, JSON, and Native Modules CommonJS, JSON, and Native modules can be used with @@ -1737,6 +1734,7 @@ success! [Babel]: https://babeljs.io/ [CommonJS]: modules.html [Conditional Exports]: #esm_conditional_exports +[Dynamic `import()`]: https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Dynamic_Imports [ECMAScript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md [ES Module Integration Proposal for Web Assembly]: https://github.com/webassembly/esm-integration [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md @@ -1748,13 +1746,14 @@ success! [`esm`]: https://github.com/standard-things/esm#readme [`export`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export [`getFormat` hook]: #esm_code_getformat_code_hook -[`import()`]: #esm_import-expressions +[`import()`]: #esm_import_expressions [`import.meta.url`]: #esm_import_meta [`import`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import [`module.createRequire()`]: modules.html#modules_module_createrequire_filename [`module.syncBuiltinESMExports()`]: modules.html#modules_module_syncbuiltinesmexports [`transformSource` hook]: #esm_code_transformsource_code_hook [dynamic instantiate hook]: #esm_code_dynamicinstantiate_code_hook +[import an ES or CommonJS module for its side effects only]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Import_a_module_for_its_side_effects_only [special scheme]: https://url.spec.whatwg.org/#special-scheme [the official standard format]: https://tc39.github.io/ecma262/#sec-modules [transpiler loader example]: #esm_transpiler_loader From ef27b5aa3162cb95bced41d9385dd5e9035500b8 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 22 Mar 2020 09:43:17 -0700 Subject: [PATCH 07/12] doc: improve wording in esm.md Simplify complex sentence. Remove use of "straightforward". PR-URL: https://github.com/nodejs/node/pull/32427 Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat --- doc/api/esm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index ba78d9e6ef1e82..13eae3a63d1f55 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -637,8 +637,8 @@ stateless): ##### Approach #2: Isolate State -The most straightforward `package.json` would be one that defines the separate -CommonJS and ES module entry points directly: +A `package.json` file can define the separate CommonJS and ES module entry +points directly: ```js From 5c7c915a3f814e9c641da1241a80e12df9b6fb28 Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Tue, 24 Mar 2020 13:17:05 -0400 Subject: [PATCH 08/12] doc: fix lint warning in doc/api/esm.md Signed-off-by: Richard Lau PR-URL: https://github.com/nodejs/node/pull/32462 Refs: https://github.com/nodejs/node/pull/31479 Reviewed-By: Luigi Pinca Reviewed-By: Beth Griggs Reviewed-By: Trivikram Kamat --- doc/api/esm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 13eae3a63d1f55..f2b1c0c6c398ee 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -934,8 +934,8 @@ It is also possible to ### `import()` expressions -[Dynamic `import()`][] is supported in both CommonJS and ES modules. It can be used -to include ES module files from CommonJS code. +[Dynamic `import()`][] is supported in both CommonJS and ES modules. It can be +used to include ES module files from CommonJS code. ## CommonJS, JSON, and Native Modules From 17a18a1f6c85097c7586cfc5dea84bfcf38e0488 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 24 Mar 2020 06:23:26 -0700 Subject: [PATCH 09/12] doc: remove unnecessary "obvious(ly)" modifiers in esm.md Remove "obvious" and "obviously" in two places in esm.md. It may be obvious to some, but likely not everyone or else it probably wouldn't be worth mentioning/documenting. PR-URL: https://github.com/nodejs/node/pull/32457 Reviewed-By: James M Snell Reviewed-By: Myles Borins --- doc/api/esm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index f2b1c0c6c398ee..98fed9bf9a701e 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -535,7 +535,7 @@ would be usable by any version of Node.js, since `import` can refer to CommonJS files; but it would not provide any of the advantages of using ES module syntax. A package could also switch from CommonJS to ES module syntax in a breaking -change version bump. This has the obvious disadvantage that the newest version +change version bump. This has the disadvantage that the newest version of the package would only be usable in ES module-supporting versions of Node.js. Every pattern has tradeoffs, but there are two broad approaches that satisfy the @@ -1362,7 +1362,7 @@ JavaScript using the [`transformSource` hook][]. Before that hook gets called, however, other hooks need to tell Node.js not to throw an error on unknown file types; and to tell Node.js how to load this new file type. -This is obviously less performant than transpiling source files before running +This is less performant than transpiling source files before running Node.js; a transpiler loader should only be used for development and testing purposes. From 26e44c946c8e2988153eedfb93a4814ba6a0d87a Mon Sep 17 00:00:00 2001 From: Nick Schonning Date: Sun, 8 Mar 2020 01:37:48 -0500 Subject: [PATCH 10/12] build: add mjs extension to lint-js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This matches what the lint-js make target passes through the CLI. PR-URL: https://github.com/nodejs/node/pull/32145 Reviewed-By: Richard Lau Reviewed-By: Denys Otrishko Reviewed-By: Michaël Zasso Reviewed-By: Yongsheng Zhang Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater Reviewed-By: Trivikram Kamat --- tools/lint-js.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/lint-js.js b/tools/lint-js.js index 30ff2d313c46c4..4ddbe228d16d37 100644 --- a/tools/lint-js.js +++ b/tools/lint-js.js @@ -1,7 +1,7 @@ 'use strict'; const rulesDirs = ['tools/eslint-rules']; -const extensions = ['.js', '.md']; +const extensions = ['.js', '.mjs', '.md']; // This is the maximum number of files to be linted per worker at any given time const maxWorkload = 60; From 4ab422acf0567f16a225ba27f0c2f77ecd4a52d4 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 4 Mar 2020 23:53:05 +0200 Subject: [PATCH 11/12] doc: update conditional exports recommendations Co-Authored-By: Geoffrey Booth PR-URL: https://github.com/nodejs/node/pull/32098 Reviewed-By: Geoffrey Booth --- doc/api/errors.md | 2 +- doc/api/esm.md | 280 ++++++++++++++++++++++------------------------ 2 files changed, 135 insertions(+), 147 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index ebc5a2e51d5b1b..8ecb313841c073 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2476,7 +2476,7 @@ such as `process.stdout.on('data')`. [crypto digest algorithm]: crypto.html#crypto_crypto_gethashes [domains]: domain.html [event emitter-based]: events.html#events_class_eventemitter -[exports]: esm.html#esm_package_exports +[exports]: esm.html#esm_package_entry_points [file descriptors]: https://en.wikipedia.org/wiki/File_descriptor [policy]: policy.html [stream-based]: stream.html diff --git a/doc/api/esm.md b/doc/api/esm.md index 98fed9bf9a701e..8114aa6914f1b1 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -183,87 +183,89 @@ unspecified. ### Package Entry Points -There are two fields that can define entry points for a package: `"main"` and -`"exports"`. The `"main"` field is supported in all versions of Node.js, but its -capabilities are limited: it only defines the main entry point of the package. -The `"exports"` field, part of [Package Exports][], provides an alternative to -`"main"` where the package main entry point can be defined while also -encapsulating the package, preventing any other entry points besides those -defined in `"exports"`. If package entry points are defined in both `"main"` and -`"exports"`, the latter takes precedence in versions of Node.js that support -`"exports"`. [Conditional Exports][] can also be used within `"exports"` to -define different package entry points per environment. - -#### package.json "main" - -The `package.json` `"main"` field defines the entry point for a package, -whether the package is included into CommonJS via `require` or into an ES -module via `import`. +In a package’s `package.json` file, two fields can define entry points for a +package: `"main"` and `"exports"`. The `"main"` field is supported in all +versions of Node.js, but its capabilities are limited: it only defines the main +entry point of the package. + +The `"exports"` field provides an alternative to `"main"` where the package +main entry point can be defined while also encapsulating the package, preventing +any other entry points besides those defined in `"exports"`. If package entry +points are defined in both `"main"` and `"exports"`, the latter takes precedence +in versions of Node.js that support `"exports"`. [Conditional Exports][] can +also be used within `"exports"` to define different package entry points per +environment, including whether the package is referenced via `require` or via +`import`. + +If both `"exports"` and `"main"` are defined, the `"exports"` field takes +precedence over `"main"`. + +Both `"main"` and `"exports"` entry points are not specific to ES modules or +CommonJS; `"main"` will be overridden by `"exports"` in a `require` so it is +not a CommonJS fallback. + +This is important with regard to `require`, since `require` of ES module files +throws an error in all versions of Node.js. To create a package that works both +in modern Node.js via `import` and `require` and also legacy Node.js versions, +see [the dual CommonJS/ES module packages section][]. + +#### Main Entry Point Export + +To set the main entry point for a package, it is advisable to define both +`"exports"` and `"main"` in the package’s `package.json` file: ```js -// ./node_modules/es-module-package/package.json { - "type": "module", - "main": "./src/index.js" + "main": "./main.js", + "exports": "./main.js" } ``` -```js -// ./my-app.mjs +The benefit of doing this is that when using the `"exports"` field all +subpaths of the package will no longer be available to importers under +`require('pkg/subpath.js')`, and instead they will get a new error, +`ERR_PACKAGE_PATH_NOT_EXPORTED`. -import { something } from 'es-module-package'; -// Loads from ./node_modules/es-module-package/src/index.js -``` +This encapsulation of exports provides more reliable guarantees +about package interfaces for tools and when handling semver upgrades for a +package. It is not a strong encapsulation since a direct require of any +absolute subpath of the package such as +`require('/path/to/node_modules/pkg/subpath.js')` will still load `subpath.js`. -An attempt to `require` the above `es-module-package` would attempt to load -`./node_modules/es-module-package/src/index.js` as CommonJS, which would throw -an error as Node.js would not be able to parse the `export` statement in -CommonJS. +#### Subpath Exports -As with `import` statements, for ES module usage the value of `"main"` must be -a full path including extension: `"./index.mjs"`, not `"./index"`. - -If the `package.json` `"type"` field is omitted, a `.js` file in `"main"` will -be interpreted as CommonJS. - -The `"main"` field can point to exactly one file, regardless of whether the -package is referenced via `require` (in a CommonJS context) or `import` (in an -ES module context). - -#### Package Exports - -By default, all subpaths from a package can be imported (`import 'pkg/x.js'`). -Custom subpath aliasing and encapsulation can be provided through the -`"exports"` field. +When using the `"exports"` field, custom subpaths can be defined along +with the main entry point by treating the main entry point as the +`"."` subpath: ```js -// ./node_modules/es-module-package/package.json { + "main": "./main.js", "exports": { + ".": "./main.js", "./submodule": "./src/submodule.js" } } ``` +Now only the defined subpath in `"exports"` can be imported by a +consumer: + ```js import submodule from 'es-module-package/submodule'; // Loads ./node_modules/es-module-package/src/submodule.js ``` -In addition to defining an alias, subpaths not defined by `"exports"` will -throw when an attempt is made to import them: +While other subpaths will error: ```js import submodule from 'es-module-package/private-module.js'; -// Throws ERR_MODULE_NOT_FOUND +// Throws ERR_PACKAGE_PATH_NOT_EXPORTED ``` -> Note: this is not a strong encapsulation as any private modules can still be -> loaded by absolute paths. - -Folders can also be mapped with package exports: +Entire folders can also be mapped with package exports: ```js @@ -275,20 +277,23 @@ Folders can also be mapped with package exports: } ``` +With the above, all modules within the `./src/features/` folder +are exposed deeply to `import` and `require`: + ```js import feature from 'es-module-package/features/x.js'; // Loads ./node_modules/es-module-package/src/features/x.js ``` -If a package has no exports, setting `"exports": false` can be used instead of -`"exports": {}` to indicate the package does not intend for submodules to be -exposed. +When using folder mappings, ensure that you do want to expose every +module inside the subfolder. Any modules which are not public +should be moved to another folder to retain the encapsulation +benefits of exports. -Any invalid exports entries will be ignored. This includes exports not -starting with `"./"` or a missing trailing `"/"` for directory exports. +#### Package Exports Fallbacks -Array fallback support is provided for exports, similarly to import maps -in order to be forwards-compatible with possible fallback workflows in future: +For possible new specifier support in future, array fallbacks are +supported for all invalid specifiers: ```js @@ -299,143 +304,127 @@ in order to be forwards-compatible with possible fallback workflows in future: } ``` -Since `"not:valid"` is not a supported target, `"./submodule.js"` is used +Since `"not:valid"` is not a valid specifier, `"./submodule.js"` is used instead as the fallback, as if it were the only target. -Defining a `"."` export will define the main entry point for the package, -and will always take precedence over the `"main"` field in the `package.json`. +#### Exports Sugar + +If the `"."` export is the only export, the `"exports"` field provides sugar +for this case being the direct `"exports"` field value. -This allows defining a different entry point for Node.js versions that support -ECMAScript modules and versions that don't, for example: +If the `"."` export has a fallback array or string value, then the `"exports"` +field can be set to this value directly. ```js { - "main": "./main-legacy.cjs", "exports": { - ".": "./main-modern.cjs" + ".": "./main.js" } } ``` +can be written: + + +```js +{ + "exports": "./main.js" +} +``` + #### Conditional Exports Conditional exports provide a way to map to different paths depending on certain conditions. They are supported for both CommonJS and ES module imports. For example, a package that wants to provide different ES module exports for -Node.js and the browser can be written: +`require()` and `import` can be written: ```js -// ./node_modules/pkg/package.json +// package.json { - "type": "module", - "main": "./index.js", + "main": "./main-require.cjs", "exports": { - "./feature": { - "import": "./feature-default.js", - "browser": "./feature-browser.js" - } - } + "import": "./main-module.js", + "require": "./main-require.cjs" + }, + "type": "module" } ``` -When resolving the `"."` export, if no matching target is found, the `"main"` -will be used as the final fallback. +Node.js supports the following conditions: -The conditions supported in Node.js condition matching: - -* `"default"` - the generic fallback that will always match. Can be a CommonJS - or ES module file. * `"import"` - matched when the package is loaded via `import` or - `import()`. Can be any module format, this field does not set the type - interpretation. -* `"node"` - matched for any Node.js environment. Can be a CommonJS or ES - module file. + `import()`. Can reference either an ES module or CommonJS file, as both + `import` and `import()` can load either ES module or CommonJS sources. * `"require"` - matched when the package is loaded via `require()`. + As `require()` only supports CommonJS, the referenced file must be CommonJS. +* `"node"` - matched for any Node.js environment. Can be a CommonJS or ES + module file. _This condition should always come after `"import"` or + `"require"`._ +* `"default"` - the generic fallback that will always match. Can be a CommonJS + or ES module file. _This condition should always come last._ Condition matching is applied in object order from first to last within the -`"exports"` object. - -Using the `"require"` condition it is possible to define a package that will -have a different exported value for CommonJS and ES modules, which can be a -hazard in that it can result in having two separate instances of the same -package in use in an application, which can cause a number of bugs. +`"exports"` object. _The general rule is that conditions should be used +from most specific to least specific in object order._ Other conditions such as `"browser"`, `"electron"`, `"deno"`, `"react-native"`, -etc. could be defined in other runtimes or tools. Condition names must not start -with `"."` or be numbers. Further restrictions, definitions or guidance on -condition names may be provided in future. +etc. are ignored by Node.js but may be used by other runtimes or tools. +Further restrictions, definitions or guidance on condition names may be +provided in the future. -#### Exports Sugar +Using the `"import"` and `"require"` conditions can lead to some hazards, +which are explained further in +[the dual CommonJS/ES module packages section][]. -If the `"."` export is the only export, the `"exports"` field provides sugar -for this case being the direct `"exports"` field value. - -If the `"."` export has a fallback array or string value, then the `"exports"` -field can be set to this value directly. +Conditional exports can also be extended to exports subpaths, for example: ```js { + "main": "./main.js", "exports": { - ".": "./main.js" + ".": "./main.js", + "./feature": { + "browser": "./feature-browser.js", + "default": "./feature.js" + } } } ``` -can be written: - - -```js -{ - "exports": "./main.js" -} -``` +Defines a package where `require('pkg/feature')` and `import 'pkg/feature'` +could provide different implementations between the browser and Node.js, +given third-party tool support for a `"browser"` condition. -When using [Conditional Exports][], the rule is that all keys in the object -mapping must not start with a `"."` otherwise they would be indistinguishable -from exports subpaths. +#### Nested conditions - -```js -{ - "exports": { - ".": { - "import": "./main.js", - "require": "./main.cjs" - } - } -} -``` +In addition to direct mappings, Node.js also supports nested condition objects. -can be written: +For example, to define a package that only has dual mode entry points for +use in Node.js but not the browser: ```js { + "main": "./main.js", "exports": { - "import": "./main.js", - "require": "./main.cjs" + "browser": "./feature-browser.mjs", + "node": { + "import": "./feature-node.mjs", + "require": "./feature-node.cjs" + } } } ``` -If writing any exports value that mixes up these two forms, an error will be -thrown: - - -```js -{ - // Throws on resolution! - "exports": { - "./feature": "./lib/feature.js", - "import": "./main.js", - "require": "./main.cjs" - } -} -``` +Conditions continue to be matched in order as with flat conditions. If +a nested conditional does not have any mapping it will continue checking +the remaining conditions of the parent condition. In this way nested +conditions behave analogously to nested JavaScript `if` statements. #### Self-referencing a package using its name @@ -567,8 +556,8 @@ CommonJS entry point for `require`. "type": "module", "main": "./index.cjs", "exports": { - "require": "./index.cjs", - "import": "./wrapper.mjs" + "import": "./wrapper.mjs", + "require": "./index.cjs" } } ``` @@ -912,8 +901,8 @@ can either be an URL-style relative path like `'./file.mjs'` or a package name like `'fs'`. Like in CommonJS, files within packages can be accessed by appending a path to -the package name; unless the package’s `package.json` contains an [`"exports"` -field][], in which case files within packages need to be accessed via the path +the package name; unless the package’s `package.json` contains an `"exports"` +field, in which case files within packages need to be accessed via the path defined in `"exports"`. ```js @@ -1637,7 +1626,7 @@ The resolver can throw the following errors: **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_, _env_) -> 1.If _target_ is a String, then +> 1. If _target_ is a String, then > 1. If _target_ does not start with _"./"_ or contains any _"node_modules"_ > segments including _"node_modules"_ percent-encoding, throw an > _Invalid Package Target_ error. @@ -1738,10 +1727,8 @@ success! [ECMAScript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md [ES Module Integration Proposal for Web Assembly]: https://github.com/webassembly/esm-integration [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md -[Package Exports]: #esm_package_exports [Terminology]: #esm_terminology [WHATWG JSON modules specification]: https://html.spec.whatwg.org/#creating-a-json-module-script -[`"exports"` field]: #esm_package_exports [`data:` URLs]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs [`esm`]: https://github.com/standard-things/esm#readme [`export`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export @@ -1756,6 +1743,7 @@ success! [import an ES or CommonJS module for its side effects only]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Import_a_module_for_its_side_effects_only [special scheme]: https://url.spec.whatwg.org/#special-scheme [the official standard format]: https://tc39.github.io/ecma262/#sec-modules +[the dual CommonJS/ES module packages section]: #esm_dual_commonjs_es_module_packages [transpiler loader example]: #esm_transpiler_loader [6.1.7 Array Index]: https://tc39.es/ecma262/#integer-index [Top-Level Await]: https://github.com/tc39/proposal-top-level-await From fce87a585074172c4675832c682a608391689cb1 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Thu, 2 Apr 2020 00:27:41 -0400 Subject: [PATCH 12/12] doc: esm spec bug --- doc/api/esm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 8114aa6914f1b1..3b2d859b190ea6 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1663,7 +1663,7 @@ The resolver can throw the following errors: **ESM_FORMAT**(_url_) -> 1. Assert: _url_ corresponds to an existing file pathname. +> 1. Assert: _url_ corresponds to an existing file. > 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_). > 1. If _url_ ends in _".mjs"_, then > 1. Return _"module"_.