Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: unflag detect-module #53619

Merged
merged 1 commit into from
Jul 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 16 additions & 33 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -940,39 +940,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:
- v21.1.0
- 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
Expand Down Expand Up @@ -1624,6 +1591,21 @@ added: v0.8.0

Silence deprecation warnings.

### `--no-experimental-detect-module`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it on by default if it is experimental? Perhaps --no-detect-module?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have lots of stuff experimental yet on by default. -1 on changing the flag, we want to keep --experimental-detect-module as a no-op anyway, the flag is experimental, and we plan to remove it without going through a deprecation cycle, which is another good reason to keep "experimental" in its name.


GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
<!-- 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-global-navigator`

<!-- YAML
Expand Down Expand Up @@ -3495,6 +3477,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
[semi-space]: https://www.memorymanagement.org/glossary/s.html#semi.space
[single executable application]: single-executable-applications.md
[snapshot testing]: test.md#snapshot-testing
[syntax detection]: packages.md#syntax-detection
[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
Expand Down
6 changes: 2 additions & 4 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1082,8 +1082,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
Expand All @@ -1093,8 +1092,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).
Expand Down
4 changes: 2 additions & 2 deletions doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,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
Expand Down Expand Up @@ -272,7 +272,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>,
Expand Down
56 changes: 47 additions & 9 deletions doc/api/packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()`
Expand Down Expand Up @@ -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
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions lib/internal/main/check_syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
} 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);
}
2 changes: 1 addition & 1 deletion lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,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`
*/
Expand Down
3 changes: 2 additions & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,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 "
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,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;
Expand Down
3 changes: 1 addition & 2 deletions test/es-module/test-esm-cjs-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ describe('ESM: importing CJS', { concurrency: !process.env.TEST_PARALLEL }, () =
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\''));
});
});
Loading
Loading