From 777a249faddd32ff5962f61d96a00afc26f62657 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Thu, 16 Jan 2025 18:17:29 +0300 Subject: [PATCH 01/22] module: add dynamic file-specific ESM warnings --- src/node_contextify.cc | 21 ++++++++++++------- .../test-esm-cjs-load-error-note.mjs | 3 +-- test/es-module/test-typescript-commonjs.mjs | 10 +++++---- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 77d35675827c67..8e779bbbfd276c 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1688,13 +1688,19 @@ static MaybeLocal CompileFunctionForCJSLoader( return scope.Escape(fn); } +static std::string GetRequireEsmWarning(Local filename) { + Isolate* isolate = Isolate::GetCurrent(); + Utf8Value filename_utf8(isolate, filename); + + std::string warning_message = + "Failed to load the ES module: " + std::string(*filename_utf8) + + ". 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; -// 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, Local message, @@ -1780,8 +1786,9 @@ static void CompileFunctionForCJSLoader( // 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 // error. + std::string warning_message = GetRequireEsmWarning(filename); should_throw = - ProcessEmitWarningSync(env, require_esm_warning).IsJust(); + ProcessEmitWarningSync(env, warning_message.c_str()).IsJust(); } if (should_throw) { isolate->ThrowException(cjs_exception); diff --git a/test/es-module/test-esm-cjs-load-error-note.mjs b/test/es-module/test-esm-cjs-load-error-note.mjs index 2875f4844de359..54000b53fea9b4 100644 --- a/test/es-module/test-esm-cjs-load-error-note.mjs +++ b/test/es-module/test-esm-cjs-load-error-note.mjs @@ -4,11 +4,10 @@ import assert from 'node:assert'; import { execPath } from 'node:process'; import { describe, it } from 'node:test'; - // Expect note to be included in the error output // Don't match the following sentence because it can change as features are // added. -const expectedNote = 'Warning: To load an ES module'; +const expectedNote = 'Failed to load the ES module'; const mustIncludeMessage = { getMessage: (stderr) => `${expectedNote} not found in ${stderr}`, diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 5b15860ab32796..a77d49cb3cbfac 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -59,12 +59,14 @@ test('require a .ts file with implicit extension fails', async () => { }); test('expect failure of an .mts file with CommonJS syntax', async () => { - const result = await spawnPromisified(process.execPath, [ - fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'), - ]); + const testFilePath = fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'); + const result = await spawnPromisified(process.execPath, [testFilePath]); strictEqual(result.stdout, ''); - match(result.stderr, /To load an ES module, set "type": "module" in the package\.json or use the \.mjs extension\./); + + 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.`; + match(result.stderr, new RegExp(expectedWarning)); + strictEqual(result.code, 1); }); From 5541de0b0907b72553a201c003d1346a1c585a54 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 19 Jan 2025 17:11:02 +0300 Subject: [PATCH 02/22] Update src/node_contextify.cc Co-authored-by: Anna Henningsen --- src/node_contextify.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 8e779bbbfd276c..ada540c4d3edd0 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1693,7 +1693,7 @@ static std::string GetRequireEsmWarning(Local filename) { Utf8Value filename_utf8(isolate, filename); std::string warning_message = - "Failed to load the ES module: " + std::string(*filename_utf8) + + "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."; From 87b5e2e744dad9a92de90f306259878e9482f104 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 24 Jan 2025 08:15:08 +0300 Subject: [PATCH 03/22] Update test-typescript-commonjs.mjs Co-authored-by: Antoine du Hamel --- test/es-module/test-typescript-commonjs.mjs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index a77d49cb3cbfac..649d02a04b0041 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -65,7 +65,16 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { strictEqual(result.stdout, ''); 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.`; - match(result.stderr, new RegExp(expectedWarning)); + try { + assert.ok(result.stderr.includes(expectedWarning), 'stderr does not contain the expected warning'); + } catch (e) { + if (e?.code === 'ERR_ASSERTION') { + e.expected = expectedWarning; + e.actual = result.stderr; + e.operator = 'includes'; + } + throw e; + } strictEqual(result.code, 1); }); From 23b8955df2beeff84226e0edda90a172cf356ddd Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 24 Jan 2025 08:26:11 +0300 Subject: [PATCH 04/22] lint --- test/es-module/test-typescript-commonjs.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 649d02a04b0041..aaf13b05b1a989 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -1,6 +1,6 @@ import { skip, spawnPromisified } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; -import { match, strictEqual } from 'node:assert'; +import { match, strictEqual, assert } from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); @@ -65,6 +65,7 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { strictEqual(result.stdout, ''); 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), 'stderr does not contain the expected warning'); } catch (e) { From 05c573f154e2e520d651e547f582cbc20ce503d0 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 24 Jan 2025 14:48:48 +0300 Subject: [PATCH 05/22] fix: resolved import issue with assert module --- test/es-module/test-typescript-commonjs.mjs | 76 ++++++++++----------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index aaf13b05b1a989..dc0215c1066893 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -1,6 +1,6 @@ import { skip, spawnPromisified } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; -import { match, strictEqual, assert } from 'node:assert'; +import * as assert from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); @@ -14,9 +14,9 @@ test('require a .ts file with explicit extension succeeds', async () => { cwd: fixtures.path('typescript/ts'), }); - strictEqual(result.stderr, ''); - strictEqual(result.stdout, 'Hello, TypeScript!\n'); - strictEqual(result.code, 0); + assert.strictEqual(result.stderr, ''); + assert.strictEqual(result.stdout, 'Hello, TypeScript!\n'); + assert.strictEqual(result.code, 0); }); test('eval require a .ts file with implicit extension fails', async () => { @@ -28,9 +28,9 @@ test('eval require a .ts file with implicit extension fails', async () => { cwd: fixtures.path('typescript/ts'), }); - strictEqual(result.stdout, ''); - match(result.stderr, /Error: Cannot find module/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /Error: Cannot find module/); + assert.strictEqual(result.code, 1); }); test('eval require a .cts file with implicit extension fails', async () => { @@ -42,9 +42,9 @@ test('eval require a .cts file with implicit extension fails', async () => { cwd: fixtures.path('typescript/ts'), }); - strictEqual(result.stdout, ''); - match(result.stderr, /Error: Cannot find module/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /Error: Cannot find module/); + assert.strictEqual(result.code, 1); }); test('require a .ts file with implicit extension fails', async () => { @@ -53,16 +53,16 @@ test('require a .ts file with implicit extension fails', async () => { fixtures.path('typescript/cts/test-extensionless-require.ts'), ]); - strictEqual(result.stdout, ''); - match(result.stderr, /Error: Cannot find module/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /Error: Cannot find module/); + assert.strictEqual(result.code, 1); }); test('expect failure of an .mts file with CommonJS syntax', async () => { const testFilePath = fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'); const result = await spawnPromisified(process.execPath, [testFilePath]); - strictEqual(result.stdout, ''); + assert.strictEqual(result.stdout, ''); 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.`; @@ -77,7 +77,7 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { throw e; } - strictEqual(result.code, 1); + assert.strictEqual(result.code, 1); }); test('execute a .cts file importing a .cts file', async () => { @@ -86,9 +86,9 @@ test('execute a .cts file importing a .cts file', async () => { fixtures.path('typescript/cts/test-require-commonjs.cts'), ]); - strictEqual(result.stderr, ''); - match(result.stdout, /Hello, TypeScript!/); - strictEqual(result.code, 0); + assert.strictEqual(result.stderr, ''); + assert.match(result.stdout, /Hello, TypeScript!/); + assert.strictEqual(result.code, 0); }); test('execute a .cts file importing a .ts file export', async () => { @@ -97,9 +97,9 @@ test('execute a .cts file importing a .ts file export', async () => { fixtures.path('typescript/cts/test-require-ts-file.cts'), ]); - strictEqual(result.stderr, ''); - match(result.stdout, /Hello, TypeScript!/); - strictEqual(result.code, 0); + assert.strictEqual(result.stderr, ''); + assert.match(result.stdout, /Hello, TypeScript!/); + assert.strictEqual(result.code, 0); }); test('execute a .cts file importing a .mts file export', async () => { @@ -108,9 +108,9 @@ test('execute a .cts file importing a .mts file export', async () => { fixtures.path('typescript/cts/test-require-mts-module.cts'), ]); - strictEqual(result.stdout, ''); - match(result.stderr, /Error \[ERR_REQUIRE_ESM\]: require\(\) of ES Module/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /Error \[ERR_REQUIRE_ESM\]: require\(\) of ES Module/); + assert.strictEqual(result.code, 1); }); test('execute a .cts file importing a .mts file export', async () => { @@ -119,8 +119,8 @@ test('execute a .cts file importing a .mts file export', async () => { fixtures.path('typescript/cts/test-require-mts-module.cts'), ]); - match(result.stdout, /Hello, TypeScript!/); - strictEqual(result.code, 0); + assert.match(result.stdout, /Hello, TypeScript!/); + assert.strictEqual(result.code, 0); }); test('expect failure of a .cts file in node_modules', async () => { @@ -128,9 +128,9 @@ test('expect failure of a .cts file in node_modules', async () => { fixtures.path('typescript/cts/test-cts-node_modules.cts'), ]); - strictEqual(result.stdout, ''); - match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + assert.strictEqual(result.code, 1); }); test('expect failure of a .ts file in node_modules', async () => { @@ -138,9 +138,9 @@ test('expect failure of a .ts file in node_modules', async () => { fixtures.path('typescript/cts/test-ts-node_modules.cts'), ]); - strictEqual(result.stdout, ''); - match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + assert.strictEqual(result.code, 1); }); test('expect failure of a .cts requiring esm without default type module', async () => { @@ -149,9 +149,9 @@ test('expect failure of a .cts requiring esm without default type module', async fixtures.path('typescript/cts/test-mts-node_modules.cts'), ]); - strictEqual(result.stdout, ''); - match(result.stderr, /ERR_REQUIRE_ESM/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /ERR_REQUIRE_ESM/); + assert.strictEqual(result.code, 1); }); test('expect failure of a .cts file requiring esm in node_modules', async () => { @@ -160,7 +160,7 @@ test('expect failure of a .cts file requiring esm in node_modules', async () => fixtures.path('typescript/cts/test-mts-node_modules.cts'), ]); - strictEqual(result.stdout, ''); - match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); - strictEqual(result.code, 1); + assert.strictEqual(result.stdout, ''); + assert.match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + assert.strictEqual(result.code, 1); }); From 17eb1d494cfbdff73f01745bb5886a13fc9d3c0c Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Wed, 12 Feb 2025 19:20:43 +0300 Subject: [PATCH 06/22] conversion error in UTF-8 string creation --- src/node_process_events.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/node_process_events.cc b/src/node_process_events.cc index 8aac953b3e0db5..c77097c75b1e7f 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -21,7 +21,12 @@ using v8::Value; Maybe ProcessEmitWarningSync(Environment* env, std::string_view message) { Isolate* isolate = env->isolate(); Local context = env->context(); - Local message_string = OneByteString(isolate, message); + v8::Local message_string = + v8::String::NewFromUtf8(isolate, + message.data(), + v8::NewStringType::kNormal, + static_cast(message.size())) + .ToLocalChecked(); Local argv[] = {message_string}; Local emit_function = env->process_emit_warning_sync(); From 7f2ff3f31ec01f3bf00e4cff0d88d7f801386c04 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 14 Feb 2025 16:45:03 +0300 Subject: [PATCH 07/22] Update node_contextify.cc Co-authored-by: Anna Henningsen --- src/node_contextify.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index ada540c4d3edd0..54aeaaac620de9 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1788,7 +1788,7 @@ static void CompileFunctionForCJSLoader( // error. std::string warning_message = GetRequireEsmWarning(filename); should_throw = - ProcessEmitWarningSync(env, warning_message.c_str()).IsJust(); + ProcessEmitWarningSync(env, warning_message).IsJust(); } if (should_throw) { isolate->ThrowException(cjs_exception); From da45468b6fc942aed3907b929c412c87a4fe6f0d Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 14 Feb 2025 17:28:26 +0300 Subject: [PATCH 08/22] lint --- src/node_contextify.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 54aeaaac620de9..ba2470f4dced8e 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1787,8 +1787,7 @@ static void CompileFunctionForCJSLoader( // the user listener throws. In that case, don't try to throw the syntax // error. std::string warning_message = GetRequireEsmWarning(filename); - should_throw = - ProcessEmitWarningSync(env, warning_message).IsJust(); + should_throw = ProcessEmitWarningSync(env, warning_message).IsJust(); } if (should_throw) { isolate->ThrowException(cjs_exception); From 9cd44cfcab8950cd45cb2ebc07de80dfc8bd208f Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sat, 15 Feb 2025 23:19:49 +0300 Subject: [PATCH 09/22] undo irrelevant changes --- test/es-module/test-typescript-commonjs.mjs | 131 ++++++++++---------- 1 file changed, 68 insertions(+), 63 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index dc0215c1066893..a6fe1c0a62a8c5 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -1,50 +1,50 @@ import { skip, spawnPromisified } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; -import * as assert from 'node:assert'; +import assert, { match, strictEqual } from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); test('require a .ts file with explicit extension succeeds', async () => { - const result = await spawnPromisified(process.execPath, [ - '--eval', - 'require("./test-typescript.ts")', - '--no-warnings', - ], { - cwd: fixtures.path('typescript/ts'), - }); + const result = await spawnPromisified( + process.execPath, + ['--eval', 'require("./test-typescript.ts")', '--no-warnings'], + { + cwd: fixtures.path('typescript/ts'), + } + ); - assert.strictEqual(result.stderr, ''); - assert.strictEqual(result.stdout, 'Hello, TypeScript!\n'); - assert.strictEqual(result.code, 0); + strictEqual(result.stderr, ''); + strictEqual(result.stdout, 'Hello, TypeScript!\n'); + strictEqual(result.code, 0); }); test('eval require a .ts file with implicit extension fails', async () => { - const result = await spawnPromisified(process.execPath, [ - '--eval', - 'require("./test-typescript")', - '--no-warnings', - ], { - cwd: fixtures.path('typescript/ts'), - }); + const result = await spawnPromisified( + process.execPath, + ['--eval', 'require("./test-typescript")', '--no-warnings'], + { + cwd: fixtures.path('typescript/ts'), + } + ); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /Error: Cannot find module/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /Error: Cannot find module/); + strictEqual(result.code, 1); }); test('eval require a .cts file with implicit extension fails', async () => { - const result = await spawnPromisified(process.execPath, [ - '--eval', - 'require("./test-cts-typescript")', - '--no-warnings', - ], { - cwd: fixtures.path('typescript/ts'), - }); + const result = await spawnPromisified( + process.execPath, + ['--eval', 'require("./test-cts-typescript")', '--no-warnings'], + { + cwd: fixtures.path('typescript/ts'), + } + ); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /Error: Cannot find module/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /Error: Cannot find module/); + strictEqual(result.code, 1); }); test('require a .ts file with implicit extension fails', async () => { @@ -53,21 +53,26 @@ test('require a .ts file with implicit extension fails', async () => { fixtures.path('typescript/cts/test-extensionless-require.ts'), ]); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /Error: Cannot find module/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /Error: Cannot find module/); + strictEqual(result.code, 1); }); test('expect failure of an .mts file with CommonJS syntax', async () => { - const testFilePath = fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'); + const testFilePath = fixtures.path( + 'typescript/cts/test-cts-but-module-syntax.cts' + ); const result = await spawnPromisified(process.execPath, [testFilePath]); - assert.strictEqual(result.stdout, ''); - - 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.`; + strictEqual(result.stdout, ''); + const expectedWarning = + 'To load an ES module, set "type": "module" in the package.json or use the .mjs extension.'; try { - assert.ok(result.stderr.includes(expectedWarning), 'stderr does not contain the expected warning'); + assert.ok( + result.stderr.includes(expectedWarning), + `stderr does not contain the expected warning. Actual stderr: ${result.stderr}` + ); } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning; @@ -77,7 +82,7 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { throw e; } - assert.strictEqual(result.code, 1); + strictEqual(result.code, 1); }); test('execute a .cts file importing a .cts file', async () => { @@ -86,9 +91,9 @@ test('execute a .cts file importing a .cts file', async () => { fixtures.path('typescript/cts/test-require-commonjs.cts'), ]); - assert.strictEqual(result.stderr, ''); - assert.match(result.stdout, /Hello, TypeScript!/); - assert.strictEqual(result.code, 0); + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); }); test('execute a .cts file importing a .ts file export', async () => { @@ -97,9 +102,9 @@ test('execute a .cts file importing a .ts file export', async () => { fixtures.path('typescript/cts/test-require-ts-file.cts'), ]); - assert.strictEqual(result.stderr, ''); - assert.match(result.stdout, /Hello, TypeScript!/); - assert.strictEqual(result.code, 0); + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); }); test('execute a .cts file importing a .mts file export', async () => { @@ -108,9 +113,9 @@ test('execute a .cts file importing a .mts file export', async () => { fixtures.path('typescript/cts/test-require-mts-module.cts'), ]); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /Error \[ERR_REQUIRE_ESM\]: require\(\) of ES Module/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /Error \[ERR_REQUIRE_ESM\]: require\(\) of ES Module/); + strictEqual(result.code, 1); }); test('execute a .cts file importing a .mts file export', async () => { @@ -119,8 +124,8 @@ test('execute a .cts file importing a .mts file export', async () => { fixtures.path('typescript/cts/test-require-mts-module.cts'), ]); - assert.match(result.stdout, /Hello, TypeScript!/); - assert.strictEqual(result.code, 0); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); }); test('expect failure of a .cts file in node_modules', async () => { @@ -128,9 +133,9 @@ test('expect failure of a .cts file in node_modules', async () => { fixtures.path('typescript/cts/test-cts-node_modules.cts'), ]); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + strictEqual(result.code, 1); }); test('expect failure of a .ts file in node_modules', async () => { @@ -138,9 +143,9 @@ test('expect failure of a .ts file in node_modules', async () => { fixtures.path('typescript/cts/test-ts-node_modules.cts'), ]); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + strictEqual(result.code, 1); }); test('expect failure of a .cts requiring esm without default type module', async () => { @@ -149,9 +154,9 @@ test('expect failure of a .cts requiring esm without default type module', async fixtures.path('typescript/cts/test-mts-node_modules.cts'), ]); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /ERR_REQUIRE_ESM/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /ERR_REQUIRE_ESM/); + strictEqual(result.code, 1); }); test('expect failure of a .cts file requiring esm in node_modules', async () => { @@ -160,7 +165,7 @@ test('expect failure of a .cts file requiring esm in node_modules', async () => fixtures.path('typescript/cts/test-mts-node_modules.cts'), ]); - assert.strictEqual(result.stdout, ''); - assert.match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); - assert.strictEqual(result.code, 1); + strictEqual(result.stdout, ''); + match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + strictEqual(result.code, 1); }); From 1b276e48216f21dc18f8041a22aa0128e132faf7 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sat, 15 Feb 2025 23:21:35 +0300 Subject: [PATCH 10/22] undo irrelevant changes --- test/es-module/test-typescript-commonjs.mjs | 57 ++++++++++----------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index a6fe1c0a62a8c5..46689f0ec56530 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -1,18 +1,18 @@ import { skip, spawnPromisified } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; -import assert, { match, strictEqual } from 'node:assert'; +import assert, { match, strictEqual } from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); test('require a .ts file with explicit extension succeeds', async () => { - const result = await spawnPromisified( - process.execPath, - ['--eval', 'require("./test-typescript.ts")', '--no-warnings'], - { - cwd: fixtures.path('typescript/ts'), - } - ); + const result = await spawnPromisified(process.execPath, [ + '--eval', + 'require("./test-typescript.ts")', + '--no-warnings', + ], { + cwd: fixtures.path('typescript/ts'), + }); strictEqual(result.stderr, ''); strictEqual(result.stdout, 'Hello, TypeScript!\n'); @@ -20,13 +20,13 @@ test('require a .ts file with explicit extension succeeds', async () => { }); test('eval require a .ts file with implicit extension fails', async () => { - const result = await spawnPromisified( - process.execPath, - ['--eval', 'require("./test-typescript")', '--no-warnings'], - { - cwd: fixtures.path('typescript/ts'), - } - ); + const result = await spawnPromisified(process.execPath, [ + '--eval', + 'require("./test-typescript")', + '--no-warnings', + ], { + cwd: fixtures.path('typescript/ts'), + }); strictEqual(result.stdout, ''); match(result.stderr, /Error: Cannot find module/); @@ -34,13 +34,13 @@ test('eval require a .ts file with implicit extension fails', async () => { }); test('eval require a .cts file with implicit extension fails', async () => { - const result = await spawnPromisified( - process.execPath, - ['--eval', 'require("./test-cts-typescript")', '--no-warnings'], - { - cwd: fixtures.path('typescript/ts'), - } - ); + const result = await spawnPromisified(process.execPath, [ + '--eval', + 'require("./test-cts-typescript")', + '--no-warnings', + ], { + cwd: fixtures.path('typescript/ts'), + }); strictEqual(result.stdout, ''); match(result.stderr, /Error: Cannot find module/); @@ -59,20 +59,15 @@ test('require a .ts file with implicit extension fails', async () => { }); test('expect failure of an .mts file with CommonJS syntax', async () => { - const testFilePath = fixtures.path( - 'typescript/cts/test-cts-but-module-syntax.cts' - ); + const testFilePath = fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'); const result = await spawnPromisified(process.execPath, [testFilePath]); strictEqual(result.stdout, ''); - const expectedWarning = - 'To load an ES module, set "type": "module" in the package.json or use the .mjs extension.'; + const expectedWarning = "To load an ES module, set \"type\": \"module\" in the package.json or use the .mjs extension."; try { - assert.ok( - result.stderr.includes(expectedWarning), - `stderr does not contain the expected warning. Actual stderr: ${result.stderr}` - ); + + assert.ok(result.stderr.includes(expectedWarning), `stderr does not contain the expected warning. Actual stderr: ${result.stderr}`); } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning; From 72da5692a92ec22c42bd9f67c1658c7e42aafd09 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sat, 15 Feb 2025 23:26:19 +0300 Subject: [PATCH 11/22] lint --- test/es-module/test-typescript-commonjs.mjs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 46689f0ec56530..bd7737be28f138 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -1,6 +1,6 @@ import { skip, spawnPromisified } from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; -import assert, { match, strictEqual } from 'node:assert'; +import assert, { match, strictEqual } from 'node:assert'; import { test } from 'node:test'; if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); @@ -64,10 +64,10 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { strictEqual(result.stdout, ''); - const expectedWarning = "To load an ES module, set \"type\": \"module\" in the package.json or use the .mjs extension."; + const expectedWarning = 'To load an ES module, set "type": "module" in the package.json or use the .mjs extension.'; try { - assert.ok(result.stderr.includes(expectedWarning), `stderr does not contain the expected warning. Actual stderr: ${result.stderr}`); + assert.ok(result.stderr.includes(expectedWarning), `stderr does not contain the expected warning. Actual stderr: ${result.stderr}`); } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning; From 3929988215e874e54420110b0d767ca2185088a0 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 16 Feb 2025 19:07:28 +0300 Subject: [PATCH 12/22] undo irrelevant changes --- test/es-module/test-typescript-commonjs.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index bd7737be28f138..8d4f027504669e 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -62,9 +62,9 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { const testFilePath = fixtures.path('typescript/cts/test-cts-but-module-syntax.cts'); const result = await spawnPromisified(process.execPath, [testFilePath]); - strictEqual(result.stdout, ''); + assert.strictEqual(result.stdout, ''); - const expectedWarning = 'To load an ES module, set "type": "module" in the package.json or use the .mjs extension.'; + 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), `stderr does not contain the expected warning. Actual stderr: ${result.stderr}`); From de2dfaa6a572ac1de5317f173a1c2aae566cb91e Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 16 Feb 2025 19:22:31 +0300 Subject: [PATCH 13/22] Update src/node_process_events.cc Co-authored-by: James M Snell --- src/node_process_events.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/node_process_events.cc b/src/node_process_events.cc index c77097c75b1e7f..f99ef407e0f3c6 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -21,12 +21,14 @@ using v8::Value; Maybe ProcessEmitWarningSync(Environment* env, std::string_view message) { Isolate* isolate = env->isolate(); Local context = env->context(); - v8::Local message_string = - v8::String::NewFromUtf8(isolate, + Local message_string; + if (!String::NewFromUtf8(isolate, message.data(), - v8::NewStringType::kNormal, + NewStringType::kNormal, static_cast(message.size())) - .ToLocalChecked(); + .ToLocal(&message_string)) { + return Nothing(); + } Local argv[] = {message_string}; Local emit_function = env->process_emit_warning_sync(); From e30094e6c9dec97297d51983356d34237f98561e Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 16 Feb 2025 19:32:37 +0300 Subject: [PATCH 14/22] using v8 NewStringType --- src/node_process_events.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_process_events.cc b/src/node_process_events.cc index f99ef407e0f3c6..177fcd83d30da6 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -17,6 +17,7 @@ using v8::Nothing; using v8::Object; using v8::String; using v8::Value; +using v8::NewStringType; Maybe ProcessEmitWarningSync(Environment* env, std::string_view message) { Isolate* isolate = env->isolate(); From b812ebc59c8ec73823d4c5fdee7ca0a2ead155b6 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 16 Feb 2025 19:32:37 +0300 Subject: [PATCH 15/22] sort edited --- src/node_process_events.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_process_events.cc b/src/node_process_events.cc index f99ef407e0f3c6..eb250f84e33aea 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -13,6 +13,7 @@ using v8::Just; using v8::Local; using v8::Maybe; using v8::MaybeLocal; +using v8::NewStringType; using v8::Nothing; using v8::Object; using v8::String; From 411b0502288325cb7f31a7d612bda62b16e8fc87 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 16 Feb 2025 19:41:16 +0300 Subject: [PATCH 16/22] lint --- src/node_process_events.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/node_process_events.cc b/src/node_process_events.cc index eb250f84e33aea..6f18f3f7e7f654 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -24,10 +24,10 @@ Maybe ProcessEmitWarningSync(Environment* env, std::string_view message) { Local context = env->context(); Local message_string; if (!String::NewFromUtf8(isolate, - message.data(), - NewStringType::kNormal, - static_cast(message.size())) - .ToLocal(&message_string)) { + message.data(), + NewStringType::kNormal, + static_cast(message.size())) + .ToLocal(&message_string)) { return Nothing(); } From 96d60388fa5f60fcd7a6bcad54269a9c059c66ef Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Wed, 19 Feb 2025 01:09:41 +0300 Subject: [PATCH 17/22] update messages for windows --- test/es-module/test-esm-long-path-win.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/es-module/test-esm-long-path-win.js b/test/es-module/test-esm-long-path-win.js index ef47759698992c..fca22172a85995 100644 --- a/test/es-module/test-esm-long-path-win.js +++ b/test/es-module/test-esm-long-path-win.js @@ -182,7 +182,7 @@ describe('long path on Windows', () => { fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";'); 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(signal, null); }); From 1c133c94ab9e16f59e9fdbf81eb16ee5c261cbed Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Wed, 19 Feb 2025 12:55:13 +0300 Subject: [PATCH 18/22] Update test-typescript-commonjs.mjs Co-authored-by: Antoine du Hamel --- test/es-module/test-typescript-commonjs.mjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 8d4f027504669e..c721c14a7efcac 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -66,8 +66,7 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { 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), `stderr does not contain the expected warning. Actual stderr: ${result.stderr}`); + assert.ok(result.stderr.includes(expectedWarning)); } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning; From 001a8230c1bc7cd6a30df62d1231c620bec287f7 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Wed, 19 Feb 2025 17:54:48 +0300 Subject: [PATCH 19/22] use regexp for CommonJS error --- test/es-module/test-typescript-commonjs.mjs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index c721c14a7efcac..0be74665d1bb99 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -66,7 +66,13 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { 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)); + assert.match( + result.stderr, + new RegExp( + `Failed to load the ES module: ${testFilePath}\\. Make sure to set "type": "module" in the nearest package\\.json file or use the \\.mjs extension\\.` + ) + ); + } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning; From 2f8aeb6873298124085ab91703b8db30279c2ed3 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Wed, 19 Feb 2025 17:56:29 +0300 Subject: [PATCH 20/22] lint --- test/es-module/test-typescript-commonjs.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 0be74665d1bb99..aadb74df8d795d 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -72,7 +72,7 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { `Failed to load the ES module: ${testFilePath}\\. Make sure to set "type": "module" in the nearest package\\.json file or use the \\.mjs extension\\.` ) ); - + } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning; From 0d2eda8e55eb5a7e7a0598e9d04d53e1d0abf8ad Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sun, 23 Feb 2025 19:20:49 +0300 Subject: [PATCH 21/22] fix: properly escape testFilePath in regex assertion --- test/es-module/test-typescript-commonjs.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index aadb74df8d795d..1b71d4dc5078a4 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -69,7 +69,8 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { assert.match( result.stderr, new RegExp( - `Failed to load the ES module: ${testFilePath}\\. Make sure to set "type": "module" in the nearest package\\.json file or use the \\.mjs extension\\.` + `Failed to load the ES module: ${testFilePath}\\. Make sure to set "type": "module" ` + + 'in the nearest package\\.json file or use the \\.mjs extension\\.' ) ); From 423640a658922e30796059a371a1e47f13ecd8bf Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Mon, 24 Feb 2025 22:50:14 +0300 Subject: [PATCH 22/22] revert regex suggestion --- test/es-module/test-typescript-commonjs.mjs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 1b71d4dc5078a4..c721c14a7efcac 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -66,14 +66,7 @@ test('expect failure of an .mts file with CommonJS syntax', async () => { 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.match( - result.stderr, - new RegExp( - `Failed to load the ES module: ${testFilePath}\\. Make sure to set "type": "module" ` + - 'in the nearest package\\.json file or use the \\.mjs extension\\.' - ) - ); - + assert.ok(result.stderr.includes(expectedWarning)); } catch (e) { if (e?.code === 'ERR_ASSERTION') { e.expected = expectedWarning;