facebook / metro

🚇 The JavaScript bundler for React Native
https://metrobundler.dev
MIT License
5.24k stars 626 forks source link

Ctrl+Z doesn't issue SIGTSTP #1364

Open samholmes opened 1 month ago

samholmes commented 1 month ago

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

  1. Run metro bundler yarn start --resetCache
  2. Send a suspend the program with a SIGTSTP signal using ctrl+z (common key binding in most terminals)

Result: nothing happens.

What is the expected behavior?

The program is suspended as if kill -SIGSTSTP <pid> was run:

zsh: suspended  yarn start --resetCache

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

yarn --version
1.22.19

Metro v0.73.10

react-native@0.71.15
huntie commented 1 month ago

Interesting. Besides the conventional behaviour, do you have a use case for this?

I think we're unlikely to prioritise this soon.

samholmes commented 1 month ago

@huntie A typical workflow for me is to have a shell session open for certain directories managed by a multiplexer or built-in tab management in the respective terminal emulator. By being able suspend long-running programs such as metro, I'm able to run more shell commands in the working directory of the project without exiting the metro program entirely. This is what I typically do with Webpack.

huntie commented 1 month ago

@samholmes Understood! (TIL 😄)

We should support this, but after playing around quickly, it's not a straightforward as we'd hope. Key handling is implemented by @react-native/community-cli-plugin, which calls process.stdin.setRawMode() — which disables all default process handling by Node 😐.

➡️ For now, this task will go into our backlog. Or alternatively, please feel free to take a stab at the issue, with the above constraints (might be fiddly) and open a PR.

In an older version of attachKeyHandlers.js, there used to be explicit CTRL_Z handling, but this didn't work:

https://github.com/facebook/react-native/blob/b955d8d0a70193e53b8bee274320c57a96485277/packages/community-cli-plugin/src/commands/start/attachKeyHandlers.js#L89-L90

samholmes commented 1 month ago

I'm aware that this will send the right signal to the process:

process.kill(process.pid, "SIGTSTP")

Does https://www.npmjs.com/package/@react-native/community-cli-plugin handle the signaling?

EDIT: I think I found the relevant file: https://github.com/facebook/react-native/blob/7b877a4bed37041a06c523eb5fe13a26dd6690df/packages/community-cli-plugin/src/commands/start/attachKeyHandlers.js#L103

Unfortunately I'm not familiar enough with the architecture of metro/react-native enough to know how to test/develop this. I'll have to take more time to setup my dev env with react-native/metro to make the contribution.

huntie commented 1 month ago

@samholmes Yes, that's the same file I linked above :)

We have a contributing guide here: https://reactnative.dev/contributing/overview

But also, here's a quickstart sufficient for this issue:

# 1/ Clone the repo
gh repo clone <your-account-with-fork>/react-native
code react-native/react-native.code-workspace

# 2/ (Edit files in community-cli-plugin - they will run from source)

# 3/ Run rn-tester
cd packages/rn-tester/
yarn start
# (in a second terminal window)
bundle install
bundle exec pod install
xed