Fix for 17497 #17498
Fix for 17497 #17498
Conversation
…t to call it to setup the environment variable
facebook-github-bot
added
CLA Signed
local-cli/runAndroid/runAndroid.js
Outdated
| @@ -263,17 +263,18 @@ function runOnAllDevices(args, cmd, packageNameWithSuffix, packageName, adbPath) | |||
| } | |||
|
|
|||
| function startServerInNewWindow(port) { | |||
| const scriptFile = /^win/.test(process.platform) ? | |||
| const isWin = /^win/.test(process.platform); | |||
There was a problem hiding this comment.
Please, don't use abbreviations.
There was a problem hiding this comment.
Renamed isWin to isWindows everywhere where it's necessary.
local-cli/runAndroid/runAndroid.js
Outdated
| // setup the .packager.env file to ensure the packager starts on the right port | ||
| const packagerEnvFile = path.join(__dirname, '..', '..', 'scripts', '.packager.env'); | ||
| const content = `export RCT_METRO_PORT=${port}`; | ||
| // setup the .packager.(env|bat) file to ensure the packager starts on the right port |
There was a problem hiding this comment.
Can we decouple file names and command names into separate variables/constants?
There was a problem hiding this comment.
I'm not sure what you want here? are you looking for new const variables, one for .packager.bat and one for .packager.env?
I'd have preferred to use just a single filename, unfortunately, Windows doesn't have a source .env equivalent, you can get the same result by using a batch file, but the batch files must have a .bat extension to be invoked with call from the parent script...
|
@ihenshaw I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project. |
react-native-bot
added
Bug Fix 🐛
Ran Commands
facebook-github-bot
added
the
Import Started
There was a problem hiding this comment.
@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Changed runAndroid.js to generate .packager.bat and launchPackager.ba…t to call it to setup the environment variable <!-- 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! --> Trying to use react-native on a Windows box with a virus killer that runs on port 8081... (Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!) 1) Start an android emulator 2) on a react-native project (with the changes), run `react-native run-android --port 9988`. 3) When the packager starts, verify that it states the correct port in the terminal window. 4) verify that the application correctly starts in the Emulator. (If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.) <!-- 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 --> [INTERNAL] [BUGFIX] [./scripts] - Fixed runAndroid to enable the use of a package on port <> 8081 for Windows. Closes facebook/react-native#17498 Differential Revision: D8682067 Pulled By: hramos fbshipit-source-id: 6604b827077b3a6a2da9914c1fd36dad6ef30e43
Changed runAndroid.js to generate .packager.bat and launchPackager.ba…t to call it to setup the environment variable
Motivation
Trying to use react-native on a Windows box with a virus killer that runs on port 8081...
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
react-native run-android --port 9988.Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)
Release Notes
[INTERNAL] [BUGFIX] [./scripts] - Fixed runAndroid to enable the use of a package on port <> 8081 for Windows.