Specify correct terminal to start the packager

[lucasbento]

Summary

This PR specifies the correct terminal to start the Metro bundler as stated in react-native-community/cli#303.

Changelog

[iOS] [Changed] - Start the packager on the correct terminal.

Test Plan

To make sure that this works just run it from within Xcode and it should go to your default terminal, you can also try setting the REACT_TERMINAL environment variable to make sure that the packager will open on the desired terminal.

[thymikee]

I actually couldn't get this to work (legacy build, fish shell, latest macOS). Turns out that printenv inside Xcode results in completely different envs and TERM_PROGRAM is not there. Ideas?

[lucasbento]

@thymikee: what happens in the end?

I'm on macOS 10.14, Hyper installed, legacy build it is working here, maybe Hyper did set it up.

I also trying running the app from fishshell.com and it has the TERM_PROGRAM on it, not entirely sure how I can reproduce your issue.

[thymikee]

It doesn't open the terminal at all. I'm really not sure why that happens. I use iTerm. The TERM_PROGRAM is there on fish, zsh, bash and sh, just Xcode doesn't see it on my machine. If it works for you, I think it would be fine to default the $terminal to Terminal.app so it doesn't regress.

[lucasbento]

Strange. That's a good idea though, I'll do it.

[noahtallen]

Nifty! I'm curious - I just opened this issue on the CLI repo. Essentially, I have react imported through Pods, and not at all through the Xcode project. Does this mean that this shellScript would not get run? Because it currently does not launch the packager for me on react-native run-ios for a simulator (though it appears that normal bundling during a release build works fine).

[lucasbento]

@noahtallen: let's continue the conversation on react-native-community/cli#317, I would like to fix that problem.

[lucasbento]

@thymikee: I just added a check to verify if it found the default terminal or not, if not just open the packager without specifying it.

[thymikee]

After fixing syntax errors (that I see now are fixed :D), when building from command line (run-ios) my default terminal launches, but if I build directly from Xcode, the system Terminal.app is opened.

I googled a bit and I found this:
https://forums.developer.apple.com/thread/8451

Looks like the default for UseSanitizedBuildSystemEnvironment arg for Xcode IDE is YES, and it's the opposite when running xcodebuild and hence the difference. Not sure if there's anything we can do about that other than document it?

[lucasbento]

@thymikee: sorry for the syntax error, thought I was testing this but in the end was testing another app.

I tried to check if I have UseSanitizedBuildSystemEnvironment set to NO but apparently I don't (I also don't recall setting it) and the Xcode still inherits the env variables.

If you define it as NO, does it work?

I'll try to check with a co-worker to see if I can reproduce that on their computer as well.

[lucasbento]

Btw I checked with defaults read-type com.apple.dt.Xcode UseSanitizedBuildSystemEnvironment not sure if this is the correct syntax.

[thymikee]

Just tried and still nope (inb4 I restarted Xcode)

[lucasbento]

I'll check it here with some of my co-workers to see if I can reproduce that problem.

[thymikee]

I've additionally tried this on a fresh project without pods, with UseSanitizedBuildSystemEnvironment disabled (and enabled as well). Still same results. Wonder if I set something weird years ago and now don't remember :D

[michalchudziak]

❤️

[cpojer]

Nice!

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[lucasbento]

@thymikee: I tested on two other MacBooks and it's the same behaviour as yours, TERM_PROGRAM is not defined for some reason, so I guess I'm the one that may have done something weird in my machine to have it.

I'm wondering now if there's any way that we can pass the TERM_PROGRAM to the build phase script otherwise this will work only for a few people (I assume) and for the ones that use the CLI.

[react-native-bot]

This pull request was successfully merged by @lucasbento in 3273d23.

^{When will my fix make it into a release? | Upcoming Releases}

[lucasbento]

@thymikee: the only way I see atm to improve this part is to have a way to customize those env variables through a file so that the script can read from it instead of trying to get the global ones, unfortunately this PR will not change the behaviour for people that use Xcode.