module: exports not exported for null resolutions

guybedford · guybedford · commit e767ed0558a1 · 2020-04-22T15:48:44.000-07:00
PR-URL: #32838 Reviewed-By: Jan Krems <jan.krems@gmail.com>
diff --git 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
@@ -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
@@ -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
@@ -64,6 +64,11 @@ 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'],
+    ['pkgexports/null/subpath', './null/subpath'],
+    // Empty fallback
+    ['pkgexports/nofallback1', './nofallback1'],
   ]);
 
   const invalidExports = new Map([
@@ -74,13 +79,11 @@ 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'],
     ['pkgexports/invalid5', 'invalid5'],
     // Missing / invalid fallbacks
-    ['pkgexports/nofallback1', './nofallback1'],
     ['pkgexports/nofallback2', './nofallback2'],
     // Reaching into nested node_modules
     ['pkgexports/nodemodules', './nodemodules'],
diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json
