node
node copied to clipboard
cli: make run and watch modes friends
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
https://github.com/nodejs/node/assets/234659/bd5c4fb0-bbbf-48ab-9e6c-b2679f08b3fa
Review requested:
- [ ] @nodejs/loaders
- [ ] @nodejs/startup
@nodejs/tooling
Why is it wonky?
@mcollina
- 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).
- 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
- 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
CI: https://ci.nodejs.org/job/node-test-pull-request/59951/
CI: https://ci.nodejs.org/job/node-test-pull-request/59954/
@anonrig said via twitter:
I think this is not the correct solution. We should reuse the existing task runner implementation instead of having 2.
I could take a look at exposing a JS binding for the task runner but it would need a variety of changes if it is to handle both scenarios due to IPC and some event management. If possible I think keeping them separated at least until the exact event management etc of --watch is ironed out would be preferable vs a fairly invasive modification.