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

Pass port when running on device (fix #17973) #17983

Closed
wants to merge 1 commit into from

Conversation

jozan
Copy link
Contributor

@jozan jozan commented Feb 14, 2018

Motivation

Building for iOS device react-native run-ios --device [id] fails since port is not passed. This is a blocker for us and prevents us from updating to latest React Native stable.

Test Plan

I've tested this in our app and also in fresh app using RN master and verified that it works.

Related PRs

None.

Related Issues

#17973: The --device option fails on command line in v53.0

Release Notes

[CLI] [BUGFIX] [local-cli/runIOS/runIOS.js] - Pass metro port when running on device

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. cla signed labels Feb 14, 2018
@chirag04
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Something went wrong executing that command, @hramos could you take a look?

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 14, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@jozan
Copy link
Contributor Author

jozan commented Feb 14, 2018 via email

@MrLoh
Copy link

MrLoh commented Feb 15, 2018

Any way to use react native 53 on device? Is this already available as some beta that can be installed?

@jozan
Copy link
Contributor Author

jozan commented Feb 15, 2018

@MrLoh This is merged to master for now. It might be considered worth a hassle to release patch. I have no idea how all this works so I suggest you to follow this thread and watch for new releases: #17520

@MrLoh
Copy link

MrLoh commented Feb 16, 2018

Well for now I fixed it by adding the args.port manually in my node_modules/react-native/local-cli/runIOS/runIOS.js it's on line 61 in the 0.53 release (there were other changes that added ten lines before your release). It's a bit scary that stuff like that doesn't get checked before a release.

I'm very glad about the --port option though.

@hramos
Copy link
Contributor

hramos commented Feb 16, 2018

You can build from source if you want to take advantage of the fix before it makes it into a release.

Plo4ox pushed a commit to Plo4ox/react-native that referenced this pull request Feb 17, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Building for iOS device `react-native run-ios --device [id]` fails since port is not passed. This is a blocker for us and prevents us from updating to latest React Native stable.

I've tested this in our app and also in fresh app using RN master and verified that it works.

None.

[CLI] [BUGFIX] [local-cli/runIOS/runIOS.js] - Pass metro port when running on device
<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#17983

Differential Revision: D6988299

Pulled By: hramos

fbshipit-source-id: 5169706600f87f13b9c9c105eb7d6db7a40194f1
@agates4
Copy link

agates4 commented Feb 17, 2018

Is there an ETA for release?

@MrLoh
Copy link

MrLoh commented Feb 18, 2018

Probably the next release beginning of March. Just fix it locally until then

Sent with GitHawk

@agates4
Copy link

agates4 commented Feb 18, 2018

For anyone else looking for a quick fix -

in node_modules/react-native/local-cli/runIOS/runIOS.js

change runOnDevice(selectedDevice, scheme, xcodeProject, args.configuration, args.packager, args.verbose);

to runOnDevice(selectedDevice, scheme, xcodeProject, args.configuration, args.packager, args.verbose, args.port);

grabbou pushed a commit that referenced this pull request Feb 20, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Building for iOS device `react-native run-ios --device [id]` fails since port is not passed. This is a blocker for us and prevents us from updating to latest React Native stable.

I've tested this in our app and also in fresh app using RN master and verified that it works.

None.

[CLI] [BUGFIX] [local-cli/runIOS/runIOS.js] - Pass metro port when running on device
<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes #17983

Differential Revision: D6988299

Pulled By: hramos

fbshipit-source-id: 5169706600f87f13b9c9c105eb7d6db7a40194f1
@mlazari
Copy link
Contributor

mlazari commented Mar 3, 2018

Why is this fix included in v0.53.3, but not in v0.54.0?

@hramos
Copy link
Contributor

hramos commented Mar 5, 2018

The fix, f8fee0a, is only on master. You should expect this fix to be part of the 0.55 release, known as March 2018. It should enter RC status this week, and be available as a stable release next month.

@jozan
Copy link
Contributor Author

jozan commented Mar 5, 2018 via email

@hramos
Copy link
Contributor

hramos commented Mar 5, 2018

Yup, this is all documented in the versions page: http://facebook.github.io/react-native/versions.html

We follow a monthly release train. Once a commit lands on master, it will make it into the next release candidate. As this commit landed after 0.54-RC was created, it has to wait for the 0.55-RC. We follow this process to ensure all changes soak for a month in order to surface any urgent issues. On rare occasion, we might cherry pick a fix into a particular RC, but the bar for doing so is quite high (i.e. it is a regression or it breaks all apps). Generally, feature fixes like this one are not good candidates for cherry-picking. Thanks for your patience.

@jozan
Copy link
Contributor Author

jozan commented Mar 6, 2018 via email

xiamx pushed a commit to xiamx/react-native that referenced this pull request Mar 13, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Building for iOS device `react-native run-ios --device [id]` fails since port is not passed. This is a blocker for us and prevents us from updating to latest React Native stable.

I've tested this in our app and also in fresh app using RN master and verified that it works.

None.

[CLI] [BUGFIX] [local-cli/runIOS/runIOS.js] - Pass metro port when running on device
<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#17983

Differential Revision: D6988299

Pulled By: hramos

fbshipit-source-id: 5169706600f87f13b9c9c105eb7d6db7a40194f1
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Building for iOS device `react-native run-ios --device [id]` fails since port is not passed. This is a blocker for us and prevents us from updating to latest React Native stable.

I've tested this in our app and also in fresh app using RN master and verified that it works.

None.

[CLI] [BUGFIX] [local-cli/runIOS/runIOS.js] - Pass metro port when running on device
<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook/react-native#17983

Differential Revision: D6988299

Pulled By: hramos

fbshipit-source-id: 5169706600f87f13b9c9c105eb7d6db7a40194f1
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.

7 participants