Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: support eval with ts syntax detection #56285

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1370,8 +1370,15 @@ added: v12.0.0
-->

This configures Node.js to interpret `--eval` or `STDIN` input as CommonJS or
as an ES module. Valid values are `"commonjs"` or `"module"`. The default is
`"commonjs"`.
as an ES module. Valid values are `"commonjs"`, `"module"`, `"module-typescript"` and `"commonjs-typescript"`.
The `"-typescript"` values are available only in combination with the flag `--experimental-strip-types`.
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
The default is `"commonjs"`.

If `--experimental-strip-types` is enabled and `--input-type` is not provided,
Node.js will first try to run the code as JavaScript,
then it will try to run the code as TypeScript.
Comment on lines +1378 to +1379
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably say something about syntax detection here, that first it attempts to run as CommonJS JavaScript, and depending on the parse error thrown, will retry as ESM JavaScript or CommonJS TypeScript; and then potentially a third time as ESM TypeScript? I’m not even sure what the flow would be, which probably means that we should spell it out 😄

Copy link
Member Author

@marco-ippolito marco-ippolito Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert on how the js syntax detection is implemented but the flow is:

  • run normally as javascript (this will perform the detection by trying cjs and esm)
  • if invalid syntax, strip the types and try again
  • if invalid again throw the first invalid syntax error enriched with the typescript message

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The details of syntax detection are here: https://github.com/nodejs/node/blob/6012a4e9a2733b25a7023baa0a58bef85c9ebc5f/doc/api/esm.md#:~:text=at%20pjsonURL.-,DETECT_MODULE_SYNTAX,-(source)

The important part is to clarify which specific errors will trigger strip-types behavior. I assume there’s no overlap between the “retry as ESM” errors and the “retry with type stripping” errors, so the flow is no more expensive than the current detection flow: try first as CommonJS JavaScript, and retry as stripped types.

It gets even more complicated because doesn’t the strip-types flow itself use syntax detection? Like after it strips the types, then it initially tries to run as CommonJS, then retries as ESM?

Copy link
Member Author

@marco-ippolito marco-ippolito Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it repeats the detection twice but its trasparent from my implementation. I'm not sure it can be skipped

To avoid the delay of multiple syntax detection passes, the `--input-type=type` flag can be used to specify
how the `--eval` input should be interpreted.

The REPL does not support this option. Usage of `--input-type=module` with
[`--print`][] will throw an error, as `--print` does not support ES module
Expand Down
49 changes: 34 additions & 15 deletions lib/internal/main/eval_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,34 @@ const {
prepareMainThreadExecution,
markBootstrapComplete,
} = require('internal/process/pre_execution');
const { evalModuleEntryPoint, evalScript } = require('internal/process/execution');
const {
evalModuleEntryPoint,
evalTypeScript,
parseAndEvalCommonjsTypeScript,
parseAndEvalModuleTypeScript,
evalScript,
} = require('internal/process/execution');
const { addBuiltinLibsToObject } = require('internal/modules/helpers');
const { stripTypeScriptModuleTypes } = require('internal/modules/typescript');
const { getOptionValue } = require('internal/options');

prepareMainThreadExecution();
addBuiltinLibsToObject(globalThis, '<eval>');
markBootstrapComplete();

const code = getOptionValue('--eval');
const source = getOptionValue('--experimental-strip-types') ?
stripTypeScriptModuleTypes(code) :
code;

