From 1c2d98d380951d654e290e7d79257534af58a75f Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Sun, 23 Mar 2025 19:17:55 +0100 Subject: [PATCH] module: improve typescript error message format PR-URL: https://github.com/nodejs/node/pull/57687 Fixes: https://github.com/nodejs/node/issues/56830 Reviewed-By: Chengzhong Wu Reviewed-By: Antoine du Hamel Reviewed-By: Rafael Gonzaga --- lib/internal/modules/typescript.js | 24 ++++++++-- lib/internal/process/execution.js | 10 ++-- test/es-module/test-typescript-eval.mjs | 20 ++++++++ test/fixtures/eval/eval_messages.snapshot | 6 +-- test/fixtures/eval/eval_typescript.snapshot | 24 ++-------- test/fixtures/eval/stdin_messages.snapshot | 6 +-- test/fixtures/eval/stdin_typescript.snapshot | 48 ++++---------------- 7 files changed, 60 insertions(+), 78 deletions(-) diff --git a/lib/internal/modules/typescript.js b/lib/internal/modules/typescript.js index 17bbc6ba944..41bab6990a7 100644 --- a/lib/internal/modules/typescript.js +++ b/lib/internal/modules/typescript.js @@ -63,10 +63,14 @@ function parseTypeScript(source, options) { * It allows us to distinguish between invalid syntax and unsupported syntax. */ switch (error?.code) { - case 'UnsupportedSyntax': - throw new ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX(error.message); - case 'InvalidSyntax': - throw new ERR_INVALID_TYPESCRIPT_SYNTAX(error.message); + case 'UnsupportedSyntax': { + const unsupportedSyntaxError = new ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX(error.message); + throw decorateErrorWithSnippet(unsupportedSyntaxError, error); /* node-do-not-add-exception-line */ + } + case 'InvalidSyntax': { + const invalidSyntaxError = new ERR_INVALID_TYPESCRIPT_SYNTAX(error.message); + throw decorateErrorWithSnippet(invalidSyntaxError, error); /* node-do-not-add-exception-line */ + } default: // SWC may throw strings when something goes wrong. if (typeof error === 'string') { assert.fail(error); } @@ -76,6 +80,18 @@ function parseTypeScript(source, options) { } } +/** + * + * @param {Error} error the error to decorate: ERR_INVALID_TYPESCRIPT_SYNTAX, ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX + * @param {object} amaroError the error object from amaro + * @returns {Error} the decorated error + */ +function decorateErrorWithSnippet(error, amaroError) { + const errorHints = `${amaroError.filename}:${amaroError.startLine}${amaroError.snippet}`; + error.stack = `${errorHints}${error.stack}`; + return error; +} + /** * Performs type-stripping to TypeScript source code. * @param {string} code TypeScript code to parse. diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index c7f35759805..9ca9a13a6bd 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -44,6 +44,8 @@ const { emitExperimentalWarning } = require('internal/util'); // communication with JS. const { shouldAbortOnUncaughtToggle } = internalBinding('util'); +const kEvalTag = '[eval]'; + function tryGetCwd() { try { return process.cwd(); @@ -259,7 +261,7 @@ function evalTypeScript(name, source, breakFirstLine, print, shouldLoadESM = fal compiledScript = compileScript(name, source, baseUrl); } catch (originalError) { try { - sourceToRun = stripTypeScriptModuleTypes(source, name, false); + sourceToRun = stripTypeScriptModuleTypes(source, kEvalTag, false); // Retry the CJS/ESM syntax detection after stripping the types. if (shouldUseModuleEntryPoint(name, sourceToRun)) { return evalTypeScriptModuleEntryPoint(source, print); @@ -322,7 +324,7 @@ function evalTypeScriptModuleEntryPoint(source, print) { moduleWrap = loader.createModuleWrap(source, url); } catch (originalError) { try { - const strippedSource = stripTypeScriptModuleTypes(source, url, false); + const strippedSource = stripTypeScriptModuleTypes(source, kEvalTag, false); // If the moduleWrap was successfully created, execute the module job. // outside the try-catch block to avoid catching runtime errors. moduleWrap = loader.createModuleWrap(strippedSource, url); @@ -355,7 +357,7 @@ function evalTypeScriptModuleEntryPoint(source, print) { */ function parseAndEvalModuleTypeScript(source, print) { // We know its a TypeScript module, we can safely emit the experimental warning. - const strippedSource = stripTypeScriptModuleTypes(source, getEvalModuleUrl()); + const strippedSource = stripTypeScriptModuleTypes(source, kEvalTag); evalModuleEntryPoint(strippedSource, print); } @@ -370,7 +372,7 @@ function parseAndEvalModuleTypeScript(source, print) { */ function parseAndEvalCommonjsTypeScript(name, source, breakFirstLine, print, shouldLoadESM = false) { // We know its a TypeScript module, we can safely emit the experimental warning. - const strippedSource = stripTypeScriptModuleTypes(source, getEvalModuleUrl()); + const strippedSource = stripTypeScriptModuleTypes(source, kEvalTag); evalScript(name, strippedSource, breakFirstLine, print, shouldLoadESM); } diff --git a/test/es-module/test-typescript-eval.mjs b/test/es-module/test-typescript-eval.mjs index 44b9fc7f655..cdafd92c7f1 100644 --- a/test/es-module/test-typescript-eval.mjs +++ b/test/es-module/test-typescript-eval.mjs @@ -262,3 +262,23 @@ test('should not allow declare module keyword', async () => { match(result.stderr, /ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX/); strictEqual(result.code, 1); }); + +// TODO (marco-ippolito) Remove the extra padding from the error message +// The padding comes from swc it will be removed in a future amaro release +test('the error message should not contain extra padding', async () => { + const result = await spawnPromisified(process.execPath, [ + '--input-type=module-typescript', + '--eval', + 'declare module F { export type x = number }']); + strictEqual(result.stdout, ''); + // Windows uses \r\n as line endings + const lines = result.stderr.replace(/\r\n/g, '\n').split('\n'); + // The extra padding at the end should not be present + strictEqual(lines[0], '[eval]:1 '); + // The extra padding at the beginning should not be present + strictEqual(lines[2], ' declare module F { export type x = number }'); + strictEqual(lines[3], ' ^^^^^^^^'); + strictEqual(lines[5], 'SyntaxError [ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX]:' + + ' `module` keyword is not supported. Use `namespace` instead.'); + strictEqual(result.code, 1); +}); diff --git a/test/fixtures/eval/eval_messages.snapshot b/test/fixtures/eval/eval_messages.snapshot index 845ff4999be..a80c5eee8e6 100644 --- a/test/fixtures/eval/eval_messages.snapshot +++ b/test/fixtures/eval/eval_messages.snapshot @@ -2,11 +2,7 @@ [eval]:1 with(this){__filename} ^^^^ - x The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'. - ,---- - 1 | with(this){__filename} - : ^^^^ - `---- +The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'. SyntaxError: Strict mode code may not include a with statement diff --git a/test/fixtures/eval/eval_typescript.snapshot b/test/fixtures/eval/eval_typescript.snapshot index 44b29e3e967..df0c2211241 100644 --- a/test/fixtures/eval/eval_typescript.snapshot +++ b/test/fixtures/eval/eval_typescript.snapshot @@ -1,11 +1,7 @@ [eval]:1 enum Foo{}; ^^^^ - x TypeScript enum is not supported in strip-only mode - ,---- - 1 | enum Foo{}; - : ^^^^^^^^^^ - `---- +TypeScript enum is not supported in strip-only mode SyntaxError: Unexpected reserved word @@ -20,11 +16,7 @@ Node.js * [eval]:1 const foo; ^^^ - x 'const' declarations must be initialized - ,---- - 1 | const foo; - : ^^^ - `---- +'const' declarations must be initialized SyntaxError: Missing initializer in const declaration @@ -35,11 +27,7 @@ false [eval]:1 interface Foo{};const foo; ^^^ - x 'const' declarations must be initialized - ,---- - 1 | interface Foo{};const foo; - : ^^^ - `---- +'const' declarations must be initialized SyntaxError: Unexpected identifier 'Foo' @@ -47,11 +35,7 @@ Node.js * [eval]:1 function foo(){ await Promise.resolve(1)}; ^^^^^ - x await isn't allowed in non-async function - ,---- - 1 | function foo(){ await Promise.resolve(1)}; - : ^^^^^^^ - `---- +await isn't allowed in non-async function SyntaxError: await is only valid in async functions and the top level bodies of modules diff --git a/test/fixtures/eval/stdin_messages.snapshot b/test/fixtures/eval/stdin_messages.snapshot index 0382a6ae3cc..d7ec8a0d17c 100644 --- a/test/fixtures/eval/stdin_messages.snapshot +++ b/test/fixtures/eval/stdin_messages.snapshot @@ -2,11 +2,7 @@ [stdin]:1 with(this){__filename} ^^^^ - x The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'. - ,---- - 1 | with(this){__filename} - : ^^^^ - `---- +The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'. SyntaxError: Strict mode code may not include a with statement diff --git a/test/fixtures/eval/stdin_typescript.snapshot b/test/fixtures/eval/stdin_typescript.snapshot index accbb406e49..d693ec34f5a 100644 --- a/test/fixtures/eval/stdin_typescript.snapshot +++ b/test/fixtures/eval/stdin_typescript.snapshot @@ -1,11 +1,7 @@ [stdin]:1 enum Foo{}; ^^^^ - x TypeScript enum is not supported in strip-only mode - ,---- - 1 | enum Foo{}; - : ^^^^^^^^^^ - `---- +TypeScript enum is not supported in strip-only mode SyntaxError: Unexpected reserved word @@ -13,11 +9,7 @@ Node.js * [stdin]:1 enum Foo{}; ^^^^ - x TypeScript enum is not supported in strip-only mode - ,---- - 1 | enum Foo{}; - : ^^^^^^^^^^ - `---- +TypeScript enum is not supported in strip-only mode SyntaxError: Unexpected reserved word @@ -39,11 +31,7 @@ Node.js * [stdin]:1 const foo; ^^^ - x 'const' declarations must be initialized - ,---- - 1 | const foo; - : ^^^ - `---- +'const' declarations must be initialized SyntaxError: Missing initializer in const declaration @@ -51,11 +39,7 @@ Node.js * [stdin]:1 const foo; ^^^ - x 'const' declarations must be initialized - ,---- - 1 | const foo; - : ^^^ - `---- +'const' declarations must be initialized SyntaxError: Missing initializer in const declaration @@ -69,11 +53,7 @@ false [stdin]:1 interface Foo{};const foo; ^^^ - x 'const' declarations must be initialized - ,---- - 1 | interface Foo{};const foo; - : ^^^ - `---- +'const' declarations must be initialized SyntaxError: Unexpected identifier 'Foo' @@ -81,11 +61,7 @@ Node.js * [stdin]:1 interface Foo{};const foo; ^^^^^^^^^ - x 'const' declarations must be initialized - ,---- - 1 | interface Foo{};const foo; - : ^^^ - `---- +'const' declarations must be initialized SyntaxError: Unexpected strict mode reserved word @@ -93,11 +69,7 @@ Node.js * [stdin]:1 function foo(){ await Promise.resolve(1)}; ^^^^^ - x await isn't allowed in non-async function - ,---- - 1 | function foo(){ await Promise.resolve(1)}; - : ^^^^^^^ - `---- +await isn't allowed in non-async function SyntaxError: await is only valid in async functions and the top level bodies of modules @@ -105,11 +77,7 @@ Node.js * [stdin]:1 function foo(){ await Promise.resolve(1)}; ^^^^^ - x await isn't allowed in non-async function - ,---- - 1 | function foo(){ await Promise.resolve(1)}; - : ^^^^^^^ - `---- +await isn't allowed in non-async function SyntaxError: await is only valid in async functions and the top level bodies of modules