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

Remove wrong-react-native.js and its bin reference from package.json #11160

Closed
wants to merge 1 commit into from

Conversation

amcsi
Copy link

@amcsi amcsi commented Nov 27, 2016

There's an issue similar to gulpjs/gulp-cli#7 with the non-cli's bin takes precedence even though the cli already is installed globally.

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

I have it that node_modules/.bin is looked at for binary files before /path/to/global/npm/bin so that if I'm dealing with a project that needs different versions of otherwise global tools, it could use the project's version.

But if I have react-native-cli installed globally and react-native on the project level, react-native's wrong-react-native script takes precedence and undesirably warns me about a non-existent problem, then not executing the actual cli program.

It is possible to install react-native-cli as well on the project level, however that causes a conflict in binary files, because both provide react-native as a bin tool. I don't know of any reliable way of prioritizing react-native-cli's bin file over react-native in yarn.

If we'd remove this wrong-react-native.js thing, it should solve issues like
#4341
#4515
yuanyan/react-native-web-example#7
https://stackoverflow.com/questions/37949641/react-native-looks-like-you-installed-react-native-globally
https://stackoverflow.com/questions/33908314/packager-wont-start

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @zertosh and @davidaurelio to be potential reviewers.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions.

@amcsi
Copy link
Author

amcsi commented Nov 27, 2016

Okay I have done the CLA registration thing.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 27, 2016
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ide
Copy link
Contributor

ide commented Nov 27, 2016

Can you come up with a solution that doesn't remove this check for the majority of users?

@amcsi
Copy link
Author

amcsi commented Nov 28, 2016

@ide If I couldn't, would this PR be invalid?

This wrong-react-cli.js script itself is nice as it can help in some cases, but is a hack that relies on assumptions about the user's PATH order. Would you have added it to the project if you had known that people have their current node_modules/.bin directory sooner than their global npm bin folder?

In any case I do have one idea, although it's a huge hack over this hack. It would involve calling where react-native or whatever the equivalent is for Unix systems for listing all places an executable was found in each component of PATH, check whether there exists at least 2 entries for it, then return the second entry. It would need to be complicated a bit further for windows, because of both webpack and webpack.com.

And also this still wouldn't solve the issue of react-native-cli being able to be installed along react-native on a project, and would demand that react-native-cli be available globally.

@ide
Copy link
Contributor

ide commented Nov 28, 2016

I'm not sure we want to support the case where react-native-cli is installed inside a project or that react-native is installed globally. Generally the trend seems to be that a CLI wrapper package is installed globally and the full implementation is installed locally (ex: Gulp). If you do want to run a package under node_modules/.bin then you would have to add an npm script for it and run it via npm run thing -- args.

@mkonicek has been thinking about the CLI and react-native init experience more, he should make the call.

@mkonicek
Copy link
Contributor

This file is useful because it tells people they do it wrong if they ran 'npm install -g react-native', instead of 'npm install -g react-native-cli'.

See http://facebook.github.io/react-native/docs/getting-started.html

@mkonicek mkonicek closed this Nov 28, 2016
@amcsi
Copy link
Author

amcsi commented Nov 28, 2016

@mkonicek Yes, but it also makes it impossible for people to run react-native at all in any way for those who have their node_modules/.bin folder higher priority in their PATH than the global npm bin directory. Without resorting to manual work putting a soft link of react-native some place among PATH that has an even higher priority.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants