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

Adds sanity check for presence of react-native conflict #934

Merged
merged 3 commits into from
Mar 23, 2017

Conversation

jamonholmgren
Copy link
Member

screen shot 2017-03-22 at 11 36 35 pm

Ref #874.

@@ -30,12 +30,27 @@ var rnCli = enforceGlobalDependency({
optional: false,
range: '>=2.0.0',
which: 'react-native',
package: 'react-native-cli',
packageName: 'react-native-cli',
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed this was missed from my previous #929 PR.

versionCommand: 'react-native --version',
installMessage: 'To install: npm i -g react-native-cli'
})

if (!rnCli) {
// If `react-native` is installed, it can cause problems with `react-native-cli`
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that I only run this if the react-native-cli check fails. That's because this check is much slower and if we ran it every time ignite ran, we'd add several seconds to the bootup.

@skellock
Copy link
Contributor

Since we are targetting es3, @jamonholmgren we will have to remove the multilines in that console.log. Unless you can test that in node 4. If its good, we can add an linter ignore directive.

Copy link
Contributor

@skellock skellock left a comment

Choose a reason for hiding this comment

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

I'm cool with this as is. A future improvement would be nice to find a different way to detect that global react-native package that isnt slow. Just not sure how.

Some folks can have it installed AND a valid react-native-cli.

@jamonholmgren jamonholmgren merged commit 47f2d73 into master Mar 23, 2017
@jamonholmgren jamonholmgren deleted the sanity-check-react-native-npm branch March 23, 2017 19:01
@skellock skellock added this to the 2.0 Beta 8 milestone Mar 24, 2017
@jamonholmgren
Copy link
Member Author

Fixes #940 too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants