[Bug]: js_run_devserver: if 'tool' consumes stdin stream, js_run_devserver no longer syncs files on ibazel changes

[gregjacobs]

What happened?

Discovered an interesting one. For js_run_devserver, if the tool/command consumes the stdin stream, then the tool receives the "IBAZEL_BUILD_COMPLETED SUCCESS" messages from ibazel rather than the js_run_devserver.mjs script itself.

This causes js_run_devserver to no longer sync files to its custom sandbox after ibazel has completed. webpack-dev-server seems to be affected btw (including rules_webpack's webpack_devserver() rule), when using the Webpack config watchOptions: { stdin: true }

I believe the issue is because the child process directly inherits the stdin stream. Line 221 of js_run_devserver.mjs:

const proc = child_process.spawn(tool, toolArgs, {
    cwd: cwd,
    stdio: 'inherit',  // <----
    env: env,
})

Version

Development (host) and target OS/architectures: MacOS Monterey

Output of bazel --version: 6.2.0

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file: rules_js@1.32.1

How to reproduce

Just playing with e2e/js_run_devserver/src, and running the :serve_simple script.

If I add the following snippet into simple.js, then js_run_devserver stops syncing when files are changed:

process.stdin.on('data', chunk => {
    console.log(`Child process's chunk: `, chunk.toString());
});

Any other information?

I think the way to fix this may be to duplicate the stdin stream's contents inside js_run_devserver.mjs, and pass that to the child process after reading it for "IBAZEL_BUILD_COMPLETED SUCCESS" messages.

There's probably some good 3rd party libraries that do this in a one-liner, but just creating a new stream.Writable for the child process's stdin and writing to it should do the trick.

[gregjacobs]

Actually, apologies, the fix is even simpler:

const proc = child_process.spawn(tool, toolArgs, {
    cwd: cwd,
    stdio: ['pipe', 'inherit', 'inherit'],  // <-- changed
    env: env,
})
process.stdin.pipe(proc.stdin)

I'll create a PR

[gregjacobs]

Hmm, actually, was just thinking: what if we delayed the write of the "IBAZEL_BUILD_COMPLETED SUCCESS" message to the child process's stdin until after js_run_devserver has sync'd all the files to its sandbox?

I'm thinking that js_run_devserver wants to listen for "IBAZEL_BUILD_COMPLETED SUCCESS" to sync its files, and then maybe a wrapper around Webpack/Vite/esbuild/etc. running in the child process can use the message to trigger a rebuild immediately after (i.e. as soon as js_run_devserver's sync is complete).

Thoughts?

[gregjacobs]

Implemented the above in #1244. Let me know what you guys think

[gregjacobs]

Discovered another reproduction:

Add the following webpack config to e2e/webpack_devserver/webpack.config.js:

watchOptions: {
    stdin: true
}

Run ibazel :dev from the e2e/webpack_devserver folder.

js_run_devserver will no longer sync source files to the sandbox after they're saved ~90% of the time (it will sync once in a while)