From f9e0170f3a88a1068ea2c4d68ca19cfab9473436 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 24 Jul 2019 14:51:00 -0700 Subject: [PATCH] Revert "module: implement "exports" proposal for CommonJS" This reverts commit dcb692918348986945cbd4fbce8158b2b97a8418. --- doc/api/errors.md | 7 -- doc/api/modules.md | 33 ------ lib/internal/errors.js | 2 - lib/internal/modules/cjs/loader.js | 105 ++---------------- src/module_wrap.cc | 2 +- src/node_errors.h | 1 - src/node_file.cc | 4 +- src/node_options.cc | 1 - test/es-module/test-esm-exports.mjs | 5 +- .../node_modules/pkgexports/package.json | 2 - .../fixtures/node_modules/pkgexports/sp ce.js | 3 - test/fixtures/pkgexports.mjs | 1 - test/parallel/test-module-package-exports.js | 47 -------- 13 files changed, 13 insertions(+), 200 deletions(-) delete mode 100644 test/fixtures/node_modules/pkgexports/sp ce.js delete mode 100644 test/parallel/test-module-package-exports.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 902f5f5c8ea93a..f638f0ba905db6 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1585,13 +1585,6 @@ compiled with ICU support. A given value is out of the accepted range. - -### ERR_PATH_NOT_EXPORTED - -> Stability: 1 - Experimental - -An attempt was made to load a protected path from a package using `exports`. - ### ERR_REQUIRE_ESM diff --git a/doc/api/modules.md b/doc/api/modules.md index abac7eaf91d854..73771f49af6639 100644 --- a/doc/api/modules.md +++ b/doc/api/modules.md @@ -202,39 +202,6 @@ NODE_MODULES_PATHS(START) 5. return DIRS ``` -If `--experimental-exports` is enabled, -node allows packages loaded via `LOAD_NODE_MODULES` to explicitly declare -which filepaths to expose and how they should be interpreted. -This expands on the control packages already had using the `main` field. -With this feature enabled, the `LOAD_NODE_MODULES` changes as follows: - -```txt -LOAD_NODE_MODULES(X, START) -1. let DIRS = NODE_MODULES_PATHS(START) -2. for each DIR in DIRS: - a. let FILE_PATH = RESOLVE_BARE_SPECIFIER(DIR, X) - a. LOAD_AS_FILE(FILE_PATH) - b. LOAD_AS_DIRECTORY(FILE_PATH) - -RESOLVE_BARE_SPECIFIER(DIR, X) -1. Try to interpret X as a combination of name and subpath where the name - may have a @scope/ prefix and the subpath begins with a slash (`/`). -2. If X matches this pattern and DIR/name/package.json is a file: - a. Parse DIR/name/package.json, and look for "exports" field. - b. If "exports" is null or undefined, GOTO 3. - c. Find the longest key in "exports" that the subpath starts with. - d. If no such key can be found, throw "not exported". - e. If the key matches the subpath entirely, return DIR/name/${exports[key]}. - f. If either the key or exports[key] do not end with a slash (`/`), - throw "not exported". - g. Return DIR/name/${exports[key]}${subpath.slice(key.length)}. -3. return DIR/X -``` - -`"exports"` is only honored when loading a package "name" as defined above. Any -`"exports"` values within nested directories and packages must be declared by -the `package.json` responsible for the "name". - ## Caching diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 64e5a2773b9aaf..c36f34eff40cab 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1098,8 +1098,6 @@ E('ERR_OUT_OF_RANGE', msg += ` It must be ${range}. Received ${received}`; return msg; }, RangeError); -E('ERR_PATH_NOT_EXPORTED', - 'Package exports for \'%s\' do not define a \'%s\' subpath', Error); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index c769ce535a8338..199405b0e24457 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -21,13 +21,7 @@ 'use strict'; -const { - JSON, - Object, - Reflect, - SafeMap, - StringPrototype, -} = primordials; +const { JSON, Object, Reflect } = primordials; const { NativeModule } = require('internal/bootstrap/loaders'); const { pathToFileURL, fileURLToPath, URL } = require('internal/url'); @@ -59,12 +53,10 @@ const { compileFunction } = internalBinding('contextify'); const { ERR_INVALID_ARG_VALUE, ERR_INVALID_OPT_VALUE, - ERR_PATH_NOT_EXPORTED, ERR_REQUIRE_ESM } = require('internal/errors').codes; const { validateString } = require('internal/validators'); const pendingDeprecation = getOptionValue('--pending-deprecation'); -const experimentalExports = getOptionValue('--experimental-exports'); module.exports = { wrapSafe, Module }; @@ -190,10 +182,12 @@ Module._debug = deprecate(debug, 'Module._debug is deprecated.', 'DEP0077'); // Check if the directory is a package.json dir. const packageMainCache = Object.create(null); -// Explicit exports from package.json files -const packageExportsCache = new SafeMap(); -function readPackageRaw(requestPath) { +function readPackage(requestPath) { + const entry = packageMainCache[requestPath]; + if (entry) + return entry; + const jsonPath = path.resolve(requestPath, 'package.json'); const json = internalModuleReadJSON(path.toNamespacedPath(jsonPath)); @@ -207,12 +201,7 @@ function readPackageRaw(requestPath) { } try { - const parsed = JSON.parse(json); - packageMainCache[requestPath] = parsed.main; - if (experimentalExports) { - packageExportsCache.set(requestPath, parsed.exports); - } - return parsed; + return packageMainCache[requestPath] = JSON.parse(json).main; } catch (e) { e.path = jsonPath; e.message = 'Error parsing ' + jsonPath + ': ' + e.message; @@ -220,31 +209,6 @@ function readPackageRaw(requestPath) { } } -function readPackage(requestPath) { - const entry = packageMainCache[requestPath]; - if (entry) - return entry; - - const pkg = readPackageRaw(requestPath); - if (pkg === false) return false; - - return pkg.main; -} - -function readExports(requestPath) { - if (packageExportsCache.has(requestPath)) { - return packageExportsCache.get(requestPath); - } - - const pkg = readPackageRaw(requestPath); - if (!pkg) { - packageExportsCache.set(requestPath, null); - return null; - } - - return pkg.exports; -} - function tryPackage(requestPath, exts, isMain, originalPath) { const pkg = readPackage(requestPath); @@ -333,59 +297,8 @@ function findLongestRegisteredExtension(filename) { return '.js'; } -// This only applies to requests of a specific form: -// 1. name/.* -// 2. @scope/name/.* -const EXPORTS_PATTERN = /^((?:@[^./@\\][^/@\\]*\/)?[^@./\\][^/\\]*)(\/.*)$/; -function resolveExports(nmPath, request, absoluteRequest) { - // The implementation's behavior is meant to mirror resolution in ESM. - if (experimentalExports && !absoluteRequest) { - const [, name, expansion] = - StringPrototype.match(request, EXPORTS_PATTERN) || []; - if (!name) { - return path.resolve(nmPath, request); - } - - const basePath = path.resolve(nmPath, name); - const pkgExports = readExports(basePath); - - if (pkgExports != null) { - const mappingKey = `.${expansion}`; - const mapping = pkgExports[mappingKey]; - if (typeof mapping === 'string') { - return fileURLToPath(new URL(mapping, `${pathToFileURL(basePath)}/`)); - } - - let dirMatch = ''; - for (const [candidateKey, candidateValue] of Object.entries(pkgExports)) { - if (candidateKey[candidateKey.length - 1] !== '/') continue; - if (candidateValue[candidateValue.length - 1] !== '/') continue; - if (candidateKey.length > dirMatch.length && - StringPrototype.startsWith(mappingKey, candidateKey)) { - dirMatch = candidateKey; - } - } - - if (dirMatch !== '') { - const dirMapping = pkgExports[dirMatch]; - const remainder = StringPrototype.slice(mappingKey, dirMatch.length); - const expectedPrefix = - new URL(dirMapping, `${pathToFileURL(basePath)}/`); - const resolved = new URL(remainder, expectedPrefix).href; - if (StringPrototype.startsWith(resolved, expectedPrefix.href)) { - return fileURLToPath(resolved); - } - } - throw new ERR_PATH_NOT_EXPORTED(basePath, mappingKey); - } - } - - return path.resolve(nmPath, request); -} - Module._findPath = function(request, paths, isMain) { - const absoluteRequest = path.isAbsolute(request); - if (absoluteRequest) { + if (path.isAbsolute(request)) { paths = ['']; } else if (!paths || paths.length === 0) { return false; @@ -409,7 +322,7 @@ Module._findPath = function(request, paths, isMain) { // Don't search further if path doesn't exist const curPath = paths[i]; if (curPath && stat(curPath) < 1) continue; - var basePath = resolveExports(curPath, request, absoluteRequest); + var basePath = path.resolve(curPath, request); var filename; var rc = stat(basePath); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 503ca8a858e2b9..c13ab3b5ed21ae 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -856,7 +856,7 @@ Maybe PackageExportsResolve(Environment* env, std::string msg = "Package exports for '" + URL(".", pjson_url).ToFilePath() + "' do not define a '" + pkg_subpath + "' subpath, imported from " + base.ToFilePath(); - node::THROW_ERR_PATH_NOT_EXPORTED(env, msg.c_str()); + node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str()); return Nothing(); } diff --git a/src/node_errors.h b/src/node_errors.h index c2587d73e67df4..939f93a4899f59 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -53,7 +53,6 @@ void PrintErrorString(const char* format, ...); V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \ V(ERR_MODULE_NOT_FOUND, Error) \ V(ERR_OUT_OF_RANGE, RangeError) \ - V(ERR_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/src/node_file.cc b/src/node_file.cc index e11aa9054640a9..564c63bad73b23 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -872,9 +872,7 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo& args) { } const size_t size = offset - start; - if (size == 0 || ( - size == SearchString(&chars[start], size, "\"main\"") && - size == SearchString(&chars[start], size, "\"exports\""))) { + if (size == 0 || size == SearchString(&chars[start], size, "\"main\"")) { return; } else { Local chars_string = diff --git a/src/node_options.cc b/src/node_options.cc index eaa3e7d049b44b..9da1ed5fb81d67 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -319,7 +319,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "experimental ES Module support and caching modules", &EnvironmentOptions::experimental_modules, kAllowedInEnvironment); - Implies("--experimental-modules", "--experimental-exports"); AddOption("--experimental-wasm-modules", "experimental ES Module support for webassembly modules", &EnvironmentOptions::experimental_wasm_modules, diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index 38cd81511fe8ef..88115026654726 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -1,9 +1,9 @@ -// Flags: --experimental-modules +// Flags: --experimental-modules --experimental-exports import { mustCall } from '../common/index.mjs'; import { ok, strictEqual } from 'assert'; -import { asdf, asdf2, space } from '../fixtures/pkgexports.mjs'; +import { asdf, asdf2 } from '../fixtures/pkgexports.mjs'; import { loadMissing, loadFromNumber, @@ -12,7 +12,6 @@ import { strictEqual(asdf, 'asdf'); strictEqual(asdf2, 'asdf'); -strictEqual(space, 'encoded path'); loadMissing().catch(mustCall((err) => { ok(err.message.toString().startsWith('Package exports')); diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index b0c8867bb46fd1..51c596ed8673ab 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -1,9 +1,7 @@ { "exports": { ".": "./asdf.js", - "./space": "./sp%20ce.js", "./asdf": "./asdf.js", - "./valid-cjs": "./asdf.js", "./sub/": "./" } } diff --git a/test/fixtures/node_modules/pkgexports/sp ce.js b/test/fixtures/node_modules/pkgexports/sp ce.js deleted file mode 100644 index 570237506e4586..00000000000000 --- a/test/fixtures/node_modules/pkgexports/sp ce.js +++ /dev/null @@ -1,3 +0,0 @@ -'use strict'; - -module.exports = 'encoded path'; diff --git a/test/fixtures/pkgexports.mjs b/test/fixtures/pkgexports.mjs index 8907ebcb0e253a..4d82ba0560ef11 100644 --- a/test/fixtures/pkgexports.mjs +++ b/test/fixtures/pkgexports.mjs @@ -1,3 +1,2 @@ export { default as asdf } from 'pkgexports/asdf'; export { default as asdf2 } from 'pkgexports/sub/asdf.js'; -export { default as space } from 'pkgexports/space'; diff --git a/test/parallel/test-module-package-exports.js b/test/parallel/test-module-package-exports.js deleted file mode 100644 index a1b9879448c17b..00000000000000 --- a/test/parallel/test-module-package-exports.js +++ /dev/null @@ -1,47 +0,0 @@ -// Flags: --experimental-exports -'use strict'; - -require('../common'); - -const assert = require('assert'); -const { createRequire } = require('module'); -const path = require('path'); - -const fixtureRequire = - createRequire(path.resolve(__dirname, '../fixtures/imaginary.js')); - -assert.strictEqual(fixtureRequire('pkgexports/valid-cjs'), 'asdf'); - -assert.strictEqual(fixtureRequire('baz/index'), 'eye catcher'); - -assert.strictEqual(fixtureRequire('pkgexports/sub/asdf.js'), 'asdf'); - -assert.strictEqual(fixtureRequire('pkgexports/space'), 'encoded path'); - -assert.throws( - () => fixtureRequire('pkgexports/not-a-known-entry'), - (e) => { - assert.strictEqual(e.code, 'ERR_PATH_NOT_EXPORTED'); - return true; - }); - -assert.throws( - () => fixtureRequire('pkgexports-number/hidden.js'), - (e) => { - assert.strictEqual(e.code, 'ERR_PATH_NOT_EXPORTED'); - return true; - }); - -assert.throws( - () => fixtureRequire('pkgexports/sub/not-a-file.js'), - (e) => { - assert.strictEqual(e.code, 'MODULE_NOT_FOUND'); - return true; - }); - -assert.throws( - () => fixtureRequire('pkgexports/sub/./../asdf.js'), - (e) => { - assert.strictEqual(e.code, 'ERR_PATH_NOT_EXPORTED'); - return true; - });