module: conditional exports condition renaming proposal for usability

doc/api/esm.md

@@ -329,8 +329,8 @@ Node.js and the browser can be written:
   "main": "./index.js",
   "exports": {
     "./feature": {
-      "browser": "./feature-browser.js",
-      "default": "./feature-default.js"
+      "import": "./feature-default.js",
+      "browser": "./feature-browser.js"
     }
   }
 }
@@ -341,16 +341,24 @@ will be used as the final fallback.
 
 The conditions supported in Node.js are matched in the following order:
 
-1. `"require"` - matched when the package is loaded via `require()`.
-  _This is currently only supported behind the
-  `--experimental-conditional-exports` flag._
-2. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
+1. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
    module file. _This is currently only supported behind the
-  `--experimental-conditional-exports` flag._
-3. `"default"` - the generic fallback that will always match if no other
+   `--experimental-conditional-exports` flag._
+2. `"require"` - matched when the package is loaded via `require()`.
+   _This is currently only supported behind the
+   `--experimental-conditional-exports` flag._
+3. `"import"` - matched when the package is loaded via `import` or
+   `import()`. Can be any module format, this field does not set the type
+   interpretation. _This is currently only supported behind the
+   `--experimental-conditional-exports` flag._
+4. `"default"` - the generic fallback that will always match if no other
    more specific condition is matched first. Can be a CommonJS or ES module
    file.
 
+> Setting any of the above flagged conditions for a published package is not
+> recommended until they are unflagged to avoid breaking changes to packages in
+> future.
+
 Using the `"require"` condition it is possible to define a package that will
 have a different exported value for CommonJS and ES modules, which can be a
 hazard in that it can result in having two separate instances of the same
@@ -394,8 +402,8 @@ from exports subpaths.
 {
   "exports": {
     ".": {
-      "require": "./main.cjs",
-      "default": "./main.js"
+      "import": "./main.js",
+      "require": "./main.cjs"
     }
   }
 }
@@ -407,8 +415,8 @@ can be written:
 ```js
 {
   "exports": {
-    "require": "./main.cjs",
-    "default": "./main.js"
+    "import": "./main.js",
+    "require": "./main.cjs"
   }
 }
 ```
@@ -422,8 +430,8 @@ thrown:
   // Throws on resolution!
   "exports": {
     "./feature": "./lib/feature.js",
-    "require": "./main.cjs",
-    "default": "./main.js"
+    "import": "./main.js",
+    "require": "./main.cjs"
   }
 }
 ```
@@ -508,9 +516,8 @@ ES module wrapper is used for `import` and the CommonJS entry point for
 `require`.
 
 > Note: While `--experimental-conditional-exports` is flagged, a package
-> using this pattern will throw when loaded via `require()` in modern
-> Node.js, unless package consumers use the `--experimental-conditional-exports`
-> flag.
+> using this pattern will throw when loaded unless package consumers use the
+> `--experimental-conditional-exports` flag.
 
 <!-- eslint-skip -->
 ```js
@@ -520,7 +527,7 @@ ES module wrapper is used for `import` and the CommonJS entry point for
   "main": "./index.cjs",
   "exports": {
     "require": "./index.cjs",
-    "default": "./wrapper.mjs"
+    "import": "./wrapper.mjs"
   }
 }
 ```
@@ -605,8 +612,8 @@ CommonJS and ES module entry points directly (requires
   "type": "module",
   "main": "./index.cjs",
   "exports": {
-    "require": "./index.cjs",
-    "default": "./index.mjs"
+    "import": "./index.mjs",
+    "require": "./index.cjs"
   }
 }
 ```
@@ -1149,7 +1156,7 @@ of these top-level routines unless stated otherwise.
 _isMain_ is **true** when resolving the Node.js application entry point.
 
 _defaultEnv_ is the conditional environment name priority array,
-`["node", "default"]`.
+`["node", "import"]`.
 
 <details>
 <summary>Resolver algorithm specification</summary>

doc/api/modules.md