const print = getOptionValue('--print');
const shouldLoadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
if (getOptionValue('--input-type') === 'module') {
evalModuleEntryPoint(source, print);
const inputType = getOptionValue('--input-type');
const tsEnabled = getOptionValue('--experimental-strip-types');
if (inputType === 'module') {
evalModuleEntryPoint(code, print);
} else if (inputType === 'module-typescript' && tsEnabled) {
parseAndEvalModuleTypeScript(code, print);
} else {
// For backward compatibility, we want the identifier crypto to be the
// `node:crypto` module rather than WebCrypto.
const isUsingCryptoIdentifier = RegExpPrototypeExec(/\bcrypto\b/, source) !== null;
const isUsingCryptoIdentifier = RegExpPrototypeExec(/\bcrypto\b/, code) !== null;
const shouldDefineCrypto = isUsingCryptoIdentifier && internalBinding('config').hasOpenSSL;

if (isUsingCryptoIdentifier && !shouldDefineCrypto) {
Expand All @@ -49,11 +55,24 @@ if (getOptionValue('--input-type') === 'module') {
};
ObjectDefineProperty(object, name, { __proto__: null, set: setReal });
}
evalScript('[eval]',
shouldDefineCrypto ? (
print ? `let crypto=require("node:crypto");{${source}}` : `(crypto=>{{${source}}})(require('node:crypto'))`
) : source,
getOptionValue('--inspect-brk'),
print,
shouldLoadESM);

let evalFunction;
if (inputType === 'commonjs') {
evalFunction = evalScript;
} else if (inputType === 'commonjs-typescript' && tsEnabled) {
evalFunction = parseAndEvalCommonjsTypeScript;
} else if (tsEnabled) {
evalFunction = evalTypeScript;
} else {
// Default to commonjs.
evalFunction = evalScript;
}

evalFunction('[eval]',
shouldDefineCrypto ? (
print ? `let crypto=require("node:crypto");{${code}}` : `(crypto=>{{${code}}})(require('node:crypto'))`
) : code,
getOptionValue('--inspect-brk'),
print,
shouldLoadESM);
}
1 change: 0 additions & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ function initializeCJS() {

const tsEnabled = getOptionValue('--experimental-strip-types');
if (tsEnabled) {
emitExperimentalWarning('Type Stripping');
Module._extensions['.cts'] = loadCTS;
Module._extensions['.ts'] = loadTS;
}
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ translators.set('require-commonjs', (url, source, isMain) => {
// Handle CommonJS modules referenced by `require` calls.
// This translator function must be sync, as `require` is sync.
translators.set('require-commonjs-typescript', (url, source, isMain) => {
emitExperimentalWarning('Type Stripping');
assert(cjsParse);
const code = stripTypeScriptModuleTypes(stringify(source), url);
return createCJSModuleWrap(url, code, isMain, 'commonjs-typescript');
Expand Down Expand Up @@ -464,7 +463,6 @@ translators.set('wasm', async function(url, source) {

// Strategy for loading a commonjs TypeScript module
translators.set('commonjs-typescript', function(url, source) {
emitExperimentalWarning('Type Stripping');
assertBufferSource(source, true, 'load');
const code = stripTypeScriptModuleTypes(stringify(source), url);
debug(`Translating TypeScript ${url}`);
Expand All @@ -473,7 +471,6 @@ translators.set('commonjs-typescript', function(url, source) {

// Strategy for loading an esm TypeScript module
translators.set('module-typescript', function(url, source) {
emitExperimentalWarning('Type Stripping');
assertBufferSource(source, true, 'load');
const code = stripTypeScriptModuleTypes(stringify(source), url);
debug(`Translating TypeScript ${url}`);
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/modules/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,13 @@ function processTypeScriptCode(code, options) {
* It is used by internal loaders.
* @param {string} source TypeScript code to parse.
* @param {string} filename The filename of the source code.
* @param {boolean} emitWarning Whether to emit a warning.
* @returns {TransformOutput} The stripped TypeScript code.
*/
function stripTypeScriptModuleTypes(source, filename) {
function stripTypeScriptModuleTypes(source, filename, emitWarning = true) {
if (emitWarning) {
emitExperimentalWarning('Type Stripping');
}
assert(typeof source === 'string');
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
Expand Down
118 changes: 118 additions & 0 deletions lib/internal/process/execution.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
'use strict';

const {
ArrayPrototypeJoin,
ArrayPrototypeSplice,
RegExpPrototypeExec,
StringPrototypeSplit,
Symbol,
globalThis,
} = primordials;
Expand All @@ -17,6 +20,7 @@ const {
} = require('internal/errors');
const { pathToFileURL } = require('internal/url');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
const { stripTypeScriptModuleTypes } = require('internal/modules/typescript');

const {
executionAsyncId,
Expand All @@ -32,6 +36,7 @@ const { getOptionValue } = require('internal/options');
const {
makeContextifyScript, runScriptInThisContext,
} = require('internal/vm');
const { emitExperimentalWarning } = require('internal/util');
// shouldAbortOnUncaughtToggle is a typed array for faster
// communication with JS.
const { shouldAbortOnUncaughtToggle } = internalBinding('util');
Expand Down Expand Up @@ -84,6 +89,9 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
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);
}

Expand Down Expand Up @@ -238,10 +246,120 @@ function readStdin(callback) {
});
}

/**
*
* Adds the TS message to the error stack
* At the 3rd line of the stack, the message is added
* @param {Error} originalStack the stack to decorate
* @param {string} newMessage the message to add to the error stack
* @returns {void}
*/
function decorateCJSErrorWithTSMessage(originalStack, newMessage) {
const lines = StringPrototypeSplit(originalStack, '\n');
ArrayPrototypeSplice(lines, 3, 0, newMessage);
return ArrayPrototypeJoin(lines, '\n');
}

/**
*
* Wrapper of evalScript
*
* This function wraps the evaluation of the source code in a try-catch block.
* If the source code fails to be evaluated, it will retry evaluating the source code
* with the TypeScript parser.
*
* If the source code fails to be evaluated with the TypeScript parser,
* it will rethrow the original error, adding the TypeScript error message to the stack.
*
* This way we don't change the behavior of the code, but we provide a better error message
* in case of a typescript error.
*/
function evalTypeScript(name, source, breakFirstLine, print, shouldLoadESM = false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add the params to the JSDoc?

try {
evalScript(name, source, breakFirstLine, print, shouldLoadESM);
} catch (originalError) {
try {
const strippedSource = stripTypeScriptModuleTypes(source, name, false);
evalScript(name, strippedSource, breakFirstLine, print, shouldLoadESM);
// Emit the experimental warning after the code was successfully evaluated.
emitExperimentalWarning('Type Stripping');
} catch (tsError) {
if (tsError.code === 'ERR_INVALID_TYPESCRIPT_SYNTAX') {
originalError.stack = decorateCJSErrorWithTSMessage(originalError.stack, tsError.message);
}
throw originalError;
}
}
}

/**
* Wrapper of evalModuleEntryPoint
*
* This function wraps the evaluation of the source code in a try-catch block.
* If the source code fails to be evaluated, it will retry evaluating the source code
* with the TypeScript parser.
*
*/
function evalTypeScriptModuleEntryPoint(source, print) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add the params to the JSDoc?

if (print) {
throw new ERR_EVAL_ESM_CANNOT_PRINT();
}

RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs.

return require('internal/modules/run_main').runEntryPointWithESMLoader(
async (loader) => {
try {
// Await here to catch the error and rethrow it with the typescript error message.
return await loader.eval(source, getEvalModuleUrl(), true);
} catch (originalError) {
try {
const url = getEvalModuleUrl();
const strippedSource = stripTypeScriptModuleTypes(source, url, false);
const result = await loader.eval(strippedSource, url, true);
// Emit the experimental warning after the code was successfully evaluated.
emitExperimentalWarning('Type Stripping');
return result;
} catch (tsError) {
if (tsError.code === 'ERR_INVALID_TYPESCRIPT_SYNTAX') {
originalError.stack = `${tsError.message}\n\n${originalError.stack}`;
}
throw originalError;
}
}
},
);
};

/**
*
* Function used to shortcut when `--input-type=module-typescript` is set.
* @param {string} source
* @param {boolean} print
*/
function parseAndEvalModuleTypeScript(source, print) {
// We know its a TypeScript module, we can safely emit the experimental warning.
const strippedSource = stripTypeScriptModuleTypes(source, getEvalModuleUrl());
evalModuleEntryPoint(strippedSource, print);
}

/**
* Function used to shortcut when `--input-type=commonjs-typescript` is set
* See evalScript signature
*/
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());
evalScript(name, strippedSource, breakFirstLine, print, shouldLoadESM);
}

module.exports = {
parseAndEvalCommonjsTypeScript,
parseAndEvalModuleTypeScript,
readStdin,
tryGetCwd,
evalModuleEntryPoint,
evalTypeScript,
evalScript,
onGlobalUncaughtException: createOnGlobalUncaughtException(),
setUncaughtExceptionCaptureCallback,
Expand Down
8 changes: 6 additions & 2 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,12 @@ void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors,
void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
std::vector<std::string>* argv) {
if (!input_type.empty()) {
if (input_type != "commonjs" && input_type != "module") {
errors->push_back("--input-type must be \"module\" or \"commonjs\"");
if (input_type != "commonjs" && input_type != "module" &&
input_type != "commonjs-typescript" &&
input_type != "module-typescript") {
errors->push_back(
"--input-type must be \"module\","
"\"commonjs\", \"module-typescript\" or \"commonjs-typescript\"");
}
}

Expand Down
Loading
Loading