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

RFC: Improve flow starting Metro #613

Conversation

szymonrybczak
Copy link
Contributor

This RFC introduces a huge improvement when starting Metro bundler. It relies on the fact that if a port in the bundler is busy, it asks the user if they would like to change it. If the bundler is already running for this project, we should inform the user about that.

View the rendered RFC

Copy link
Collaborator

@huntie huntie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @szymonrybczak, thanks for this proposal!

How could we better handle this on the simulator / emulator side? Right now until we don't type properly port inside DevMenu in simulator / emulator, we won't be able to connect to the bundler.

I think this is the main concern to figure out. We could add some mechanism to automatically change the dev server port for connected devices on start — but I'm not convinced this is what developers would expect. If the goal were to improve the UX of running more than one Metro instance on the same development machine, this is an advanced use case (e.g. to simultaneously develop two apps) — and it's fair to expect developers to set RCT_METRO_PORT explicitly and for the relevant simulator/emulator.

All of we know what pain is to search through the terminal windows, and find the window with the running bundler.

With the motivation centred around this pain point, perhaps we want to narrow this proposal: we could help the developer identify what is running on the current RCT_METRO_PORT and to optionally terminate it.

  • At minimum we can provide a more instructive message to replace Error: listen EADDRINUSE ....
    • This could say something like, "Detected an [existing Metro server/another process] running on port [port]. Please terminate the other instance before starting Metro in this directory, or provide an alternative port number. See [docs link].".
    • Perhaps we could also provide, "Would you like to terminate the other Metro instance (pid: [pid])? [y/N]" — but I doubt this is necessary.

⬆️ Happy to proceed with direct PRs into CLI around improving the error feedback, or please let me know if I missed anything.


## Unresolved questions

- How we should properly handle the case when running app from Xcode / Android Studio? Is it doable to ask via prompt?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, coordinating Metro start from a native IDE is done only with Xcode (AFAIK), and this is not terminated when the iOS process ends.

  • A prompt on start (or better, detection of a modified RCT_METRO_PORT) might be useful but small UX improvement.
  • However, I don't think we want to invest more in automatically starting/stopping Metro when run from an IDE (and should perhaps remove the iOS project integration! cc @cortinico). We intend for (and are continuing to invest in) Metro to be run as a continuous dev server that serves multiple devices without a restart.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, I don't think we want to invest more in automatically starting/stopping Metro when run from an IDE (and should perhaps remove the iOS project integration! cc @cortinico)

Yup we should probably remove them.
IDE integration is delegated to 3rd party plugins. Today is totally doable to just create an IntelliJ plugin that starts/stops Metro or just invoke a command before the "Run" step of the app is triggered.

Ideally we should keep our tools agnostic from the IDE they will end up running into

@szymonrybczak
Copy link
Contributor Author

Hello @huntie, thanks for providing feedback!

I think the best solution for this will be to connect pieces that you proposed with mine. So for me the best flow will look something like this:

  1. Detect whether port is busy
  2. If it's free - launch Metro bundler ✅
  3. If it's busy - ask user whether they want to kill process running on this port or not.
    "Detected an [existing Metro server/another process] running on port [port]. Do you want to terminate this process? [y/N]"
  4. If not - find free port and ask them if they want to start bundler on that free port.
    "Do you want to start Metro server on [port]? [y/N]"

It adds prompts, so starting Metro server in case the port is busy will be easier and faster, even if the user doesn't want to select new port.

@huntie
Copy link
Collaborator

huntie commented Aug 2, 2023

Will be addressed by react-native-community/cli#2021, which adds process detection and better messaging, implemented in CLI. Thanks @szymonrybczak! 🎉

@huntie huntie closed this Aug 2, 2023
@szymonrybczak szymonrybczak deleted the improve-flow-starting-metro branch August 2, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants