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

Add FORCE/SKIP_BUNDLING flags for iOS builds #14731

Closed
wants to merge 3 commits into from

Conversation

cooperka
Copy link
Contributor

@cooperka cooperka commented Jun 26, 2017

Motivation

See discussion below for updated motivation.

Anything running in Debug should use the packager anyway; no need to bundle. This saves a huge amount of time during development when testing things like push notifications that require a real device.

The code being modified was originally moved here in 9ae3714 to make sure bundles are always created in Release, but the change can be applied to real devices, too. Ideally there should be very little difference in how a simulator is treated compared to a physical device.

Test Plan

Run a Debug build in Xcode targeting a physical device before and after this commit.

You can use the FORCE_BUNDLING and SKIP_BUNDLING flags to manually change the default behavior. For example, under Build Phases > Bundle React Native code and images:

export SKIP_BUNDLING=true
../node_modules/react-native/packager/react-native-xcode.sh

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 26, 2017
@javache
Copy link
Member

javache commented Jul 6, 2017

Everyone has different workflows for this. Can you instead make this script look for an environment variable that can be set by your xcodeproj, e.g. SKIP_BUNDLING?

Anything running in Debug should use the packager; no need to bundle. This saves a huge amount of time during development.

The code being modified was originally introduced in facebook@9ae3714 to speed up builds on the simulator, but the change can be applied to real devices, too.
e.g.

```
export SKIP_BUNDLING=true
../node_modules/react-native/packager/react-native-xcode.sh
```
@cooperka
Copy link
Contributor Author

Thanks for the feedback @javache, I've rebased and added the flag you mentioned. I like this even better now.

@javache
Copy link
Member

javache commented Jul 17, 2017

Cool! Let's keep the old behaviour too when you don't specify either of these options.

@cooperka
Copy link
Contributor Author

@javache is there a particular reason you want to keep the old behavior? I'm happy to do so, but to me it seems buggy that it would need to bundle every time just for the device. By default (e.g. after using CRNA) there doesn't seem to be any reason why a physical device would need special treatment.

The original commit that caused the code to be like this doesn't seem to care whether it's a simulator or physical device; what's relevant is Debug mode. Curious to hear your thoughts. Thanks!

@javache
Copy link
Member

javache commented Jul 18, 2017

Because we shouldn't assume that people will see this commit and enable FORCE_BUNDLING. The current version is a good set of defaults: e.g. you want to install a quick debug build on your phone so you can play it when disconnected from your computer, so let's keep them.

@cooperka
Copy link
Contributor Author

@javache updated. That reasoning makes sense.

@cooperka cooperka changed the title Skip creation of offline Debug package even on real devices Add FORCE_BUNDLING and SKIP_BUNDLING flags for iOS builds Jul 18, 2017
@cooperka cooperka changed the title Add FORCE_BUNDLING and SKIP_BUNDLING flags for iOS builds Add FORCE/SKIP_BUNDLING flags for iOS builds Jul 18, 2017
@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 18, 2017
@facebook-github-bot
Copy link
Contributor

@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@emusgrave
Copy link

@cooperka Have you tested the SKIP_BUNDLING flag after a clean of your build folder? I don't think it will work unless you have hard-coded the IP of your development workstation somewhere else.

If running on a physical device, the bundling step is providing an ip.txt file in the bundle which is required in order to figure out what IP address to use to connect to the packager.
If that file isn't found, then it erroneously attempts to connect to localhost, which in this case would be the iOS device.

I think this means that even if you are loading the bundle from the packager, you still need a bundle to be included in your ipa so that it can find the packager's IP address.

I was able to get around this by still running the section of code in react-native-xcode.sh to create the ip.txt even if SKIP_BUNDLING was true. I wanted to run this concept by you before I make a pull request.

@cooperka
Copy link
Contributor Author

cooperka commented Oct 4, 2017

@emusgrave thanks for the detailed context. I didn't dig that deep because it seemed to work for me, but if that file is removed during cleaning, I can see how that would cause problems. Feel free to submit a PR 💯

@antoinerousseau
Copy link
Contributor

why is this not included by default in the HelloWorld template of the .pbxproj file?

if [ "${CONFIGURATION}" == "Debug" ]; then
  export SKIP_BUNDLING=true
 fi

I mean, why would a new project create the bundle when in dev mode?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants