microsoft/rushstack

[rush] watch mode fails as "process.stdin.setRawMode is not a function" since 5.107.0

antoine-coulon opened this issue · 2 comments

Summary

Hello folks, Rush 5.107.0 release and specifically this commit introduced a great feature, which is enabling pausing/resuming the watch mode. However, when running Rush commands on watch mode via child_processes, it produces the following error process.stdin.setRawMode is not a function as this requires the Node process to be initialized with process.stdin as an instance of tty.ReadStream, otherwise these methods are not available. This currently broke the custom toolchain we wrote on top of Rush.js, starting from 5.107.0.

This can be fixed by providing stdio: "inherit" to the child_process so that the child process shares the TTY instance however I feel like we should be able to run watched commands in the background whether it's simply by piping (stdio: "pipe") or ignoring the stdio (stdio: "ignore"). In other words, we should be able to benefit from interactivity is the process is spawned in a way that is meant to be interactive, but it should not crash the process otherwise.

Repro steps

Just run a Rush command under watch mode from a child_process with no TTY enabled:

import { spawn } from "node:child_process";

spawn("<whatever_rush_command_under_watch_mode>", { stdio: "pipe" });

Will provoke the following: process.stdin.setRawMode is not a function since 5.107.0

Expected result: Run the watch mode, with no interactivity whatsoever

Actual result: process.stdin.setRawMode is not a function

Details

I'll land a PR that first checks if the process.stdin is an instance of TTY before registering keyboard listeners (coming from _registerWatchModeInterface)

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version? 5.107.0
rushVersion from rush.json? 5.107.0
useWorkspaces from rush.json? yes
Operating system? Mac
Would you consider contributing a PR? Yes
Node.js version (node -v)? 18.18.0

This was merged in #4365 by @dmichon-msft, should I close it or wait for the release?

We usually close out issues once they are merged. The release notes will contain what was fixed in each version, so this should be surfaced through that.

Thanks for the fix!