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

Running an app in debug should not fail due to other apps running packager #1209

Closed
plrdev opened this issue Jun 29, 2020 · 11 comments
Closed
Assignees
Labels
feature request no-stale-bot This issue cannot be marked as stale by stale bot

Comments

@plrdev
Copy link

plrdev commented Jun 29, 2020

Describe the Feature

Running new app should work no matter if there has been other apps running (with packager) - ideally they would not collide.

Currently if you have an app already running in debug, it has it's packager running (by default in port 8081). When you try to run a second app, it will not spawn a launcher due to finding 8081 part having a packager already running. This will try to run the second app in the first apps packager, which will fail with an error:

rn_metro

This is unnecessary friction and requires manually killing the terminal each time you are switching to dev another application (or define the port yourself).

Possible Implementations

I did a twitter poll to see the expectations, here are the results

image

This confirmed my own feeling: Best option would be not to mess with the already running app. Instead, we would find a new port to run it.

I did some experimentations and based on those here is my proposed solution:

  1. If --port is used and is not the default (8081) try to use the specified port. If it is in use, we don't launch packager - you are manually requesting the port and we don't want to change it
  2. If default port, we check for running processes listening in TCP range 8081-8089 (for example)
  3. We check if any of those is packager instance for the app we want to run (for example, slight modification for /status that stores also the project root). If yes, we don't launch a new packager, but use the existing one
  4. Find first free port in the range (again, 8081-8089 for example) and launch packager there

This would replace the current logic:

  1. If defined port (default 8081) is in use and it's status is running, don't do anything
  2. Otherwise try launching a new packager in the port - fail if not able to

POC

I have a POC for iOS locally very close to working, but I wanted to make sure this would be something that is wanted before going forward and finishing it. Android side of implementation is pending still as well.

Changes required so far:

  • Slight modification for cli-server-api statusMiddleWare (/status), so it returns JSON with running status and projectRoot (so we can later check if the running packager is for the current project or not)
  • Start packager bin/sh script changes to do steps 1-4 listed in the proposed solution

Any comments on the proposed solution? Some reasons why it wouldn't work or not wanted to have in the first place? Any changes requested / required? Good to go? If all is good, I'll keep working on this and make a PR out of it.

@thymikee
Copy link
Member

I'd be super supportive to merge a PR adding that 👍

@thymikee
Copy link
Member

cc @fson to consult changes to cli-server-api

@thymikee
Copy link
Member

One thing that needs to be taken into consideration is that when building e.g. android, the port is defaulted to 8081 as well and you're only able to connect with Metro on that very port. So when you start Metro separately, you'd need to adjust build command to have the correct --port passed. Ideas how to resolve this use case? It's possible to offset this when Metro is spawned from a build command, but that's not the only use case (and not mine for example, I tend to spawn Metro separately).

@plrdev
Copy link
Author

plrdev commented Jun 29, 2020

Thanks for the comments @thymikee . About Android, I need to familiarize myself more on that flow, but iOS is using this .packager.env file within react-native to save the used port. Maybe same approach could work on Android as well? So when running packager, save the port in the file and when building without setting up --port, check the file and use that port instead of default, if found. But like I said, need to check the whole Android side still.

@plrdev
Copy link
Author

plrdev commented Jul 1, 2020

Changes to statusPageMiddleware stops packager bundling for debug builds. Packager status is checked within react native as well: https://github.com/facebook/react-native/blob/382f0898cf4112e64150c3b771e85df53ae86575/React/Base/RCTBundleURLProvider.mm#L98

After changing it there, it works. It could be like this to avoid making it a breaking change:

-- return [status isEqualToString:@"packager-status:running"];
++ return [status containsString:@"packager-status"];

Optionally, if there is a better way/place to get the project root for running packager instance, open to other ideas..

@github-actions
Copy link

There hasn't been any activity on this issue in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

@thymikee
Copy link
Member

thymikee commented Dec 5, 2022

@plrdev are you still interested in this feature? I'm gonna reopen, because it's clearly beneficial to have this

@thymikee thymikee reopened this Dec 5, 2022
@github-actions github-actions bot removed the stale label Dec 6, 2022
@TMisiukiewicz TMisiukiewicz added the no-stale-bot This issue cannot be marked as stale by stale bot label Dec 6, 2022
@plrdev
Copy link
Author

plrdev commented Dec 6, 2022

@plrdev are you still interested in this feature? I'm gonna reopen, because it's clearly beneficial to have this

Hey thanks for the reminder on this. I'd be willing to work on this but I think I'd like to check this together with someone with the experience on the repo/codebase. Anyone willing to give some guidance and discuss this further?

@thymikee
Copy link
Member

thymikee commented Dec 7, 2022

Asked internally if someone finds some time to help on this. Let's see how it goes :)

@szymonrybczak szymonrybczak self-assigned this Dec 20, 2022
@omgoshjosh
Copy link

omgoshjosh commented Jan 6, 2023

this sounds nice. sorry if this is the wrong place to ask but is there a way to plug the port into the xcode build process in version 0.66?

right now i have a shell script running in an xcode build phase called "Start Packager" that runs the launchPackager.command file but the node environment process.env.RCT_METRO_PORT can't be found so i'm stuck with port 8081.

i believe this is because my script uses open yada to open a new terminal window. looking at later versions i found that .packager.env is sourced in the launchPackager.command file which would work with another part of my script (echo "export RCT_METRO_PORT=${RCT_METRO_PORT}" > "${SRCROOT}/../node_modules/react-native/scripts/.packager.env"), but i am left wondering how it was done for version 0.66 since that line didn't exist at that time.

@szymonrybczak
Copy link
Collaborator

Finally I think we can close this issue as I implemented this as part of #2021. Shipping with React Native 0.73 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request no-stale-bot This issue cannot be marked as stale by stale bot
Projects
None yet
Development

No branches or pull requests

5 participants