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

Dont call npm before node is available #5156

Conversation

hkjorgensen
Copy link
Contributor

This fixes an error introduced in 0.18.0-rc

node and npm isn't available if the developer is using nvm or nodeenv. XCode throws an error because of the call to npm. Simple move the line to after node and npm has been setup.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @lamberta, @frantic and @ide to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 6, 2016
@skevy
Copy link
Contributor

skevy commented Jan 6, 2016

Good catch!

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/932279090153788/int_phab to review.

@ghost ghost closed this in 7ecb693 Jan 6, 2016
csudanthi pushed a commit to healthiqeng/react-native that referenced this pull request Jan 16, 2016
Summary:
This fixes an error introduced in `0.18.0-rc`

`node` and `npm` isn't available if the developer is using `nvm` or `nodeenv`. XCode throws an error because of the call to `npm`. Simple move the line to after `node` and `npm` has been setup.
Closes facebook#5156

Reviewed By: svcscm

Differential Revision: D2807589

Pulled By: androidtrunkagent

fb-gh-sync-id: 30c33145b2cb6f30ff67f6648153d5aa67fb74ed
@mkonicek
Copy link
Contributor

Cherry-picking the commit to 0.18.0-rc and creating a new project breaks the Xcode build for me with:

node_modules/react-native/local-cli/cli.js:73
  const setupEnvScript = /^win/.test(process.platform)
  ^^^^^
SyntaxError: Use of const in strict mode.

Stack trace: http://pastebin.com/DSFirYm1

npm and node version (I am using nvm):

$ npm -v
2.7.1
$ node --version
v4.2.1
$ nvm --version
0.26.1
$ which node
/Users/mkonicek/.nvm/versions/node/v4.2.1/bin/node

Any ideas?

@mkonicek
Copy link
Contributor

cc @davidaurelio, @martinbigio for ideas why this happens.

mkonicek pushed a commit that referenced this pull request Jan 18, 2016
Summary:
This fixes an error introduced in `0.18.0-rc`

`node` and `npm` isn't available if the developer is using `nvm` or `nodeenv`. XCode throws an error because of the call to `npm`. Simple move the line to after `node` and `npm` has been setup.
Closes #5156

Reviewed By: svcscm

Differential Revision: D2807589

Pulled By: androidtrunkagent

fb-gh-sync-id: 30c33145b2cb6f30ff67f6648153d5aa67fb74ed
mkonicek pushed a commit that referenced this pull request Jan 18, 2016
This reverts commit 7ecb693.

This breaks Xcode build, see discussion on #5156
@mkonicek mkonicek mentioned this pull request Jan 18, 2016
1 task
@ide
Copy link
Contributor

ide commented Jan 18, 2016

Is the cherry pick a problem or is master broken as well? If cherry picking is the issue how about we let this go out in a few weeks with 0.19?

@hkjorgensen
Copy link
Contributor Author

@mkonicek I think my fix is unrelated to the error you're getting.
You're node is complaining about the use of const + 'use strict'; in the same file afaik :-)

@martinbigio
Copy link
Contributor

@mkonicek this file doesn't support ES6 because the babekRegisterOnly is run on it (it works on the paths enabled by that call but not on the file which calls it because that one has already been processed). I'm not sure how this was working before, but changing the const for var should fix the problem.

mkonicek pushed a commit that referenced this pull request Jan 29, 2016
Summary:
This fixes an error introduced in `0.18.0-rc`

`node` and `npm` isn't available if the developer is using `nvm` or `nodeenv`. XCode throws an error because of the call to `npm`. Simple move the line to after `node` and `npm` has been setup.
Closes #5156

Reviewed By: svcscm

Differential Revision: D2807589

Pulled By: androidtrunkagent

fb-gh-sync-id: 30c33145b2cb6f30ff67f6648153d5aa67fb74ed
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
This fixes an error introduced in `0.18.0-rc`

`node` and `npm` isn't available if the developer is using `nvm` or `nodeenv`. XCode throws an error because of the call to `npm`. Simple move the line to after `node` and `npm` has been setup.
Closes facebook/react-native#5156

Reviewed By: svcscm

Differential Revision: D2807589

Pulled By: androidtrunkagent

fb-gh-sync-id: 30c33145b2cb6f30ff67f6648153d5aa67fb74ed
This pull request was closed.
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.

6 participants