module: unflag detect-module

PR-URL: nodejs#53619
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

Author: GeoffreyBooth

doc/api/cli.md

@@ -887,38 +887,6 @@ files with no extension will be treated as WebAssembly if they begin with the
 WebAssembly magic number (`\0asm`); otherwise they will be treated as ES module
 JavaScript.
 
-### `--experimental-detect-module`
-
-<!-- YAML
-added:
-  - v20.10.0
--->
-
-> Stability: 1.1 - Active development
-
-Node.js will inspect the source code of ambiguous input to determine whether it
-contains ES module syntax; if such syntax is detected, the input will be treated
-as an ES module.
-
-Ambiguous input is defined as:
-
-* Files with a `.js` extension or no extension; and either no controlling
-  `package.json` file or one that lacks a `type` field; and
-  `--experimental-default-type` is not specified.
-* String input (`--eval` or STDIN) when neither `--input-type` nor
-  `--experimental-default-type` are specified.
-
-ES module syntax is defined as syntax that would throw when evaluated as
-CommonJS. This includes the following:
-
-* `import` statements (but _not_ `import()` expressions, which are valid in
-  CommonJS).
-* `export` statements.
-* `import.meta` references.
-* `await` at the top level of a module.
-* Lexical redeclarations of the CommonJS wrapper variables (`require`, `module`,
-  `exports`, `__dirname`, `__filename`).
-
 ### `--experimental-eventsource`
 
 <!-- YAML
@@ -1527,6 +1495,21 @@ added: v0.8.0
 
 Silence deprecation warnings.
 
+### `--no-experimental-detect-module`
+
+<!-- YAML
+added:
+  - v21.1.0
+  - v20.10.0
+changes:
+  - version:
+    - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/53619
+    description: Syntax detection is enabled by default.
+-->
+
+Disable using [syntax detection][] to determine module type.
+
 ### `--no-experimental-fetch`
 
 <!-- YAML
@@ -3361,6 +3344,11 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [security warning]: #warning-binding-inspector-to-a-public-ipport-combination-is-insecure
 [semi-space]: https://www.memorymanagement.org/glossary/s.html#semi.space
 [single executable application]: single-executable-applications.md
+<<<<<<< HEAD
+=======
+[snapshot testing]: test.md#snapshot-testing
+[syntax detection]: packages.md#syntax-detection
+>>>>>>> 604ce4cc66 (module: unflag detect-module)
 [test reporters]: test.md#test-reporters
 [timezone IDs]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
 [tracking issue for user-land snapshots]: https://github.com/nodejs/node/issues/44014

doc/api/esm.md

@@ -1064,8 +1064,7 @@ _isImports_, _conditions_)
 > 10. If _url_ ends in _".js"_, then
 >     1. If _packageType_ is not **null**, then
 >        1. Return _packageType_.
->     2. If `--experimental-detect-module` is enabled and the result of
->        **DETECT\_MODULE\_SYNTAX**(_source_) is true, then
+>     2. If the result of **DETECT\_MODULE\_SYNTAX**(_source_) is true, then
 >        1. Return _"module"_.
 >     3. Return _"commonjs"_.
 > 11. If _url_ does not have any extension, then
@@ -1075,8 +1074,7 @@ _isImports_, _conditions_)
 >        1. Return _"wasm"_.
 >     2. If _packageType_ is not **null**, then
 >        1. Return _packageType_.
->     3. If `--experimental-detect-module` is enabled and the source of
->        module contains static import or export syntax, then
+>     3. If the result of **DETECT\_MODULE\_SYNTAX**(_source_) is true, then
 >        1. Return _"module"_.
 >     4. Return _"commonjs"_.
 > 12. Return **undefined** (will throw during load phase).

doc/api/modules.md

@@ -192,7 +192,7 @@ loaded by `require()` meets the following requirements:
   1. The file has a `.mjs` extension.
   2. The file has a `.js` extension, and the closest `package.json` contains `"type": "module"`
   3. The file has a `.js` extension, the closest `package.json` does not contain
-     `"type": "commonjs"`, and `--experimental-detect-module` is enabled.
+     `"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
@@ -279,7 +279,7 @@ require(X) from module at path Y
 
 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` and `--experimental-detect-module` are
+2. Else, if `--experimental-require-module` is
   enabled, and 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>,

doc/api/packages.md

@@ -69,14 +69,14 @@ expressions:
 * Strings passed in as an argument to `--eval`, or piped to `node` via `STDIN`,
   with the flag `--input-type=module`.
 
-* When using [`--experimental-detect-module`][], code containing syntax only
-  successfully parsed as [ES modules][], such as `import` or `export`
-  statements or `import.meta`, having no explicit marker of how it should be
-  interpreted. Explicit markers are `.mjs` or `.cjs` extensions, `package.json`
-  `"type"` fields with either `"module"` or `"commonjs"` values, or
-  `--input-type` or `--experimental-default-type` flags. Dynamic `import()`
-  expressions are supported in either CommonJS or ES modules and would not
-  cause a file to be treated as an ES module.
+* Code containing syntax only successfully parsed as [ES modules][], such as
+  `import` or `export` statements or `import.meta`, with no explicit marker of
+  how it should be interpreted. Explicit markers are `.mjs` or `.cjs`
+  extensions, `package.json` `"type"` fields with either `"module"` or
+  `"commonjs"` values, or `--input-type` or `--experimental-default-type` flags.
+  Dynamic `import()` expressions are supported in either CommonJS or ES modules
+  and would not force a file to be treated as an ES module. See
+  [Syntax detection][].
 
 Node.js will treat the following as [CommonJS][] when passed to `node` as the
 initial input, or when referenced by `import` statements or `import()`
@@ -115,6 +115,44 @@ package in case the default type of Node.js ever changes, and it will also make
 things easier for build tools and loaders to determine how the files in the
 package should be interpreted.
 
+### Syntax detection
+
+<!-- YAML
+added:
+  - v21.1.0
+  - v20.10.0
+changes:
+  - version:
+    - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/53619
+    description: Syntax detection is enabled by default.
+-->
+
+> Stability: 1.2 - Release candidate
+
+Node.js will inspect the source code of ambiguous input to determine whether it
+contains ES module syntax; if such syntax is detected, the input will be treated
+as an ES module.
+
+Ambiguous input is defined as:
+
+* Files with a `.js` extension or no extension; and either no controlling
+  `package.json` file or one that lacks a `type` field; and
+  `--experimental-default-type` is not specified.
+* String input (`--eval` or STDIN) when neither `--input-type` nor
+  `--experimental-default-type` are specified.
+
+ES module syntax is defined as syntax that would throw when evaluated as
+CommonJS. This includes the following:
+
+* `import` statements (but _not_ `import()` expressions, which are valid in
+  CommonJS).
+* `export` statements.
+* `import.meta` references.
+* `await` at the top level of a module.
+* Lexical redeclarations of the CommonJS wrapper variables (`require`, `module`,
+  `exports`, `__dirname`, `__filename`).
+
 ### Modules loaders
 
 Node.js has two systems for resolving a specifier and loading modules.
@@ -1355,6 +1393,7 @@ This field defines [subpath imports][] for the current package.
 [ES modules]: esm.md
 [Node.js documentation for this section]: https://github.com/nodejs/node/blob/HEAD/doc/api/packages.md#conditions-definitions
 [Runtime Keys]: https://runtime-keys.proposal.wintercg.org/
+[Syntax detection]: #syntax-detection
 [WinterCG]: https://wintercg.org/
 [`"exports"`]: #exports
 [`"imports"`]: #imports
@@ -1364,7 +1403,6 @@ This field defines [subpath imports][] for the current package.
 [`"type"`]: #type
 [`--conditions` / `-C` flag]: #resolving-user-conditions
 [`--experimental-default-type`]: cli.md#--experimental-default-typetype
-[`--experimental-detect-module`]: cli.md#--experimental-detect-module
 [`--no-addons` flag]: cli.md#--no-addons
 [`ERR_PACKAGE_PATH_NOT_EXPORTED`]: errors.md#err_package_path_not_exported
 [`esm`]: https://github.com/standard-things/esm#readme

lib/internal/main/check_syntax.js

@@ -57,23 +57,23 @@ function loadESMIfNeeded(cb) {
 }
 
 async function checkSyntax(source, filename) {
-  let isModule = true;
+  let format;
   if (filename === '[stdin]' || filename === '[eval]') {
-    isModule = getOptionValue('--input-type') === 'module' ||
-      (getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs');
+    format = (getOptionValue('--input-type') === 'module' ||
+      (getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) ?
+      'module' : 'commonjs';
   } else {
     const { defaultResolve } = require('internal/modules/esm/resolve');
     const { defaultGetFormat } = require('internal/modules/esm/get_format');
     const { url } = await defaultResolve(pathToFileURL(filename).toString());
-    const format = await defaultGetFormat(new URL(url));
-    isModule = format === 'module';
+    format = await defaultGetFormat(new URL(url));
   }
 
-  if (isModule) {
+  if (format === 'module') {
     const { ModuleWrap } = internalBinding('module_wrap');
     new ModuleWrap(filename, undefined, source, 0, 0);
     return;
   }
 
-  wrapSafe(filename, source, undefined, 'commonjs');
+  wrapSafe(filename, source, undefined, format);
 }

lib/internal/modules/run_main.js

@@ -149,7 +149,7 @@ function runEntryPointWithESMLoader(callback) {
  * by `require('module')`) even when the entry point is ESM.
  * This monkey-patchable code is bypassed under `--experimental-default-type=module`.
  * Because of backwards compatibility, this function is exposed publicly via `import { runMain } from 'node:module'`.
- * When `--experimental-detect-module` is passed, this function will attempt to run ambiguous (no explicit extension, no
+ * Because of module detection, this function will attempt to run ambiguous (no explicit extension, no
  * `package.json` type field) entry points as CommonJS first; under certain conditions, it will retry running as ESM.
  * @param {string} main - First positional CLI argument, such as `'entry.js'` from `node entry.js`
  */

src/node_options.cc

@@ -364,7 +364,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "when ambiguous modules fail to evaluate because they contain "
             "ES module syntax, try again to evaluate them as ES modules",
             &EnvironmentOptions::detect_module,
-            kAllowedInEnvvar);
+            kAllowedInEnvvar,
+            true);
   AddOption("--experimental-print-required-tla",
             "Print pending top-level await. If --experimental-require-module "
             "is true, evaluate asynchronous graphs loaded by `require()` but "

src/node_options.h

@@ -115,7 +115,7 @@ class EnvironmentOptions : public Options {
  public:
   bool abort_on_uncaught_exception = false;
   std::vector<std::string> conditions;
-  bool detect_module = false;
+  bool detect_module = true;
   bool print_required_tla = false;
   bool require_module = false;
   std::string dns_result_order;

test/es-module/test-esm-cjs-exports.js

@@ -21,9 +21,8 @@ describe('ESM: importing CJS', { concurrency: true }, () => {
     const invalidEntry = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
     const { code, signal, stderr } = await spawnPromisified(execPath, [invalidEntry]);
 
+    assert.match(stderr, /SyntaxError: The requested module '\.\/invalid-cjs\.js' does not provide an export named 'default'/);
     assert.strictEqual(code, 1);
     assert.strictEqual(signal, null);
-    assert.ok(stderr.includes('Warning: To load an ES module'));
-    assert.ok(stderr.includes('Unexpected token \'export\''));
   });
 });