Skip to content

Commit

Permalink
module: support detection when require()ing implicit ESM
Browse files Browse the repository at this point in the history
  • Loading branch information
joyeecheung committed Mar 11, 2024
1 parent 80678e1 commit 289a1cd
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 12 deletions.
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ doc/changelogs/CHANGELOG_v1*.md
!doc/changelogs/CHANGELOG_v18.md
!doc/api_assets/*.js
!.eslintrc.js
test/es-module/test-require-module-detect-entry-point.js
test/es-module/test-require-module-detect-entry-point-aou.js
14 changes: 14 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,19 @@ Supports loading a synchronous ES module graph in `require()`.

See [Loading ECMAScript modules using `require()`][].

### `--experimental-require-module-with-detection`

<!-- YAML
added: REPLACEME
-->

> Stability: 1.1 - Active Developement
In addition to what `--experimental-require-module` supports, when the module
being `require()`'d is not explicitly marked as an ES Module using the
`"type": "module"` field in `package.json` or the `.mjs` extension, it will
try to detect module type based on the syntax of the module.

### `--experimental-sea-config`

<!-- YAML
Expand Down Expand Up @@ -2537,6 +2550,7 @@ Node.js options that are allowed are:
* `--experimental-permission`
* `--experimental-policy`
* `--experimental-print-required-tla`
* `--experimental-require-module-with-detection`
* `--experimental-require-module`
* `--experimental-shadow-realm`
* `--experimental-specifier-resolution`
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,9 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
if (!loadAsESM) {
const result = wrapSafe(filename, content, this);
compiledWrapper = result.function;
// If C++ land confirms it looks like ESM and we should retry here (because fallback
// detection is enabled), retry.
loadAsESM = result.retryAsESM;
}

if (loadAsESM) {
Expand Down
43 changes: 35 additions & 8 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1510,6 +1510,13 @@ const char* require_esm_warning =
"or the module should use the .mjs extension.\n"
"- If it's loaded using require(), use --experimental-require-module";

static bool warned_about_require_esm_detection = false;
const char* require_esm_warning_without_detection =
"The module being require()d looks like an ES module, but it is not "
"explicitly marked with \"type\": \"module\" in the package.json or "
"with a .mjs extention. To enable automatic detection of module syntax "
"in require(), use --experimental-require-module-with-detection.";

static void CompileFunctionForCJSLoader(
const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsString());
Expand Down Expand Up @@ -1553,6 +1560,7 @@ static void CompileFunctionForCJSLoader(

std::vector<Local<String>> params = GetCJSParameters(env->isolate_data());

bool retry_as_esm = false;
Local<Function> fn;

{
Expand Down Expand Up @@ -1597,13 +1605,29 @@ static void CompileFunctionForCJSLoader(
return;
}

if (!warned_about_require_esm) {
USE(ProcessEmitWarningSync(env, require_esm_warning));
warned_about_require_esm = true;
if (!env->options()->require_module) {
if (!warned_about_require_esm) {
USE(ProcessEmitWarningSync(env, require_esm_warning));
warned_about_require_esm = true;
}
errors::DecorateErrorStack(env, try_catch);
try_catch.ReThrow();
return;
}
errors::DecorateErrorStack(env, try_catch);
try_catch.ReThrow();
return;

if (!env->options()->require_module_with_detection) {
if (!warned_about_require_esm_detection) {
USE(ProcessEmitWarningSync(env,
require_esm_warning_without_detection));
warned_about_require_esm_detection = true;
}
errors::DecorateErrorStack(env, try_catch);
try_catch.ReThrow();
return;
}

// The file being compiled is likely ESM.
retry_as_esm = true;
}
}
}
Expand All @@ -1623,11 +1647,14 @@ static void CompileFunctionForCJSLoader(
env->cached_data_rejected_string(),
env->source_map_url_string(),
env->function_string(),
FIXED_ONE_BYTE_STRING(isolate, "retryAsESM"),
};
std::vector<Local<Value>> values = {
Boolean::New(isolate, cache_rejected),
fn->GetScriptOrigin().SourceMapUrl(),
fn.As<Value>(),
retry_as_esm ? v8::Undefined(isolate).As<Value>()
: fn->GetScriptOrigin().SourceMapUrl(),
retry_as_esm ? v8::Undefined(isolate).As<Value>() : fn.As<Value>(),
Boolean::New(isolate, retry_as_esm),
};
Local<Object> result = Object::New(
isolate, v8::Null(isolate), names.data(), values.data(), names.size());
Expand Down
8 changes: 8 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,14 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"Allow loading explicit ES Modules in require().",
&EnvironmentOptions::require_module,
kAllowedInEnvvar);
AddOption("--experimental-require-module-with-detection",
"Allow loading ES Modules with syntax-based detection of module "
"types for implicit ES modules in require().",
&EnvironmentOptions::require_module_with_detection,
kAllowedInEnvvar);
Implies("--experimental-require-module-with-detection",
"--experimental-require-module");

AddOption("--diagnostic-dir",
"set dir for all output files"
" (default: current working directory)",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class EnvironmentOptions : public Options {
bool detect_module = false;
bool print_required_tla = false;
bool require_module = false;
bool require_module_with_detection = false;
std::string dns_result_order;
bool enable_source_maps = false;
bool experimental_fetch = true;
Expand Down
8 changes: 8 additions & 0 deletions test/es-module/test-require-module-detect-entry-point-aou.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Flags: --experimental-require-module-with-detection --abort-on-uncaught-exception
'use strict';

import { mustCall } from '../common/index.mjs';
const fn = mustCall(() => {
console.log('hello');
});
fn();
8 changes: 8 additions & 0 deletions test/es-module/test-require-module-detect-entry-point.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Flags: --experimental-require-module-with-detection
'use strict';

import { mustCall } from '../common/index.mjs';
const fn = mustCall(() => {
console.log('hello');
});
fn();
11 changes: 11 additions & 0 deletions test/es-module/test-require-module-dont-detect-cjs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Flags: --experimental-require-module-with-detection
'use strict';

require('../common');
const assert = require('assert');

assert.throws(() => {
require('../fixtures/es-modules/es-note-unexpected-export-1.cjs');
}, {
message: /Unexpected token 'export'/
});
9 changes: 5 additions & 4 deletions test/es-module/test-require-module-implicit.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ const assert = require('assert');

common.expectWarning(
'Warning',
'To load an ES module:\n' +
'- Either the nearest package.json should set "type":"module", ' +
'or the module should use the .mjs extension.\n' +
'- If it\'s loaded using require(), use --experimental-require-module'
'The module being require()d looks like an ES module, but it is ' +
'not explicitly marked with "type": "module" in the package.json ' +
'or with a .mjs extention. To enable automatic detection of module ' +
'syntax in require(), use --experimental-require-module-with-detection.'
);

common.expectWarning(
'ExperimentalWarning',
'Support for loading ES Module in require() is an experimental feature ' +
Expand Down
18 changes: 18 additions & 0 deletions test/es-module/test-require-module-with-detection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Flags: --experimental-require-module-with-detection
'use strict';

require('../common');
const assert = require('assert');
const { isModuleNamespaceObject } = require('util/types');

{
const mod = require('../fixtures/es-modules/loose.js');
assert.deepStrictEqual({ ...mod }, { default: 'module' });
assert(isModuleNamespaceObject(mod));
}

{
const mod = require('../fixtures/es-modules/package-without-type/noext-esm');
assert.deepStrictEqual({ ...mod }, { default: 'module' });
assert(isModuleNamespaceObject(mod));
}

0 comments on commit 289a1cd

Please sign in to comment.