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

Ctrl+Z doesn't issue SIGTSTP #1364

Open
samholmes opened this issue Oct 3, 2024 · 5 comments
Open

Ctrl+Z doesn't issue SIGTSTP #1364

samholmes opened this issue Oct 3, 2024 · 5 comments

Comments

@samholmes
Copy link

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

[email protected]
@huntie
Copy link
Member

huntie commented Oct 4, 2024

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

  • I can imagine a scenario where you'd want to suspend Metro in order to pause Fast Refresh updates to connected app(s), with "Press return to resume". However, we offer this capability in the in-app Dev Menu.

I think we're unlikely to prioritise this soon.

@samholmes
Copy link
Author

@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
Copy link
Member

huntie commented Oct 7, 2024

@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 😐.

  • We need raw mode because we want to trigger a graceful shutdown of Metro's server by handling CTRL_C manually.
  • This means we'd also need to handle CTRL_Z explicitly.

➡️ 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
Copy link
Author

samholmes commented Oct 8, 2024

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
Copy link
Member

huntie commented Oct 8, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants