From 38d78e4518a1cfed4b287365e6a0e9e16ece01fb Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Thu, 19 Dec 2024 18:45:47 +0100 Subject: [PATCH] fixup! split CJS compile - execute --- lib/internal/process/execution.js | 167 +++++++++++++++++++----- test/es-module/test-typescript-eval.mjs | 24 +++- 2 files changed, 149 insertions(+), 42 deletions(-) diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 52db57042499fc..e8a1a29efcc8a6 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -74,24 +74,14 @@ function evalModuleEntryPoint(source, print) { } function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { - const CJSModule = require('internal/modules/cjs/loader').Module; - - const cwd = tryGetCwd(); const origModule = globalThis.module; // Set e.g. when called from the REPL. - - const module = new CJSModule(name); - module.filename = path.join(cwd, name); - module.paths = CJSModule._nodeModulePaths(cwd); - + const module = createModule(name); const baseUrl = pathToFileURL(module.filename).href; - if (getOptionValue('--experimental-detect-module') && - getOptionValue('--input-type') === '' && - containsModuleSyntax(body, name, null, 'no CJS variables')) { - if (getOptionValue('--experimental-strip-types')) { - return evalTypeScriptModuleEntryPoint(body, print); - } - return evalModuleEntryPoint(body, print); + if (shouldUseModuleEntryPoint(name, body)) { + return getOptionValue('--experimental-strip-types') ? + evalTypeScriptModuleEntryPoint(body, print) : + evalModuleEntryPoint(body, print); } const runScript = () => { @@ -106,23 +96,8 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { globalThis.__filename = name; RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. const result = module._compile(script, `${name}-wrapper`)(() => { - const hostDefinedOptionId = Symbol(name); - async function importModuleDynamically(specifier, _, importAttributes) { - const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); - return cascadedLoader.import(specifier, baseUrl, importAttributes); - } - const script = makeContextifyScript( - body, // code - name, // filename, - 0, // lineOffset - 0, // columnOffset, - undefined, // cachedData - false, // produceCachedData - undefined, // parsingContext - hostDefinedOptionId, // hostDefinedOptionId - importModuleDynamically, // importModuleDynamically - ); - return runScriptInThisContext(script, true, !!breakFirstLine); + const compiledScript = compileScript(name, body, baseUrl); + return runScriptInThisContext(compiledScript, true, !!breakFirstLine); }); if (print) { const { log } = require('internal/console/global'); @@ -284,16 +259,33 @@ function decorateCJSErrorWithTSMessage(originalStack, newMessage) { * @returns {void} */ function evalTypeScript(name, source, breakFirstLine, print, shouldLoadESM = false) { + const origModule = globalThis.module; // Set e.g. when called from the REPL. + const module = createModule(name); + const baseUrl = pathToFileURL(module.filename).href; + + if (shouldUseModuleEntryPoint(name, source)) { + return evalTypeScriptModuleEntryPoint(source, print); + } + + let compiledScript; + // This variable can be modified if the source code is stripped. + let sourceToRun = source; try { - evalScript(name, source, breakFirstLine, print, shouldLoadESM); + compiledScript = compileScript(name, source, baseUrl); } catch (originalError) { // If it's not a SyntaxError, rethrow it. if (!isError(originalError) || originalError.name !== 'SyntaxError') { throw originalError; } try { - const strippedSource = stripTypeScriptModuleTypes(source, name, false); - evalScript(name, strippedSource, breakFirstLine, print, shouldLoadESM); + sourceToRun = stripTypeScriptModuleTypes(source, name, false); + // Retry the CJS/ESM syntax detection after stripping the types. + if (shouldUseModuleEntryPoint(name, sourceToRun)) { + return evalTypeScriptModuleEntryPoint(source, print); + } + // If the ContextifiedScript was successfully created, execute it. + // outside the try-catch block to avoid catching runtime errors. + compiledScript = compileScript(name, sourceToRun, baseUrl); // Emit the experimental warning after the code was successfully evaluated. emitExperimentalWarning('Type Stripping'); } catch (tsError) { @@ -308,6 +300,20 @@ function evalTypeScript(name, source, breakFirstLine, print, shouldLoadESM = fal throw originalError; } } + + if (shouldLoadESM) { + return require('internal/modules/run_main').runEntryPointWithESMLoader( + () => runScriptInContext(name, + sourceToRun, + breakFirstLine, + print, + module, + baseUrl, + compiledScript, + origModule)); + } + + runScriptInContext(name, sourceToRun, breakFirstLine, print, module, baseUrl, compiledScript, origModule); } /** @@ -393,6 +399,97 @@ function parseAndEvalCommonjsTypeScript(name, source, breakFirstLine, print, sho evalScript(name, strippedSource, breakFirstLine, print, shouldLoadESM); } +/** + * + * @param {string} name - The filename of the script. + * @param {string} body - The code of the script. + * @param {string} baseUrl Path of the parent importing the module. + * @returns {ContextifyScript} The created contextify script. + */ +function compileScript(name, body, baseUrl) { + const hostDefinedOptionId = Symbol(name); + async function importModuleDynamically(specifier, _, importAttributes) { + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + return cascadedLoader.import(specifier, baseUrl, importAttributes); + } + return makeContextifyScript( + body, // code + name, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); +} + +/** + * @param {string} name - The filename of the script. + * @param {string} body - The code of the script. + * @returns {boolean} Whether the module entry point should be evaluated as a module. + */ +function shouldUseModuleEntryPoint(name, body) { + return getOptionValue('--experimental-detect-module') && + getOptionValue('--input-type') === '' && + containsModuleSyntax(body, name, null, 'no CJS variables'); +} + +/** + * + * @param {string} name - The filename of the script. + * @returns {import('internal/modules/esm/loader').CJSModule} The created module. + */ +function createModule(name) { + const CJSModule = require('internal/modules/cjs/loader').Module; + const cwd = tryGetCwd(); + const module = new CJSModule(name); + module.filename = path.join(cwd, name); + module.paths = CJSModule._nodeModulePaths(cwd); + return module; +} + +/** + * + * @param {string} name - The filename of the script. + * @param {string} body - The code of the script. + * @param {boolean} breakFirstLine Whether to break on the first line + * @param {boolean} print If the result should be printed + * @param {import('internal/modules/esm/loader').CJSModule} module The module + * @param {string} baseUrl Path of the parent importing the module. + * @param {object} compiledScript The compiled script. + * @param {any} origModule The original module. + * @returns {void} + */ +function runScriptInContext(name, body, breakFirstLine, print, module, baseUrl, compiledScript, origModule) { + // Create wrapper for cache entry + const script = ` + globalThis.module = module; + globalThis.exports = exports; + globalThis.__dirname = __dirname; + globalThis.require = require; + return (main) => main(); + `; + globalThis.__filename = name; + RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. + const result = module._compile(script, `${name}-wrapper`)(() => { + // If the script was already compiled, use it. + return runScriptInThisContext( + compiledScript, + true, !!breakFirstLine); + }); + if (print) { + const { log } = require('internal/console/global'); + + process.on('exit', () => { + log(result); + }); + } + if (origModule !== undefined) + globalThis.module = origModule; +} + module.exports = { parseAndEvalCommonjsTypeScript, parseAndEvalModuleTypeScript, diff --git a/test/es-module/test-typescript-eval.mjs b/test/es-module/test-typescript-eval.mjs index 03bafffcaa118f..64e334e001beac 100644 --- a/test/es-module/test-typescript-eval.mjs +++ b/test/es-module/test-typescript-eval.mjs @@ -181,16 +181,12 @@ test('check warning is emitted when eval TypeScript CommonJS syntax', async () = strictEqual(result.code, 0); }); -test('code is throwing a non Error', async () => { +test('code is throwing a non Error is rethrown', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', '--eval', `throw null;`]); - // TODO(marco-ippolito) fix the stack trace of non errors - // being re-thrown - // the stack trace is wrong because it is rethrown - // but it's not an Error object - match(result.stderr, /node:internal\/process\/execution/); + doesNotMatch(result.stderr, /node:internal\/process\/execution/); strictEqual(result.stdout, ''); strictEqual(result.code, 1); }); @@ -223,7 +219,21 @@ test('typescript ESM code is throwing a syntax error at runtime', async () => { '--experimental-strip-types', '--eval', 'import util from "node:util"; function foo(){}; throw new SyntaxError(foo(1));']); - // Trick by passing ambiguous syntax to trigger to see if evaluated in TypeScript or JavaScript + // Trick by passing ambiguous syntax to see if evaluated in TypeScript or JavaScript + // If evaluated in JavaScript `foo(1)` is evaluated as `foo < Number > (1)` + // result in false + // If evaluated in TypeScript `foo(1)` is evaluated as `foo(1)` + match(result.stderr, /SyntaxError: false/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); + +test('typescript CJS code is throwing a syntax error at runtime', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--eval', + 'const util = require("node:util"); function foo(){}; throw new SyntaxError(foo(1));']); + // Trick by passing ambiguous syntax to see if evaluated in TypeScript or JavaScript // If evaluated in JavaScript `foo(1)` is evaluated as `foo < Number > (1)` // result in false // If evaluated in TypeScript `foo(1)` is evaluated as `foo(1)`