Added option to disable xip

[jkimbo]

Motivation (required)

When running an app on a real iPhone in debug mode, I occasionally get issues when trying to load the js bundle and I suspect the issue is with xip.io (and it doesn't look like I'm the only one: #12786 #9688 (comment)). So I've added the ability to optionally disable the use of xip.io if an env variable DISABLE_XIP is set.

Test Plan (required)

Add export DISABLE_XIP=true to the Bundle React Native code and images build phase. Run the app on a real iPhone and ensure that it can load the JS bundle from the host computers IP.

[ericvicenti]

LGTM!

[facebook-github-bot]

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

[mo22]

Is there any reason or benefit to use xip.io? I had a lot of trouble because DNS responses with private ip ranges are blocked.

[ghost]

Possible to drop that xip.io? If I change

node_modules/react-native/scripts/react-native-xcode.sh

and remove the .xip.io extension, everything is fine with the remote packager.

[rimzici]

Facing similar issue. Will this PR, solve the issue in Android too?

The connection with the development server lost frequently.
Happens even when debug is off, and it comes and goes randomly.

RN v0.42.3
Android
Ubuntu 16.04

[AlicanC]

Hey, I'm working on this file and don't know what "+x" in ${DISABLE_XIP+x} does. (AFAICS, it is also not mentioned in the Bash manual.) Could you please explain?

[jkimbo]

It's parameter expansion:

${parameter:+word}
If parameter is null or unset, nothing is substituted, otherwise the expansion of word is substituted.

So if DISABLE_XIP is null or unset then the check will fail otherwise it will pass. x is used here just as a placeholder.

[AlicanC]

Turns out I've missed the part where it explains that the colon can be omitted:

Omitting the colon results in a test only for a parameter that is unset. Put another way, if the colon is included, the operator tests for both parameter’s existence and that its value is not null; if the colon is omitted, the operator tests only for existence.

Thanks!

I think I will change this to [[ $DISABLE_XIP != true ]] like I did with all the other boolean-y variables.

To be honest, ${DISABLE_XIP+x} looks ugly. I have completely rewritten this script and this is the last part of the whole thing that I could not understand. I don't like saying "If that was hard for me, then it will be for other people." but I guess this one deserves an exception.

Also, correct me if I'm wrong, but if a variable is set and I want to run a single command with that variable being unset, I have to use unset DISABLE_XIP; command which unsets the variable for the whole env and not just the command. A subshell can be used to avoid that but it just gets uglier. With a != true check, you can just do DISABLE_XIP= command; to turn it off and DISABLE_XIP=true command; to turn it on. This doesn't really matter in this use case, but I guess I will adopt this as one of my "Bash Best Practices".

What do you think?

[jkimbo]

Yep that sounds completely reasonable and a nicer solution all round. Thanks!

[mo22]

Hi,
a majority of home routers blocks dns responses to the local ip range, making xip unusable and causing errors that are quite difficult to debug.
I would remove xip as a default and only make it optional for those people who need it.

[mo22]

And - I know this thread is not specificly about this - I'd like to modify the script to support using SKIP_BUNDLING also for device builds, while still updating the IP address, which would require creating the ip.txt also if SKIP_BUNDLING is true:

if [[ "$SKIP_BUNDLING" ]]; then
  IP=$(ipconfig getifaddr en0)
  if [ -z "$IP" ]; then
    IP=$(ifconfig | grep 'inet ' | grep -v ' 127.' | cut -d\   -f2  | awk 'NR==1{print $1}')
  fi
  DEST=$CONFIGURATION_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH
  echo "$IP" > "$DEST/ip.txt"
  echo "SKIP_BUNDLING enabled; skipping."
  exit 0;
fi