module: unify package exports test for CJS and ESM

Refs: https://github.com/nodejs/modules/issues/358

PR-URL: https://github.com/nodejs/node/pull/28831
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

src/module_wrap.cc

@@ -847,8 +847,11 @@ Maybe<URL> PackageExportsResolve(Environment* env,
         std::string target(*target_utf8, target_utf8.length());
         if (target.back() == '/' && target.substr(0, 2) == "./") {
           std::string subpath = pkg_subpath.substr(best_match_str.length());
-          URL target_url(target + subpath, pjson_url);
-          return FinalizeResolution(env, target_url, base);
+          URL url_prefix(target, pjson_url);
+          URL target_url(subpath, url_prefix);
+          if (target_url.path().find(url_prefix.path()) == 0) {
+            return FinalizeResolution(env, target_url, base);
+          }
         }
       }
     }

test/es-module/test-esm-exports.mjs

@@ -1,29 +1,88 @@
 // Flags: --experimental-modules
 
 import { mustCall } from '../common/index.mjs';
-import { ok, strictEqual } from 'assert';
+import { ok, deepStrictEqual, strictEqual } from 'assert';
 
-import { asdf, asdf2, space } from '../fixtures/pkgexports.mjs';
-import {
-  loadMissing,
-  loadFromNumber,
-  loadDot,
-} from '../fixtures/pkgexports-missing.mjs';
+import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
 
-strictEqual(asdf, 'asdf');
-strictEqual(asdf2, 'asdf');
-strictEqual(space, 'encoded path');
+[requireFixture, importFixture].forEach((loadFixture) => {
+  const isRequire = loadFixture === requireFixture;
 
-loadMissing().catch(mustCall((err) => {
-  ok(err.message.toString().startsWith('Package exports'));
-  ok(err.message.toString().indexOf('do not define a \'./missing\' subpath'));
-}));
+  const validSpecifiers = new Map([
+    // A simple mapping of a path.
+    ['pkgexports/valid-cjs', { default: 'asdf' }],
+    // A directory mapping, pointing to the package root.
+    ['pkgexports/sub/asdf.js', { default: 'asdf' }],
+    // A mapping pointing to a file that needs special encoding (%20) in URLs.
+    ['pkgexports/space', { default: 'encoded path' }],
+    // Verifying that normal packages still work with exports turned on.
+    isRequire ? ['baz/index', { default: 'eye catcher' }] : [null],
+  ]);
+  for (const [validSpecifier, expected] of validSpecifiers) {
+    if (validSpecifier === null) continue;
 
-loadFromNumber().catch(mustCall((err) => {
-  ok(err.message.toString().startsWith('Package exports'));
-  ok(err.message.toString().indexOf('do not define a \'./missing\' subpath'));
-}));
+    loadFixture(validSpecifier)
+      .then(mustCall((actual) => {
+        deepStrictEqual({ ...actual }, expected);
+      }));
+  }
 
-loadDot().catch(mustCall((err) => {
-  ok(err.message.toString().startsWith('Cannot find main entry point'));
-}));
+  // There's no such export - so there's nothing to do.
+  loadFixture('pkgexports/missing').catch(mustCall((err) => {
+    strictEqual(err.code, 'ERR_PATH_NOT_EXPORTED');
+    assertStartsWith(err.message, 'Package exports');
+    assertIncludes(err.message, 'do not define a \'./missing\' subpath');
+  }));
+
+  // The file exists but isn't exported. The exports is a number which counts
+  // as a non-null value without any properties, just like `{}`.
+  loadFixture('pkgexports-number/hidden.js').catch(mustCall((err) => {
+    strictEqual(err.code, 'ERR_PATH_NOT_EXPORTED');
+    assertStartsWith(err.message, 'Package exports');
+    assertIncludes(err.message, 'do not define a \'./hidden.js\' subpath');
+  }));
+
+  // There's no main field so we won't find anything when importing the name.
+  // The fact that "." is mapped is ignored, it's not a valid main config.
+  loadFixture('pkgexports').catch(mustCall((err) => {
+    if (isRequire) {
+      strictEqual(err.code, 'MODULE_NOT_FOUND');
+      assertStartsWith(err.message, 'Cannot find module \'pkgexports\'');
+    } else {
+      strictEqual(err.code, 'ERR_MODULE_NOT_FOUND');
+      assertStartsWith(err.message, 'Cannot find main entry point');
+    }
+  }));
+
+  // Even though 'pkgexports/sub/asdf.js' works, alternate "path-like" variants
+  // do not to prevent confusion and accidental loopholes.
+  loadFixture('pkgexports/sub/./../asdf.js').catch(mustCall((err) => {
+    strictEqual(err.code, 'ERR_PATH_NOT_EXPORTED');
+    assertStartsWith(err.message, 'Package exports');
+    assertIncludes(err.message,
+                   'do not define a \'./sub/./../asdf.js\' subpath');
+  }));
+
+  // Covering out bases - not a file is still not a file after dir mapping.
+  loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => {
+    if (isRequire) {
+      strictEqual(err.code, 'MODULE_NOT_FOUND');
+      assertStartsWith(err.message,
+                       'Cannot find module \'pkgexports/sub/not-a-file.js\'');
+    } else {
+      strictEqual(err.code, 'ERR_MODULE_NOT_FOUND');
+      // ESM currently returns a full file path
+      assertStartsWith(err.message, 'Cannot find module');
+    }
+  }));
+});
+
+function assertStartsWith(actual, expected) {
+  const start = actual.toString().substr(0, expected.length);
+  strictEqual(start, expected);
+}
+
+function assertIncludes(actual, expected) {
+  ok(actual.toString().indexOf(expected),
+     `${JSON.stringify(actual)} includes ${JSON.stringify(expected)}`);
+}

test/fixtures/node_modules/pkgexports/package.json

@@ -2,7 +2,6 @@
   "exports": {
     ".": "./asdf.js",
     "./space": "./sp%20ce.js",
-    "./asdf": "./asdf.js",
     "./valid-cjs": "./asdf.js",
     "./sub/": "./"
   }

test/fixtures/pkgexports-missing.mjs

@@ -1,11 +0,0 @@
-export function loadMissing() {
-  return import('pkgexports/missing');
-}
-
-export function loadFromNumber() {
-  return import('pkgexports-number/hidden.js');
-}
-
-export function loadDot() {
-  return import('pkgexports');
-}

test/fixtures/pkgexports.mjs

@@ -1,3 +1,12 @@
-export { default as asdf } from 'pkgexports/asdf';
-export { default as asdf2 } from 'pkgexports/sub/asdf.js';
-export { default as space } from 'pkgexports/space';
+import { fileURLToPath } from 'url';
+import { createRequire } from 'module';
+
+const rawRequire = createRequire(fileURLToPath(import.meta.url));
+
+export async function requireFixture(specifier) {
+  return { default: rawRequire(specifier ) };
+}
+
+export function importFixture(specifier) {
+  return import(specifier);
+}

test/parallel/test-module-package-exports.js

@@ -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;
-  });