From dff8c86d2999cd50e49580329aee7a29ce2a54a3 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 13 Apr 2020 22:36:15 -0700 Subject: [PATCH 1/2] module: empty fallback and null exports as not exported --- doc/api/esm.md | 4 +++- lib/internal/modules/cjs/loader.js | 14 +++++++++----- lib/internal/modules/esm/resolve.js | 7 +++++-- test/es-module/test-esm-exports.mjs | 8 +++++--- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 3d7882c98d9302..3245b0aa2edb20 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1661,13 +1661,15 @@ The resolver can throw the following errors: > 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. If _target.length is zero, throw a _Package Path Not Exported_ error. > 1. For each item _targetValue_ in _target_, do > 1. If _targetValue_ is an Array, continue the loop. > 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, if _target_ is _null_, throw a _Package Path Not Exported_ +> error. > 1. Otherwise throw an _Invalid Package Target_ error. **ESM_FORMAT**(_url_) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 96aa4e2ebfc1aa..a4247d07b16efe 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -552,21 +552,22 @@ function resolveExportsTarget(baseUrl, target, subpath, mappingKey) { , 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); + throw new ERR_PACKAGE_PATH_NOT_EXPORTED( + StringPrototypeSlice(baseUrl.pathname, 0, -1), mappingKey + subpath); + let lastException; for (const targetValue of target) { try { return resolveExportsTarget(baseUrl, targetValue, subpath, mappingKey); } catch (e) { + lastException = 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); + assert(lastException !== undefined); + throw lastException; } else if (typeof target === 'object' && target !== null) { const keys = ObjectKeys(target); if (keys.some(isArrayIndex)) { @@ -595,6 +596,9 @@ function resolveExportsTarget(baseUrl, target, subpath, mappingKey) { } throw new ERR_PACKAGE_PATH_NOT_EXPORTED( StringPrototypeSlice(baseUrl.pathname, 0, -1), mappingKey + subpath); + } else if (target === null) { + throw new ERR_PACKAGE_PATH_NOT_EXPORTED( + StringPrototypeSlice(baseUrl.pathname, 0, -1), mappingKey + subpath); } throw new ERR_INVALID_PACKAGE_TARGET( StringPrototypeSlice(baseUrl.pathname, 0, -1), mappingKey, subpath, target); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 04c6abe54269f6..5bb2f37e53eeb5 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -16,7 +16,7 @@ const { StringPrototypeStartsWith, StringPrototypeSubstr, } = primordials; - +const assert = require('internal/assert'); const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { @@ -345,7 +345,7 @@ function resolveExportsTarget( return finalizeResolution(resolved, base); } else if (ArrayIsArray(target)) { if (target.length === 0) - throwExportsInvalid(packageSubpath, target, packageJSONUrl, base); + throwExportsNotFound(packageSubpath, packageJSONUrl, base); let lastException; for (let i = 0; i < target.length; i++) { @@ -366,6 +366,7 @@ function resolveExportsTarget( return finalizeResolution(resolved, base); } + assert(lastException !== undefined); throw lastException; } else if (typeof target === 'object' && target !== null) { const keys = ObjectGetOwnPropertyNames(target); @@ -392,6 +393,8 @@ function resolveExportsTarget( } } throwExportsNotFound(packageSubpath, packageJSONUrl, base); + } else if (target === null) { + throwExportsNotFound(packageSubpath, packageJSONUrl, base); } throwExportsInvalid(packageSubpath, target, packageJSONUrl, base); } diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index f71e2172951843..2245358bcba19e 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -64,6 +64,10 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; // Conditional exports with no match are "not exported" errors ['pkgexports/invalid1', './invalid1'], ['pkgexports/invalid4', './invalid4'], + // Null mapping + ['pkgexports/null', './null'], + // Empty fallback + ['pkgexports/nofallback1', './nofallback1'], ]); const invalidExports = new Map([ @@ -74,12 +78,10 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/belowdir/pkgexports/asdf.js', './belowdir/'], // This target file steps below the package ['pkgexports/belowfile', './belowfile'], - // Invalid target handling - ['pkgexports/null', './null'], + // Invalid targets ['pkgexports/invalid2', './invalid2'], ['pkgexports/invalid3', './invalid3'], // Missing / invalid fallbacks - ['pkgexports/nofallback1', './nofallback1'], ['pkgexports/nofallback2', './nofallback2'], // Reaching into nested node_modules ['pkgexports/nodemodules', './nodemodules'], From c2c0a15be7f1d7f2ef68f403c0c01e62213cc463 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 14 Apr 2020 10:47:34 -0700 Subject: [PATCH 2/2] add test for null subpaths --- test/es-module/test-esm-exports.mjs | 1 + test/fixtures/node_modules/pkgexports/package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index 2245358bcba19e..2132a87810efdd 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -66,6 +66,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/invalid4', './invalid4'], // Null mapping ['pkgexports/null', './null'], + ['pkgexports/null/subpath', './null/subpath'], // Empty fallback ['pkgexports/nofallback1', './nofallback1'], ]); diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index f3ec20c49b2b91..f7f6ace400a630 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -9,6 +9,7 @@ "./belowfile": "../belowfile", "./missingtrailer/": ".", "./null": null, + "./null/": null, "./invalid1": {}, "./invalid2": 1234, "./invalid3": "",