Skip to content

Conversation

@clayallsopp
Copy link
Contributor

I (and others) dislike installing dependencies globally. My workflow with react-native is to add the cli to the package.json so that the versions are locked for each project, etc - general reasons why global dependencies are avoided

The current Xcode build script expects react-native to be on the Xcode shell's $PATH, which can be troublesome (lots of issues related to this, i.e. #3948). This PR adds support for two things:

  1. Automatically detects react-native-cli installed via package.json instead of globally
  2. Allows a REACT_NATIVE_PATH variable that can be set via Xcode if your package exists elsewhere (not sure how helpful this would be for other folks, but hey)

Happy to explore other implementations, but the gist here is that generated react-native projects should be able to work with a project-specific version of react-native-cli

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @frantic, @ide and @lpil 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 18, 2016
@lpil
Copy link
Contributor

lpil commented Jan 18, 2016

I like this change a lot. :)

@brentvatne
Copy link
Collaborator

cc @martinbigio

Copy link
Contributor

Choose a reason for hiding this comment

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

the reliance on cwd here might be fragile. just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The generated Xcode project's build step also relies on cwd, so (I think) it's a consistent assumption:

screen shot 2016-01-19 at 7 53 00 am

Copy link
Contributor

Choose a reason for hiding this comment

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

We rely on cwd inside Xcode because I don't see any other way. In react-native-xcode.sh we can actually resolve the path to local-cli/cli.js relative to packager/react-native-xcode.sh location.

@martinbigio
Copy link
Contributor

@frantic could you take a look?

@frantic frantic self-assigned this Jan 24, 2016
@frantic
Copy link
Contributor

frantic commented Jan 24, 2016

Hey @clayallsopp, thanks for the pull request! You mentioned that you'd be happy to explore other suggestions, and I think I have one for you that would solve a lot of problems with react-native-xcode.sh.

react-native-cli is a very tiny package that does nothing but delegates all commands to locally installed react-native/cli.js, which ends up calling react-native/local-cli/cli.js.

Instead of relying on globally installed react-native command, we can just invoke local-cli/cli.js directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: this is a dummy binary that warns users that they have installed react-native globally (instead of react-native-cli)

@frantic
Copy link
Contributor

frantic commented Jan 24, 2016

Closing in favor of #5518, @clayallsopp thanks again for bringing up this topic!

@frantic frantic closed this Jan 24, 2016
ghost pushed a commit that referenced this pull request Jan 25, 2016
Summary:
Inspired by conversation in #5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes #5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
mkonicek pushed a commit that referenced this pull request Jan 29, 2016
Summary:
Inspired by conversation in #5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes #5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
Inspired by conversation in facebook/react-native#5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes facebook/react-native#5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
Inspired by conversation in facebook/react-native#5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes facebook/react-native#5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
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.

7 participants