module: add dynamic file-specific ESM warnings

PR-URL: https://github.com/nodejs/node/pull/56628
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This commit is contained in:
Mert Can Altin 2025-03-03 10:35:35 +03:00 committed by GitHub
parent 208f07adda
commit 7c910d09e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 54 additions and 19 deletions

View File

@ -1702,13 +1702,19 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
return scope.Escape(fn); return scope.Escape(fn);
} }
static std::string GetRequireEsmWarning(Local<String> filename) {
Isolate* isolate = Isolate::GetCurrent();
Utf8Value filename_utf8(isolate, filename);
std::string warning_message =
"Failed to load the ES module: " + filename_utf8.ToString() +
". Make sure to set \"type\": \"module\" in the nearest package.json "
"file "
"or use the .mjs extension.";
return warning_message;
}
static bool warned_about_require_esm = false; static bool warned_about_require_esm = false;
// TODO(joyeecheung): this was copied from the warning previously emitted in the
// JS land, but it's not very helpful. There should be specific information
// about which file or which package.json to update.
const char* require_esm_warning =
"To load an ES module, set \"type\": \"module\" in the package.json or use "
"the .mjs extension.";
static bool ShouldRetryAsESM(Realm* realm, static bool ShouldRetryAsESM(Realm* realm,
Local<String> message, Local<String> message,
@ -1794,8 +1800,8 @@ static void CompileFunctionForCJSLoader(
// This needs to call process.emit('warning') in JS which can throw if // This needs to call process.emit('warning') in JS which can throw if
// the user listener throws. In that case, don't try to throw the syntax // the user listener throws. In that case, don't try to throw the syntax
// error. // error.
should_throw = std::string warning_message = GetRequireEsmWarning(filename);
ProcessEmitWarningSync(env, require_esm_warning).IsJust(); should_throw = ProcessEmitWarningSync(env, warning_message).IsJust();
} }
if (should_throw) { if (should_throw) {
isolate->ThrowException(cjs_exception); isolate->ThrowException(cjs_exception);

View File

@ -13,6 +13,7 @@ using v8::Just;
using v8::Local; using v8::Local;
using v8::Maybe; using v8::Maybe;
using v8::MaybeLocal; using v8::MaybeLocal;
using v8::NewStringType;
using v8::Nothing; using v8::Nothing;
using v8::Object; using v8::Object;
using v8::String; using v8::String;
@ -21,7 +22,14 @@ using v8::Value;
Maybe<bool> ProcessEmitWarningSync(Environment* env, std::string_view message) { Maybe<bool> ProcessEmitWarningSync(Environment* env, std::string_view message) {
Isolate* isolate = env->isolate(); Isolate* isolate = env->isolate();
Local<Context> context = env->context(); Local<Context> context = env->context();
Local<String> message_string = OneByteString(isolate, message); Local<String> message_string;
if (!String::NewFromUtf8(isolate,
message.data(),
NewStringType::kNormal,
static_cast<int>(message.size()))
.ToLocal(&message_string)) {
return Nothing<bool>();
}
Local<Value> argv[] = {message_string}; Local<Value> argv[] = {message_string};
Local<Function> emit_function = env->process_emit_warning_sync(); Local<Function> emit_function = env->process_emit_warning_sync();

View File

@ -4,11 +4,10 @@ import assert from 'node:assert';
import { execPath } from 'node:process'; import { execPath } from 'node:process';
import { describe, it } from 'node:test'; import { describe, it } from 'node:test';
// Expect note to be included in the error output // Expect note to be included in the error output
// Don't match the following sentence because it can change as features are // Don't match the following sentence because it can change as features are
// added. // added.
const expectedNote = 'Warning: To load an ES module'; const expectedNote = 'Failed to load the ES module';
const mustIncludeMessage = { const mustIncludeMessage = {
getMessage: (stderr) => `${expectedNote} not found in ${stderr}`, getMessage: (stderr) => `${expectedNote} not found in ${stderr}`,

View File

@ -182,7 +182,7 @@ describe('long path on Windows', () => {
fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";'); fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";');
const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]); const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]);
assert.ok(stderr.includes('Warning: To load an ES module')); assert.ok(stderr.includes('Failed to load the ES module'));
assert.strictEqual(code, 1); assert.strictEqual(code, 1);
assert.strictEqual(signal, null); assert.strictEqual(signal, null);
}); });

View File

@ -1,6 +1,6 @@
import { skip, spawnPromisified } from '../common/index.mjs'; import { skip, spawnPromisified } from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs'; import * as fixtures from '../common/fixtures.mjs';
import { match, strictEqual } from 'node:assert'; import assert, { match, strictEqual } from 'node:assert';
import { test } from 'node:test'; import { test } from 'node:test';
if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); if (!process.config.variables.node_use_amaro) skip('Requires Amaro');
@ -59,13 +59,35 @@ test('require a .ts file with implicit extension fails', async () => {
}); });
test('expect failure of an .mts file with CommonJS syntax', async () => { test('expect failure of an .mts file with CommonJS syntax', async () => {
const result = await spawnPromisified(process.execPath, [ const testFilePath = fixtures.path(
fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'), 'typescript/cts/test-cts-but-module-syntax.cts'
]); );
const result = await spawnPromisified(process.execPath, [testFilePath]);
strictEqual(result.stdout, ''); assert.strictEqual(result.stdout, '');
match(result.stderr, /To load an ES module, set "type": "module" in the package\.json or use the \.mjs extension\./);
strictEqual(result.code, 1); const expectedWarning = `Failed to load the ES module: ${testFilePath}. Make sure to set "type": "module" in the nearest package.json file or use the .mjs extension.`;
try {
assert.ok(
result.stderr.includes(expectedWarning),
`Expected stderr to include: ${expectedWarning}`
);
} catch (e) {
if (e?.code === 'ERR_ASSERTION') {
assert.match(
result.stderr,
/Failed to load the ES module:.*test-cts-but-module-syntax\.cts/
);
e.expected = expectedWarning;
e.actual = result.stderr;
e.operator = 'includes';
}
throw e;
}
assert.strictEqual(result.code, 1);
}); });
test('execute a .cts file importing a .cts file', async () => { test('execute a .cts file importing a .cts file', async () => {