[v22.x] backport unflagging of --experimental-require-module and related fixes/refactorings

doc/api/cli.md

@@ -1054,6 +1054,10 @@
 
 <!-- YAML
 added: v22.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/55085
+    description: This is now true by default.
 -->
 
 > Stability: 1.1 - Active Development
@@ -1695,6 +1699,22 @@
 
 Use this flag to disable top-level await in REPL.
 
+### `--no-experimental-require-module`
+
+<!-- YAML
+added: v22.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/55085
+    description: This is now false by default.
+-->
+
+> Stability: 1.1 - Active Development
+
+Disable support for loading a synchronous ES module graph in `require()`.
+
+See [Loading ECMAScript modules using `require()`][].
+
 ### `--no-experimental-websocket`
 
 <!-- YAML
@@ -1900,9 +1920,7 @@
 added: v22.0.0
 -->
 
-This flag is only useful when `--experimental-require-module` is enabled.
-
-If the ES module being `require()`'d contains top-level await, this flag
+If the ES module being `require()`'d contains top-level `await`, this flag
 allows Node.js to evaluate the module, try to locate the
 top-level awaits, and print their location to help users find them.
 

doc/api/errors.md

@@ -2478,8 +2478,8 @@
 
 > Stability: 1 - Experimental
 
-When trying to `require()` a [ES Module][] under `--experimental-require-module`,
-a CommonJS to ESM or ESM to CommonJS edge participates in an immediate cycle.
+When trying to `require()` a [ES Module][], a CommonJS to ESM or ESM to CommonJS edge
+participates in an immediate cycle.
 This is not allowed because ES Modules cannot be evaluated while they are
 already being evaluated.
 
@@ -2493,8 +2493,8 @@
 
 > Stability: 1 - Experimental
 
-When trying to `require()` a [ES Module][] under `--experimental-require-module`,
-the module turns out to be asynchronous. That is, it contains top-level await.
+When trying to `require()` a [ES Module][], the module turns out to be asynchronous.
+That is, it contains top-level await.
 
 To see where the top-level await is, use
 `--experimental-print-required-tla` (this would execute the modules
@@ -2504,12 +2504,20 @@
 
 ### `ERR_REQUIRE_ESM`
 
-> Stability: 1 - Experimental
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/55085
+    description: require() now supports loading synchronous ES modules by default.
+-->
+
+> Stability: 0 - Deprecated
 
 An attempt was made to `require()` an [ES Module][].
 
-To enable `require()` for synchronous module graphs (without
-top-level `await`), use `--experimental-require-module`.
+This error has been deprecated since `require()` now supports loading synchronous
+ES modules. When `require()` encounters an ES module that contains top-level
+`await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead.
 
 <a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
 
@@ -4123,6 +4131,7 @@
 [`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type
 [`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list
 [`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #err_missing_transferable_in_transfer_list
+[`ERR_REQUIRE_ASYNC_MODULE`]: #err_require_async_module
 [`EventEmitter`]: events.md#class-eventemitter
 [`MessagePort`]: worker_threads.md#class-messageport
 [`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf

doc/api/esm.md

@@ -468,7 +468,7 @@ compatibility.
 ### `require`
 
 The CommonJS module `require` currently only supports loading synchronous ES
-modules when `--experimental-require-module` is enabled.
+modules (that is, ES modules that do not use top-level `await`).
 
 See [Loading ECMAScript modules using `require()`][] for details.
 

doc/api/modules.md

@@ -171,21 +171,25 @@
 ## Loading ECMAScript modules using `require()`
 
 <!-- YAML
-added: v22.0.0
+added:
+  - v22.0.0
+  - v20.17.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/55085
+    description: This feature is no longer behind the `--experimental-require-module` CLI flag.
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/54563
+    description: Support `'module.exports'` interop export in `require(esm)`.
 -->
 
-> Stability: 1.1 - Active Development. Enable this API with the
-> [`--experimental-require-module`][] CLI flag.
+> Stability: 1.2 - Release candidate
 
 The `.mjs` extension is reserved for [ECMAScript Modules][].
-Currently, if the flag `--experimental-require-module` is not used, loading
-an ECMAScript module using `require()` will throw a [`ERR_REQUIRE_ESM`][]
-error, and users need to use [`import()`][] instead. See
-[Determining module system][] section for more info
+See [Determining module system][] section for more info
 regarding which files are parsed as ECMAScript modules.
 
-If `--experimental-require-module` is enabled, and the ECMAScript module being
-loaded by `require()` meets the following requirements:
+`require()` only supports loading ECMAScript modules that meet the following requirements:
 
 * The module is fully synchronous (contains no top-level `await`); and
 * One of these conditions are met:
@@ -194,8 +198,8 @@
   3. The file has a `.js` extension, the closest `package.json` does not contain
      `"type": "commonjs"`, and the module contains ES module syntax.
 
-`require()` will load the requested module as an ES Module, and return
-the module namespace object. In this case it is similar to dynamic
+If the ES Module being loaded meet the requirements, `require()` can load it and
+return the module namespace object. In this case it is similar to dynamic
 `import()` but is run synchronously and returns the name space object
 directly.
 
@@ -208,13 +212,12 @@
 
 ```mjs
 // point.mjs
-class Point {
+export default class Point {
   constructor(x, y) { this.x = x; this.y = y; }
 }
-export default Point;
 ```
 
-A CommonJS module can load them with `require()` under `--experimental-detect-module`:
+A CommonJS module can load them with `require()`:
 
 ```cjs
 const distance = require('./distance.mjs');
@@ -240,16 +243,81 @@
 by tools converting ES modules into CommonJS modules, following existing ecosystem
 conventions. Code authored directly in CommonJS should avoid depending on it.
 
+When a ES Module contains both named exports and a default export, the result returned by `require()`
+is the module namespace object, which places the default export in the `.default` property, similar to
+the results returned by `import()`.
+To customize what should be returned by `require(esm)` directly, the ES Module can export the
+desired value using the string name `"module.exports"`.
+
+<!-- eslint-disable @stylistic/js/semi -->
+
+```mjs
+// point.mjs
+export default class Point {
+  constructor(x, y) { this.x = x; this.y = y; }
+}
+
+// `distance` is lost to CommonJS consumers of this module, unless it's
+// added to `Point` as a static property.
+export function distance(a, b) { return (b.x - a.x) ** 2 + (b.y - a.y) ** 2; }
+export { Point as 'module.exports' }
+```
+
+<!-- eslint-disable node-core/no-duplicate-requires -->
+
+```cjs
+const Point = require('./point.mjs');
+console.log(Point); // [class Point]
+
+// Named exports are lost when 'module.exports' is used
+const { distance } = require('./point.mjs');
+console.log(distance); // undefined
+```
+
+Notice in the example above, when the `module.exports` export name is used, named exports
+will be lost to CommonJS consumers. To allow  CommonJS consumers to continue accessing
+named exports, the module can make sure that the default export is an object with the
+named exports attached to it as properties. For example with the example above,
+`distance` can be attached to the default export, the `Point` class, as a static method.
+
+<!-- eslint-disable @stylistic/js/semi -->
+
+```mjs
+export function distance(a, b) { return (b.x - a.x) ** 2 + (b.y - a.y) ** 2; }
+
+export default class Point {
+  constructor(x, y) { this.x = x; this.y = y; }
+  static distance = distance;
+}
+
+export { Point as 'module.exports' }
+```
+
+<!-- eslint-disable node-core/no-duplicate-requires -->
+
+```cjs
+const Point = require('./point.mjs');
+console.log(Point); // [class Point]
+
+const { distance } = require('./point.mjs');
+console.log(distance); // [Function: distance]
+```
+
 If the module being `require()`'d contains top-level `await`, or the module
 graph it `import`s contains top-level `await`,
 [`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
-load the asynchronous module using `import()`.
+load the asynchronous module using [`import()`][].
 
 If `--experimental-print-required-tla` is enabled, instead of throwing
 `ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
 module, try to locate the top-level awaits, and print their location to
 help users fix them.
 
+Support for loading ES modules using `require()` is currently
+experimental and can be disabled using `--no-experimental-require-module`.
+When `require()` actually encounters an ES module for the
+first time in the process, it will emit an experimental warning. The
+warning is expected to be removed when this feature stablizes.
 This feature can be detected by checking if
 [`process.features.require_module`][] is `true`.
 
@@ -282,8 +350,7 @@
 
 MAYBE_DETECT_AND_LOAD(X)
 1. If X parses as a CommonJS module, load X as a CommonJS module. STOP.
-2. Else, if `--experimental-require-module` is
-  enabled, and the source code of X can be parsed as ECMAScript module using
+2. Else, if the source code of X can be parsed as ECMAScript module using
   <a href="esm.md#resolver-algorithm-specification">DETECT_MODULE_SYNTAX defined in
   the ESM resolver</a>,
   a. Load X as an ECMAScript module. STOP.
@@ -1199,9 +1266,7 @@
 [GLOBAL_FOLDERS]: #loading-from-the-global-folders
 [`"main"`]: packages.md#main
 [`"type"`]: packages.md#type
-[`--experimental-require-module`]: cli.md#--experimental-require-module
 [`ERR_REQUIRE_ASYNC_MODULE`]: errors.md#err_require_async_module
-[`ERR_REQUIRE_ESM`]: errors.md#err_require_esm
 [`ERR_UNSUPPORTED_DIR_IMPORT`]: errors.md#err_unsupported_dir_import
 [`MODULE_NOT_FOUND`]: errors.md#module_not_found
 [`__dirname`]: #__dirname

doc/api/packages.md

@@ -172,8 +172,7 @@ There is the CommonJS module loader:
 * It treats all files that lack `.json` or `.node` extensions as JavaScript
   text files.
 * It can only be used to [load ECMASCript modules from CommonJS modules][] if
-  the module graph is synchronous (that contains no top-level `await`) when
-  `--experimental-require-module` is enabled.
+  the module graph is synchronous (that contains no top-level `await`).
   When used to load a JavaScript text file that is not an ECMAScript module,
   the file will be loaded as a CommonJS module.
 
@@ -662,8 +661,7 @@ specific to least specific as conditions should be defined:
 * `"require"` - matches when the package is loaded via `require()`. The
   referenced file should be loadable with `require()` although the condition
   matches regardless of the module format of the target file. Expected
-  formats include CommonJS, JSON, native addons, and ES modules
-  if `--experimental-require-module` is enabled. _Always mutually
+  formats include CommonJS, JSON, native addons, and ES modules. _Always mutually
   exclusive with `"import"`._
 * `"module-sync"` - matches no matter the package is loaded via `import`,
   `import()` or `require()`. The format is expected to be ES modules that does

lib/buffer.js

@@ -79,10 +79,10 @@ const {
     ONLY_ENUMERABLE,
   },
   getOwnNonIndexProperties,
+  isInsideNodeModules,
 } = internalBinding('util');
 const {
   customInspectSymbol,
-  isInsideNodeModules,
   lazyDOMException,
   normalizeEncoding,
   kIsEncodingSymbol,
@@ -178,13 +178,15 @@ function showFlaggedDeprecation() {
   if (bufferWarningAlreadyEmitted ||
       ++nodeModulesCheckCounter > 10000 ||
       (!require('internal/options').getOptionValue('--pending-deprecation') &&
-       isInsideNodeModules())) {
+       isInsideNodeModules(100, true))) {
     // We don't emit a warning, because we either:
     // - Already did so, or
     // - Already checked too many times whether a call is coming
     //   from node_modules and want to stop slowing down things, or
     // - We aren't running with `--pending-deprecation` enabled,
     //   and the code is inside `node_modules`.
+    // - We found node_modules in up to the topmost 100 frames, or
+    //   there are more than 100 frames and we don't want to search anymore.
     return;
   }