search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.75k stars 28.3k forks source link

cli: add `NODE_RUN_SCRIPT_NAME` env to `node --run` #53032

Closed anonrig closed 3 weeks ago

anonrig commented 4 weeks ago

Adds the NODE_RUN_SCRIPT_NAME to share name of the event that's run while executing node --run. For example, if the developer runs node --run yagiz, NODE_RUN_SCRIPT_NAME will be yagiz

This PR also includes a small refactor. I moved setting environment variables into SetEnvironmentVariables private function to improve readability.

Ref: https://github.com/nodejs/node/issues/52673

cc @nodejs/cpp-reviewers

nodejs-github-bot commented 4 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59261/

nodejs-github-bot commented 4 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59262/

targos commented 4 weeks ago

I don't understand the name. What's the connection between running a script and the concept of a "lifecycle event"?

anonrig commented 4 weeks ago

I don't understand the name. What's the connection between running a script and the concept of a "lifecycle event"?

I don't understand it either. It's named after "npm_lifecycle_event" environment variable that is required for CLI runners to detect which task is being run. The reference issue in PR description might give you more context.

tniessen commented 4 weeks ago

Lifecycle events make sense in the context of a package manager, but since this feature apparently doesn't aim for compatibility with any existing package manager or runtime, I think it'd be acceptable to deviate from existing naming conventions.

anonrig commented 4 weeks ago

Lifecycle events make sense in the context of a package manager, but since this feature apparently doesn't aim for compatibility with any existing package manager or runtime, I think it'd be acceptable to deviate from existing naming conventions.

That seems valid to me. @tniessen. Any naming suggestions?

legendecas commented 4 weeks ago

An environ named as something like NODE_RUN_COMMAND_NAME would make more sense to me.

legendecas commented 3 weeks ago

Sorry about the churn. Do we have a consensus on "command" vs "script"? I found both in the doc and codebase, but node --run parses the package.json#scripts and runs a script.

tniessen commented 3 weeks ago

I'm not necessarily in favor of any of this, but script sounds better than command to me.

nodejs-github-bot commented 3 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59275/

benjamingr commented 3 weeks ago

(the ask and solution seem reasonable to me)

nodejs-github-bot commented 3 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59288/

nodejs-github-bot commented 3 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59292/

nodejs-github-bot commented 3 weeks ago

Landed in cb90a316d020a8188d225e5cd9d03dedac4b14b1