@@ -241,7 +241,8 @@ RESOLVE_BARE_SPECIFIER(DIR, X)
    g. If no such key can be found, throw "not found".
    h. let RESOLVED_URL =
         PACKAGE_EXPORTS_TARGET_RESOLVE(pathToFileURL(DIR/name), exports[key],
-        subpath.slice(key.length)), as defined in the ESM resolver.
+        subpath.slice(key.length), ["node", "require"]), as defined in the ESM
+        resolver.
    i. return fileURLToPath(RESOLVED_URL)
 3. return DIR/X
 ```

lib/internal/modules/cjs/loader.js

@@ -585,9 +585,9 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
     }
   } else if (typeof target === 'object' && target !== null) {
     if (experimentalConditionalExports &&
-        ObjectPrototypeHasOwnProperty(target, 'require')) {
+        ObjectPrototypeHasOwnProperty(target, 'node')) {
       try {
-        const result = resolveExportsTarget(pkgPath, target.require, subpath,
+        const result = resolveExportsTarget(pkgPath, target.node, subpath,
                                             basePath, mappingKey);
         emitExperimentalWarning('Conditional exports');
         return result;
@@ -596,9 +596,9 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
       }
     }
     if (experimentalConditionalExports &&
-        ObjectPrototypeHasOwnProperty(target, 'node')) {
+        ObjectPrototypeHasOwnProperty(target, 'require')) {
       try {
-        const result = resolveExportsTarget(pkgPath, target.node, subpath,
+        const result = resolveExportsTarget(pkgPath, target.require, subpath,
                                             basePath, mappingKey);
         emitExperimentalWarning('Conditional exports');
         return result;

src/env.h

@@ -255,6 +255,7 @@ constexpr size_t kFsStatsBufferLength =
   V(hostmaster_string, "hostmaster")                                           \
   V(http_1_1_string, "http/1.1")                                               \
   V(ignore_string, "ignore")                                                   \
+  V(import_string, "import")                                                   \
   V(infoaccess_string, "infoAccess")                                           \
   V(inherit_string, "inherit")                                                 \
   V(input_string, "input")                                                     \

src/module_wrap.cc

@@ -967,6 +967,17 @@ Maybe<URL> ResolveExportsTarget(Environment* env,
         return resolved;
       }
     }
+    if (env->options()->experimental_conditional_exports &&
+        target_obj->HasOwnProperty(context, env->import_string()).FromJust()) {
+      matched = true;
+      conditionalTarget =
+          target_obj->Get(context, env->import_string()).ToLocalChecked();
+      Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
+            conditionalTarget, subpath, pkg_subpath, base, false);
+      if (!resolved.IsNothing()) {
+        return resolved;
+      }
+    }
     if (target_obj->HasOwnProperty(context, env->default_string()).FromJust()) {
       matched = true;
       conditionalTarget =

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

@@ -31,7 +31,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
     ['pkgexports-sugar', { default: 'main' }],
     // Conditional object exports sugar
     ['pkgexports-sugar2', isRequire ? { default: 'not-exported' } :
-      { default: 'main' }]
+      { default: 'main' }],
   ]);
 
   for (const [validSpecifier, expected] of validSpecifiers) {
@@ -51,7 +51,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
     ['pkgexports-number/hidden.js', './hidden.js'],
     // Sugar cases still encapsulate
     ['pkgexports-sugar/not-exported.js', './not-exported.js'],
-    ['pkgexports-sugar2/not-exported.js', './not-exported.js']
+    ['pkgexports-sugar2/not-exported.js', './not-exported.js'],
   ]);
 
   const invalidExports = new Map([
@@ -97,6 +97,15 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
     }));
   }
 
+  // Conditional export, even with no match, should still be used instead
+  // of falling back to main
+  if (isRequire) {
+    loadFixture('pkgexports-main').catch(mustCall((err) => {
+      strictEqual(err.code, 'MODULE_NOT_FOUND');
+      assertStartsWith(err.message, 'No valid export');
+    }));
+  }
+
   // Covering out bases - not a file is still not a file after dir mapping.
   loadFixture('pkgexports/sub/not-a-file.js').catch(mustCall((err) => {
     strictEqual(err.code, (isRequire ? '' : 'ERR_') + 'MODULE_NOT_FOUND');