Skip to content

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

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
anonrig:add-node-lifecycle-event
May 19, 2024
Merged

cli: add NODE_RUN_SCRIPT_NAME env to node --run#53032
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
anonrig:add-node-lifecycle-event

Conversation

@anonrig

@anonriganonrig commented May 17, 2024•
edited
Loading

Copy link
Copy Markdown
Member

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: #52673

cc @nodejs/cpp-reviewers

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels May 17, 2024
@anonriganonrig mentioned this pull request May 17, 2024
Closed
@anonrig
anonrigforce-pushed the add-node-lifecycle-event branch from e5953cf to 69dbfd7CompareMay 17, 2024 01:16
@anonriganonrig added the semver-minor PRs that contain new features and should be released in the next minor version. label May 17, 2024
@anonrig
anonrigforce-pushed the add-node-lifecycle-event branch 2 times, most recently from d1dbd64 to f26e6ddCompareMay 17, 2024 02:14
@anonrig
anonrigforce-pushed the add-node-lifecycle-event branch from f26e6dd to 073643eCompareMay 17, 2024 02:16
@anonriganonrig added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 17, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 17, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@anonrig
anonrigforce-pushed the add-node-lifecycle-event branch from 073643e to 612181dCompareMay 17, 2024 03:04
@nodejs-github-bot

nodejs-github-bot commented May 17, 2024

Copy link
Copy Markdown
Collaborator

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

@targos

targos commented May 17, 2024

Copy link
Copy Markdown
Member

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

@anonrig

anonrig commented May 17, 2024

Copy link
Copy Markdown
MemberAuthor

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

tniessen commented May 17, 2024

Copy link
Copy Markdown
Member

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

anonrig commented May 17, 2024

Copy link
Copy Markdown
MemberAuthor

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

legendecas commented May 17, 2024

Copy link
Copy Markdown
Member

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

@anonriganonrig changed the title cli: add lifecycle event env to node --runcli: add NODE_RUN_COMMAND_NAME env to node --runMay 17, 2024
@anonrig
anonrigforce-pushed the add-node-lifecycle-event branch from 612181d to 5afff43CompareMay 17, 2024 20:09
@legendecas

legendecas commented May 17, 2024•
edited
Loading

Copy link
Copy Markdown
Member

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

tniessen commented May 17, 2024

Copy link
Copy Markdown
Member

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

@anonriganonrig changed the title cli: add NODE_RUN_COMMAND_NAME env to node --runcli: add NODE_RUN_SCRIPT_NAME env to node --runMay 17, 2024
@anonrig
anonrigforce-pushed the add-node-lifecycle-event branch from 5afff43 to 1ae34baCompareMay 17, 2024 23:56
@nodejs-github-bot

nodejs-github-bot commented May 18, 2024

Copy link
Copy Markdown
Collaborator

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

@benjamingr

benjamingr commented May 18, 2024

Copy link
Copy Markdown
Member

(the ask and solution seem reasonable to me)

@nodejs-github-bot

nodejs-github-bot commented May 18, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented May 18, 2024

Copy link
Copy Markdown
Collaborator

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

@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label May 18, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 19, 2024
@nodejs-github-bot
nodejs-github-bot merged commit cb90a31 into nodejs:mainMay 19, 2024
@nodejs-github-bot

nodejs-github-bot commented May 19, 2024

Copy link
Copy Markdown
Collaborator

Landed in cb90a31

targos pushed a commit that referenced this pull request May 21, 2024
@anonrig@targos
cli: add NODE_RUN_SCRIPT_NAME env to node --run
c70b2f7
PR-URL: #53032
Refs: #52673
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Jun 7, 2024
Merged
RafaelGSS added a commit that referenced this pull request Jun 7, 2024
@RafaelGSS
2024-06-11, Version 22.3.0 (Current)
f52be7b
Notable changes:
buffer:
* (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
* (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
* (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
* (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
* (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
* add StefanStojanovic to collaborators (StefanStojanovic) #53118
* add Marco Ippolito to TSC (Rafael Gonzaga) #53008
fs:
* mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
* (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
* (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
* (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
* (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
* (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src,permission:
* (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
* (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
* (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
* (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848
PR-URL: #53379
RafaelGSS added a commit that referenced this pull request Jun 10, 2024
@RafaelGSS
2024-06-11, Version 22.3.0 (Current)
2e7a776
Notable changes:
buffer:
* (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
* (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
* (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
* (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
* (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
* add StefanStojanovic to collaborators (StefanStojanovic) #53118
* add Marco Ippolito to TSC (Rafael Gonzaga) #53008
fs:
* mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
* (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
* (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
* (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
* (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
* (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src:
* (SEMVER-MINOR) traverse parent folders while running `--run` (Yagiz Nizipli) #53154
src,permission:
* (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
* (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
* (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
* (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848
PR-URL: #53379
RafaelGSS added a commit that referenced this pull request Jun 11, 2024
@RafaelGSS
2024-06-11, Version 22.3.0 (Current)
d6c77de
Notable changes:
buffer:
* (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
* (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
* (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
* (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
* (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
* add StefanStojanovic to collaborators (StefanStojanovic) #53118
* add Marco Ippolito to TSC (Rafael Gonzaga) #53008
fs:
* mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
* (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
* (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
* (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
* (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
* (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src:
* (SEMVER-MINOR) traverse parent folders while running `--run` (Yagiz Nizipli) #53154
src,permission:
* (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
* (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
* (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
* (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848
PR-URL: #53379
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
@anonrig@soophoo
cli: add NODE_RUN_SCRIPT_NAME env to node --run
8156521
PR-URL: nodejs#53032
Refs: nodejs#52673
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
@RafaelGSS@soophoo
2024-06-11, Version 22.3.0 (Current)
ecded13
Notable changes:
buffer:
* (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) nodejs#53221
cli:
* (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) nodejs#53058
* (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) nodejs#53032
doc:
* (SEMVER-MINOR) add context.assert docs (Colin Ihrig) nodejs#53169
* (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) nodejs#52762
* add StefanStojanovic to collaborators (StefanStojanovic) nodejs#53118
* add Marco Ippolito to TSC (Rafael Gonzaga) nodejs#53008
fs:
* mark recursive cp methods as stable (Théo LUDWIG) nodejs#53127
lib:
* (SEMVER-MINOR) add EventSource Client (Aras Abbasi) nodejs#51575
* (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) nodejs#52370
module:
* (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) nodejs#52213
net:
* (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) nodejs#53136
process:
* (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) nodejs#52762
src:
* (SEMVER-MINOR) traverse parent folders while running `--run` (Yagiz Nizipli) nodejs#53154
src,permission:
* (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) nodejs#53124
test_runner:
* (SEMVER-MINOR) add snapshot testing (Colin Ihrig) nodejs#53169
* (SEMVER-MINOR) add context.fullName (Colin Ihrig) nodejs#53169
* (SEMVER-MINOR) support module mocking (Colin Ihrig) nodejs#52848
PR-URL: nodejs#53379
@aomarksaomarks mentioned this pull request Aug 22, 2024
Merged
aomarks added a commit to google/wireit that referenced this pull request Aug 22, 2024
@aomarks
Support node --run (#1169)
66338d2
Adds support for the new node --run feature which is available starting in Node 22 (basically npm run but faster, see https://nodejs.org/en/blog/announcements/v22-release-announce#running-packagejson-scripts and https://www.yagiz.co/developing-fast-builtin-task-runner/).
Note that node --run is stricter than the other runners when it comes to distinguishing between arguments for the runner vs the script, so an additional -- is needed to set wireit flags and script flags (explained in the README).
Thank you very much to @anonrig for adding the environment variables this required (nodejs/node#53032, nodejs/node#53058) and @justinfagnani for filing the issue (nodejs/node#52673)!
There is a problem with recursive invocations on Windows that I believe is a Node bug but need to double-check, tracking at #1168.
Fixes#1094
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lemirelemirelemire approved these changes
@benjamingrbenjamingrbenjamingr approved these changes
@mcollinamcollinaAwaiting requested review from mcollina
@jasnelljasnellAwaiting requested review from jasnell
@joyeecheungjoyeecheungAwaiting requested review from joyeecheung
@legendecaslegendecasAwaiting requested review from legendecas
@tniessentniessenAwaiting requested review from tniessen

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@anonrig@nodejs-github-bot@targos@tniessen@legendecas@benjamingr@lemire@marco-ippolito