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

cli: make run and watch modes friends #53457

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bmeck
Copy link
Member

@bmeck bmeck commented Jun 14, 2024

This is without tests until some conversation is had because it is a bit wonky. This makes --watch work with --run so you can do things like node --watch-path src --run start with the following package.json:

{
  "devDependencies": {
    "esbuild": "^0.21.5"
  },
  "scripts": {
    "start": "esbuild --platform=node --bundle src/main.mts --format=cjs --outfile=dist/main.cjs && node dist/main.cjs"
  }
}

Demo

Screen.Recording.2024-06-14.at.2.48.26.PM.mov

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 14, 2024
@GeoffreyBooth
Copy link
Member

@nodejs/tooling

@GeoffreyBooth GeoffreyBooth added the cli Issues and PRs related to the Node.js command line interface. label Jun 14, 2024
@mcollina
Copy link
Member

Why is it wonky?

@bmeck
Copy link
Member Author

bmeck commented Jun 17, 2024

@mcollina

  1. the task runner in C++ doesn't quite match the behavior of the shim I did to make it work with the JS based watch mode (perhaps this is fine since it is just around process management).
  2. the CLI option --run is per_process based but the --watch CLI option is per env. So I had to do a weird pulling out of a env based value in a per_process check https://github.com/nodejs/node/pull/53457/files#diff-6c1386cbc6e4a11941649af31a0257d5c3e40b5f41223cdddf87ab2e984b705aR995
  3. the design of --run (drops positional args) and --watch (uses positional args) have a slight conflict so I banned positional arguments and was seeing some weird results anyway if allowed:
% ../node/node --watch --run a test this a b
Running 'arr=("$@"); echo $# ${#arr} ${#@}'
0 0 0 test this a b

@bmeck bmeck marked this pull request as ready for review June 21, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. cli Issues and PRs related to the Node.js command line interface. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants