Fix local-cli's installedGlobally check to work on Windows platforms #17036
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@facebook-github-bot label Android Generated by 🚫 dangerJS |
facebook-github-bot
added
the
CLA Signed
GantMan
reviewed
Dec 1, 2017
local-cli/wrong-react-native.js
Outdated
| var script = process.argv[1]; | ||
| var installedGlobally = script.indexOf('node_modules/.bin/react-native') === -1; | ||
| const fs = require('fs'); | ||
| const path = require('path'); |
There was a problem hiding this comment.
you can move these requires inside of the windows code, so they won't be required on non-win32.
Not a huge refactor, but I'd do it.
facebook-github-bot
added
the
Import Started
facebook-github-bot
approved these changes
Dec 4, 2017
There was a problem hiding this comment.
@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification. |
facebook-github-bot
added
Import Failed
Import Started
facebook-github-bot
approved these changes
Dec 4, 2017
There was a problem hiding this comment.
@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Thanks for contributing! |
bowerman0
pushed a commit
to bowerman0/react-native
that referenced
this pull request
Dec 5, 2017
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! --> The react-native local-cli does a check to see if it is being run from a global install or not. If running from a global install, an error is printed and the CLI exits. This check for a global install does not work on Windows. The check of `process.argv` does not contain the expected `node_modules/.bin/react-native`. It instead contains a direct path to the `wrong-react-native.js` file, as determined by the `node_modules/.bin/react-native.cmd` entry point. This update will, on Windows platforms, do a global check by instead looking for the existence of a package.json above the node_modules. If not found, we assume a global install and print the error. In a react-native project, I originally tried running the local react-native cli: ``` > yarn react-native --version yarn run v1.3.2 $ E:\myproject\node_modules\.bin\react-native --version Looks like you installed react-native globally, maybe you meant react-native-cli? To fix the issue, run: npm uninstall -g react-native npm install -g react-native-cli error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` I replaced the `wrong-react-native.js` with the modified version and reran the command: ``` > yarn react-native --version yarn run v1.3.2 $ E:\myproject\node_modules\.bin\react-native --version Scanning folders for symlinks in E:\myproject\node_modules (93ms) 0.50.3 Done in 1.86s. ``` <!-- 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 --> [CLI] [BUGFIX] [local-cli/wrong-react-native.js] - Updated local-cli on Windows to check for the absence of a package.json file to determine if being run from a global installation or not. Closes facebook#17036 Differential Revision: D6471925 Pulled By: TheSavior fbshipit-source-id: cc5560d1c102d05f378e5ae537f13d31b5343045
bowerman0
pushed a commit
to bowerman0/react-native
that referenced
this pull request
Dec 5, 2017
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! --> The react-native local-cli does a check to see if it is being run from a global install or not. If running from a global install, an error is printed and the CLI exits. This check for a global install does not work on Windows. The check of `process.argv` does not contain the expected `node_modules/.bin/react-native`. It instead contains a direct path to the `wrong-react-native.js` file, as determined by the `node_modules/.bin/react-native.cmd` entry point. This update will, on Windows platforms, do a global check by instead looking for the existence of a package.json above the node_modules. If not found, we assume a global install and print the error. In a react-native project, I originally tried running the local react-native cli: ``` > yarn react-native --version yarn run v1.3.2 $ E:\myproject\node_modules\.bin\react-native --version Looks like you installed react-native globally, maybe you meant react-native-cli? To fix the issue, run: npm uninstall -g react-native npm install -g react-native-cli error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` I replaced the `wrong-react-native.js` with the modified version and reran the command: ``` > yarn react-native --version yarn run v1.3.2 $ E:\myproject\node_modules\.bin\react-native --version Scanning folders for symlinks in E:\myproject\node_modules (93ms) 0.50.3 Done in 1.86s. ``` <!-- 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 --> [CLI] [BUGFIX] [local-cli/wrong-react-native.js] - Updated local-cli on Windows to check for the absence of a package.json file to determine if being run from a global installation or not. Closes facebook#17036 Differential Revision: D6471925 Pulled By: TheSavior fbshipit-source-id: cc5560d1c102d05f378e5ae537f13d31b5343045
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! --> The react-native local-cli does a check to see if it is being run from a global install or not. If running from a global install, an error is printed and the CLI exits. This check for a global install does not work on Windows. The check of `process.argv` does not contain the expected `node_modules/.bin/react-native`. It instead contains a direct path to the `wrong-react-native.js` file, as determined by the `node_modules/.bin/react-native.cmd` entry point. This update will, on Windows platforms, do a global check by instead looking for the existence of a package.json above the node_modules. If not found, we assume a global install and print the error. In a react-native project, I originally tried running the local react-native cli: ``` > yarn react-native --version yarn run v1.3.2 $ E:\myproject\node_modules\.bin\react-native --version Looks like you installed react-native globally, maybe you meant react-native-cli? To fix the issue, run: npm uninstall -g react-native npm install -g react-native-cli error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. ``` I replaced the `wrong-react-native.js` with the modified version and reran the command: ``` > yarn react-native --version yarn run v1.3.2 $ E:\myproject\node_modules\.bin\react-native --version Scanning folders for symlinks in E:\myproject\node_modules (93ms) 0.50.3 Done in 1.86s. ``` <!-- 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 --> [CLI] [BUGFIX] [local-cli/wrong-react-native.js] - Updated local-cli on Windows to check for the absence of a package.json file to determine if being run from a global installation or not. Closes facebook/react-native#17036 Differential Revision: D6471925 Pulled By: TheSavior fbshipit-source-id: cc5560d1c102d05f378e5ae537f13d31b5343045
react-native-bot
removed
the
Import Started
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The react-native local-cli does a check to see if it is being run from a global install or not. If running from a global install, an error is printed and the CLI exits.
This check for a global install does not work on Windows. The check of
process.argvdoes not contain the expectednode_modules/.bin/react-native. It instead contains a direct path to thewrong-react-native.jsfile, as determined by thenode_modules/.bin/react-native.cmdentry point.This update will, on Windows platforms, do a global check by instead looking for the existence of a package.json above the node_modules. If not found, we assume a global install and print the error.
Test Plan
In a react-native project, I originally tried running the local react-native cli:
I replaced the
wrong-react-native.jswith the modified version and reran the command:Release Notes
[CLI] [BUGFIX] [local-cli/wrong-react-native.js] - Updated local-cli on Windows to check for the absence of a package.json file to determine if being run from a global installation or not.