-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Fixed Android cliPath Gradle configuration option for build.gradle #31839
Conversation
Hi @vitalyiegorov! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Base commit: 25a1612 |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Base commit: 25a1612 |
@sshic has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
1 similar comment
@sshic has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: This Diff is backporting the change in facebook#31839 applied to `react.gradle` also to the React Gradle Plugin. Ideally we would like to two logic to be in sync as much as possible. Changelog: [Internal] [Changed] - Backport the cliPath fix to the Gradle Plugin Differential Revision: D30899057 fbshipit-source-id: 1583d929760509337671263f135163c465dfea98
Summary: Pull Request resolved: #32193 This Diff is backporting the change in #31839 applied to `react.gradle` also to the React Gradle Plugin. Ideally we would like to two logic to be in sync as much as possible. Changelog: [Internal] [Changed] - Backport the cliPath fix to the Gradle Plugin Reviewed By: ShikaSD Differential Revision: D30899057 fbshipit-source-id: a28628b36b3dfe565dbdc8d6416c5d25ddf1fe03
@sshic merged this pull request in ce51b62. |
Hello, when this will be released? A monorepo structure depends on this as
|
This should be included in 0.67.0 |
Even though this issue is already closed, I want to add my two cents here since the change that "fixed" this issue broke what we had done to solve the problem of correctly finding the CLI in our monorepo. Pinning relative paths the way it's being done here really isn't the way to go... it'll lead you down a path where you're Doing something like what expo suggests -- that is resolving paths to node modules dynamically -- works much, much better. Expo's approach, however, returns an absolute path and -- prior to the change introduced in this PR -- worked flawlessly. Prepending the absolute path with another absolute path ( In order to work around the issue this PR introduces, we now had to add code that relativizes exclusively the CLI path so that it yields the correct path when concatenated with What I propose that is even more cumbersome but I guess better than what is available now is to add a check for |
I totally agree here. Frankly the problem is that we should not even use Ultimately, once the user passes a File reference, we should not transform it in any form. The fact that those properties were designed as |
@leoagomes #32983 is a jab at fixing this issue, also including trying to resolve the path via |
When using monorepo with react-native you need to provide
android/app/build.gradle
following params(cliPath
):With latest react-native
0.64.2
version you will get:Debugging this issue showed that providing
cliPath
options ends up building wrong path to cli:Summary
Changed
react.gradle
for proper File creation forcliPath
to support this configuration option.Changelog
[Android] [Fixed] - Changed
react.gradle
detectCliPath
function logic forcliPath
caseTest Plan
Run
./gradlew assembleRelease
or./gradlew assembleDebug