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

Wrong path in settings.gradle when linking dependency on Windows #129

Closed
cherniavskii opened this issue Jan 25, 2019 · 6 comments
Closed
Labels
bug Something isn't working

Comments

@cherniavskii
Copy link

Environment

react-native: 0.58.1

System:
OS: Windows 10
CPU: (8) x64 Intel(R) Core(TM) i7-4712HQ CPU @ 2.30GHz
Memory: 6.09 GB / 15.91 GB
Binaries:
Yarn: 1.13.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
npm: 6.7.0 - C:\Program Files\nodejs\npm.CMD
IDEs:
Android Studio: Version 3.3.0.0 AI-182.5107.16.33.5199772

Description

Expected path in android/settings.gradle:

project(':react-native-gesture-handler').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-gesture-handler/android')

Actual path:

project(':react-native-gesture-handler').projectDir = new File(rootProject.projectDir, '..\node_modules\react-native-gesture-handler\android')

Because of backslashes, running react-native run-android fails.

Note, that this works as expected in react-native 0.57.8, because there's additional code:

/*
   * Fix for Windows
   * Backslashes is the escape character and will result in
   * an invalid path in settings.gradle
   * https://github.com/rnpm/rnpm/issues/113
   */
  if (isWin) {
    projectDir = projectDir.replace(/\\/g, '/');
  }

See https://unpkg.com/react-native@0.57.8/local-cli/link/android/patches/makeSettingsPatch.js

There's no such a fix for Windows in 0.58.1, see https://unpkg.com/react-native@0.58.1/local-cli/link/android/patches/makeSettingsPatch.js

I've checked History and Blame on that file in this repo, but didn't find any trace of that fix.

Let me know if that's appropriate place to submit local-cli issues, since I'm not sure about that.

Also, I can submit a PR reverting that fix.

Reproducible Demo

Steps to reproduce:

  1. install package (e.g. yarn add react-native-gesture-handler)
  2. link package (react-native link react-native-gesture-handler)
@cherniavskii cherniavskii added the bug Something isn't working label Jan 25, 2019
@thymikee
Copy link
Member

0.58.1 runs on cli located in RN repo: https://github.com/facebook/react-native/tree/0.58-stable/local-cli

@cherniavskii can you trace the commit this was added? I'm sure @grabbou can cherry-pick it to 0.58-stable branch.

We'll also add similar fix to this repo as well.

@cherniavskii
Copy link
Author

@thymikee thanks for clarification.

That Windows patch was removed in facebook/react-native#21203, it has something to do with CI, but IMHO that patch should be added back.

BTW, why is cli code duplicated in react-native and react-native-cli repos? Just curious how it works.

@thymikee
Copy link
Member

CLI was recently extracted to this repository so we can iterate faster.

@cherniavskii
Copy link
Author

@thymikee makes sense, thanks!

Would you like me to submit a PR?

@thymikee
Copy link
Member

I'm working on it right now, will appreciate your review and e2e testing, as I don't have Windows machine set up at the moment

@cherniavskii
Copy link
Author

Sure

Esemesek pushed a commit that referenced this issue Jan 28, 2019
Summary:
---------

Fixes #129


Test Plan:
----------

I added a test that simulates Windows on POSIX machines, because we don't run our tests on Windows.
@cherniavskii can you test this on a Windows machine?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants