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

Fix symlink resolving for examples in the repo #12435

Closed
wants to merge 1 commit into from

Conversation

Kureev
Copy link
Contributor

@Kureev Kureev commented Feb 17, 2017

Motivation:
Few days ago @gaearon filed an issue that examples in the react-native repo doesn't work after the recent changes in local-cli. This PR fixes reported bug.

Test plan (required)

  • No UI changes
  • Run UIExplorer from XCode (no package.json in the folder) and check if packager/application runs correctly

cc @davidaurelio @satya164 @grabbou

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Feb 17, 2017
@Swaagie
Copy link

Swaagie commented Feb 17, 2017

A similar fix, among others, were posted 2 days ago, #12400. Aside from the addtional fixes, I like this cleaner approach to config btw, the hardcoded NODE_MODULES was resolving to a wrong directory when RN itself is symlinked

@Kureev
Copy link
Contributor Author

Kureev commented Feb 17, 2017

Hi @Swaagie! Thanks for your PR, it looks really great. I think we can find a way how we can combine our PRs and ship both. I love the work you did with scoped modules, that's just great! Also, you caught a bug with projectRoots. I think it's a very valuable contribution to the project.

Maybe we can wait someone to review our PRs and find how we can merge them. I think in either case the merge is not going to be that big.

@Swaagie
Copy link

Swaagie commented Feb 17, 2017

Sounds like a plan.

@gaearon
Copy link
Collaborator

gaearon commented Feb 17, 2017

Thanks for the quick fix!

@Kureev
Copy link
Contributor Author

Kureev commented Feb 27, 2017

Rebased in a hope it'll fix CI builds

@ericvicenti
Copy link
Contributor

I can not repro the issue on master. Do we still need this?

@gaearon
Copy link
Collaborator

gaearon commented Mar 3, 2017

By can't repro, do you mean it doesn't crawl to folders above, or that you just don't see the same error? My issue description is specific to my computer—if you don't have a broken symlink, you won't see the same error. But the problem is it crawls parent directories in the first place.

@gaearon
Copy link
Collaborator

gaearon commented Mar 3, 2017

This is the key part of my report:

screen shot 2017-03-03 at 11 45 30 pm

You won't see the same error if you don't have broken symlinks in a folder above.

@Kureev
Copy link
Contributor Author

Kureev commented Mar 5, 2017

I just double-checked the code in master and it still looks broken. That's how you can reproduce this issue on Mac:

  • In the folder where you have react-native repo run:
    $ ln -s foo ./bar
    
    (here I assume that "bar" doesn't exist and symlink is broken)
  • Go to react-native/examples/UIExplorer and open UIExplorer.xcodeproj
  • Run

You should see something like this:
image

@Kureev
Copy link
Contributor Author

Kureev commented Mar 5, 2017

I'd also like to explain the reason why this issue occurs: there is the line that defines a path to node_modules as ../../../ (because usually local-cli is located in node_modules/react-native/local-cli which is apparently not the case if you run a project from examples in RN repo: react-native/examples/UIExplorer).

Let me know if you need any additional information to review this PR. Also, feel free to ping me on Slack.

@ericvicenti
Copy link
Contributor

Thanks @Kureev, this is a well documented fix. My apologies for glossing over the important detail. I'll land it now.

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Mar 5, 2017
@facebook-github-bot
Copy link
Contributor

@ericvicenti has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
Motivation:
Few days ago gaearon [filed an issue](facebook/react-native#12406) that examples in the react-native repo doesn't work after the [recent changes in local-cli](facebook/react-native@bce6ece). This PR fixes reported bug.

**Test plan (required)**
- No UI changes
- [x] Run UIExplorer from XCode (no package.json in the folder) and check if packager/application runs correctly

cc davidaurelio satya164 grabbou
Closes facebook/react-native#12435

Differential Revision: D4657370

Pulled By: ericvicenti

fbshipit-source-id: 72ee4b96cae37c7ed2794ed4490ce7b4fbbd66c3
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants