module: unify package exports test for CJS and ESM

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/common/index.mjs

@@ -1,12 +1,7 @@
 // Flags: --experimental-modules
 /* eslint-disable node-core/require-common-first, node-core/required-modules */
 
-import { createRequireFromPath } from 'module';
-import { fileURLToPath as toPath } from 'url';
-
-function createRequire(metaUrl) {
-  return createRequireFromPath(toPath(metaUrl));
-}
+import { createRequire } from 'module';
 
 const require = createRequire(import.meta.url);
 const common = require('./index.js');

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 { asdf, asdf2, space } from '../fixtures/pkgexports.mjs';
-import {
-  loadMissing,
-  loadFromNumber,
-  loadDot,
-} from '../fixtures/pkgexports-missing.mjs';
-
-strictEqual(asdf, 'asdf');
-strictEqual(asdf2, 'asdf');
-strictEqual(space, 'encoded path');
-
-loadMissing().catch(mustCall((err) => {
-  ok(err.message.toString().startsWith('Package exports'));
-  ok(err.message.toString().indexOf('do not define a \'./missing\' subpath'));
-}));
-
-loadFromNumber().catch(mustCall((err) => {
-  ok(err.message.toString().startsWith('Package exports'));
-  ok(err.message.toString().indexOf('do not define a \'./missing\' subpath'));
-}));
-
-loadDot().catch(mustCall((err) => {
-  ok(err.message.toString().startsWith('Cannot find main entry point'));
-}));
+import { ok, deepStrictEqual, strictEqual } from 'assert';
+
+import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
+
+[requireFixture, importFixture].forEach((loadFixture) => {
+  const isRequire = loadFixture === requireFixture;
+
+  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;
+
+    loadFixture(validSpecifier)
+      .then(mustCall((actual) => {
+        deepStrictEqual({ ...actual }, expected);
+      }));
+  }
+
+  // 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/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/message/async_error_sync_esm.out

@@ -5,4 +5,3 @@ Error: test
     at async three (*fixtures*async-error.js:20:3)
     at async four (*fixtures*async-error.js:24:3)
     at async main (*message*async_error_sync_esm.mjs:7:5)
-(node:*) [DEP0130] DeprecationWarning: Module.createRequireFromPath() is deprecated. Use Module.createRequire() instead.