FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: add dynamic file-specific ESM warnings · nodejs/node@718305d · GitHub

/ node Public

Commit 718305d

Browse files
mertcanaltin
authored and
aduh95
committed
module: add dynamic file-specific ESM warnings
PR-URL: #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>
1 parent 0fd3d91 commit 718305d

5 files changed

Lines changed: 55 additions & 20 deletions

File tree

‎src/node_contextify.cc‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1702,13 +1702,19 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
17021702
return scope.Escape(fn);
17031703
}
17041704

1705+
static std::string GetRequireEsmWarning(Local<String> filename) {
1706+
Isolate* isolate = Isolate::GetCurrent();
1707+
Utf8Value filename_utf8(isolate, filename);
1708+
1709+
std::string warning_message =
1710+
"Failed to load the ES module: " + filename_utf8.ToString() +
1711+
". Make sure to set \"type\": \"module\" in the nearest package.json "
1712+
"file "
1713+
"or use the .mjs extension.";
1714+
return warning_message;
1715+
}
1716+
17051717
static bool warned_about_require_esm = false;
1706-
// TODO(joyeecheung): this was copied from the warning previously emitted in the
1707-
// JS land, but it's not very helpful. There should be specific information
1708-
// about which file or which package.json to update.
1709-
const char* require_esm_warning =
1710-
"To load an ES module, set \"type\": \"module\" in the package.json or use "
1711-
"the .mjs extension.";
17121718

17131719
static bool ShouldRetryAsESM(Realm* realm,
17141720
Local<String> message,
@@ -1794,8 +1800,8 @@ static void CompileFunctionForCJSLoader(
17941800
// This needs to call process.emit('warning') in JS which can throw if
17951801
// the user listener throws. In that case, don't try to throw the syntax
17961802
// error.
1797-
should_throw =
1798-
ProcessEmitWarningSync(env, require_esm_warning).IsJust();
1803+
std::string warning_message = GetRequireEsmWarning(filename);
1804+
should_throw = ProcessEmitWarningSync(env, warning_message).IsJust();
17991805
}
18001806
if (should_throw) {
18011807
isolate->ThrowException(cjs_exception);

‎src/node_process_events.cc‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ using v8::Just;
1313
using v8::Local;
1414
using v8::Maybe;
1515
using v8::MaybeLocal;
16+
using v8::NewStringType;
1617
using v8::Nothing;
1718
using v8::Object;
1819
using v8::String;
@@ -21,7 +22,14 @@ using v8::Value;
2122
Maybe<bool> ProcessEmitWarningSync(Environment* env, std::string_view message) {
2223
Isolate* isolate = env->isolate();
2324
Local<Context> context = env->context();
24-
Local<String> message_string = OneByteString(isolate, message);
25+
Local<String> message_string;
26+
if (!String::NewFromUtf8(isolate,
27+
message.data(),
28+
NewStringType::kNormal,
29+
static_cast<int>(message.size()))
30+
.ToLocal(&message_string)) {
31+
return Nothing<bool>();
32+
}
2533

2634
Local<Value> argv[] = {message_string};
2735
Local<Function> emit_function = env->process_emit_warning_sync();

‎test/es-module/test-esm-cjs-load-error-note.mjs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ import assert from 'node:assert';
44
import { execPath } from 'node:process';
55
import { describe, it } from 'node:test';
66

7-
87
// Expect note to be included in the error output
98
// Don't match the following sentence because it can change as features are
109
// added.
11-
const expectedNote = 'Warning: To load an ES module';
10+
const expectedNote = 'Failed to load the ES module';
1211

1312
const mustIncludeMessage = {
1413
getMessage: (stderr) => `${expectedNote} not found in ${stderr}`,

‎test/es-module/test-esm-long-path-win.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ describe('long path on Windows', () => {
182182
fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";');
183183
const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]);
184184

185-
assert.ok(stderr.includes('Warning: To load an ES module'));
185+
assert.ok(stderr.includes('Failed to load the ES module'));
186186
assert.strictEqual(code, 1);
187187
assert.strictEqual(signal, null);
188188
});

‎test/es-module/test-typescript-commonjs.mjs‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { skip, spawnPromisified } from '../common/index.mjs';
22
import * as fixtures from '../common/fixtures.mjs';
3-
import { match, strictEqual } from 'node:assert';
3+
import assert, { match, strictEqual } from 'node:assert';
44
import { test } from 'node:test';
55

66
if (!process.config.variables.node_use_amaro) skip('Requires Amaro');
@@ -59,13 +59,35 @@ test('require a .ts file with implicit extension fails', async () => {
5959
});
6060

6161
test('expect failure of an .mts file with CommonJS syntax', async () => {
62-
const result = await spawnPromisified(process.execPath, [
63-
fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'),
64-
]);
65-
66-
strictEqual(result.stdout, '');
67-
match(result.stderr, /To load an ES module, set "type": "module" in the package\.json or use the \.mjs extension\./);
68-
strictEqual(result.code, 1);
62+
const testFilePath = fixtures.path(
63+
'typescript/cts/test-cts-but-module-syntax.cts'
64+
);
65+
const result = await spawnPromisified(process.execPath, [testFilePath]);
66+
67+
assert.strictEqual(result.stdout, '');
68+
69+
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.`;
70+
71+
try {
72+
assert.ok(
73+
result.stderr.includes(expectedWarning),
74+
`Expected stderr to include: ${expectedWarning}`
75+
);
76+
} catch (e) {
77+
if (e?.code === 'ERR_ASSERTION') {
78+
assert.match(
79+
result.stderr,
80+
/Failed to load the ES module:.*test-cts-but-module-syntax\.cts/
81+
);
82+
e.expected = expectedWarning;
83+
e.actual = result.stderr;
84+
e.operator = 'includes';
85+
}
86+
throw e;
87+
}
88+
89+
90+
assert.strictEqual(result.code, 1);
6991
});
7092

7193
test('execute a .cts file importing a .cts file', async () => {

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL