Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add gradle plugin and androidX versions into project-level gradle #151

Closed
SaeedZhiany opened this issue Aug 15, 2019 · 34 comments
Closed

Add gradle plugin and androidX versions into project-level gradle #151

SaeedZhiany opened this issue Aug 15, 2019 · 34 comments
Labels
🗣 Discussion This label identifies an ongoing discussion on a subject

Comments

@SaeedZhiany
Copy link

SaeedZhiany commented Aug 15, 2019

Introduction

Hi, everybody,
I come from a discussion from here.
The discussion briefly is about how we should handle the version of androidX packages used by RN libraries from main project gradle file in a consistency way.

The Core of It

The main problem before RN 0.60.x is that RN library developers using his/her preferred gradle plugin version within their libraries without any attention to main project gradle plugin version. also, due to inactivity of its maintainer, there are lots of RN libraries that not using latest gradle plugin and even sending PR to upgrade the version doesn't help due to lack of PR reviews by the maintainers. so it causes some sort of problems in building android project due to multiple gradle plugin version used in the process. so CI servers usually fail in the building (and in my case, I can't automate my release building)
some workaround for the issue (that I currently use it) is writing a script that reads main project's gradle plugin version and replaces the versions in the libraries' gradle files to make all the libraries compatible with the main project. to be honest I don't like such patchy works. I like to solve problems from their root.

Recently, androidX support has been added to RN and library developers has begun to migrate their libraries to using androidX instead of support libraries. react-native doesn't include androidX packages' version into gradle file, so I think the second wave of nightmares(!!!) will come shortly.

Each developer has own strategy to migrate.

  1. Some of them use a fix version number just like to gradle plugin version and obviously, it has
    the same problem as a result.

  2. The others assume that an extra property is defined in project-level gradle with a name that their library is using.
    although this is a better way compared to the first one, it has another problem. due to the lacking naming convention, every developer can assume their pattern for the name of androidX packages within ext. as an example for androidX's annotation package, developer A assumes an extra property with the name androidXAnnotation and developer B can assume androidX_annotation and so on.)

So I start this discussion to make a general solution that avoids patchy workarounds and solve the mentioned issues in a more precise way.

solution for gradle plugin version
I currently submit a PR for android gradle plugin issue here but unfortunately, it does not even receive a precise review yet, only one opposition that can not convince me why my proposed solution isn't correct.

solution for androidX package's version
This is just an idea in my mind from discussing with @mikehardy, @matt-oakes, and @jdnichollsc, (it's would so good if you take a time to read the discussion)

I know androidX packages are separately maintained and have their versioning scheme that is not necessarily the same among the packages so having lots of properties within the main project gradle file seems not a good solution.
So I'm thinking about having a separated RN project-level file in for specifying the packages' version, then somehow importing them in project-level gradle as extra properties using gradle commands. I don't much familiar with gradle system and groovy language so I couldn't submit a PR for that. is anybody can confirm if is it possible to do such thing at all? and if yes, how we can achieve that.

Discussion points

  • I like to reach a naming convention about androidX packages so RN library developers can rely on that to import and use the packages' version in their libraries in a consistent way with the other libs used in an RN project.

  • Also, Finding a solution to make RN libraries automatically use project-level gradle plugin version.

The sooner the better, because I think the more time passes, the harder it becomes to solve these problems.

@mikehardy
Copy link

Useful discussion I think

Do we need to specify these version?

Per the linked discussion, I'm not aware of a solution that currently meets all use cases and is better than specifying library-specific versions in a gradle-consumable way, somehow. We're currently using android/build.gradle::ext{} scope variables for that in projects, and it works okay.

Where to specify the versions if we need to specify them?

The list of library versions specified in that threatens to grow yes, but I admit I like simple solutions. I see no reason to have what appears to me to be an extra complication of another file, and an import mechanism. The android/build.gradle file is not very large, it is already under edit sometimes, it is known, and the ext{} block is pretty easy to handle. So I'd just keep it there.

What naming convention if we need to specify versions?

I'd love to have a naming convention though. I propose just using lower-case-first / camel-case / +Version for names. So if you needed the androidx.appcompat:appcompat dependency, you'd specify appcompatVersion. For androidx.vectordrawable, you'd specify vectordrawableVersion etc. Maybe this sucks? The important thing is to agree :-), propose something better as a convention formula and we can go with it.

Can we centralize the gradle versions?

I also like the idea of the gradle plugin version itself specified. I hate building my project on a new machine where it downloads 10 different gradle versions. For truly poorly maintained repos, the solution is to not use the repo. That's harsh but it's open source. Fork the thing and maintain it. Alternatively use patch-package, or don't use it. But even well-maintained packages skew between patch versions of gradle and harmony would be nice. So I'd like to see a gradle version specified in a single location if possible as well.

@kelset kelset added the 🗣 Discussion This label identifies an ongoing discussion on a subject label Aug 15, 2019
@SaeedZhiany
Copy link
Author

I see no reason to have what appears to me to be an extra complication of another file, and an import mechanism. The android/build.gradle file is not very large, it is already under edit sometimes, it is known, and the ext{} block is pretty easy to handle. So I'd just keep it there.

I also like to keep them just in gradle file, my idea is changed because of what @matt-oakes said in this comment

@jdnichollsc
Copy link

jdnichollsc commented Aug 15, 2019

Hello guys, this is my proposal https://github.com/proyecto26/react-native-inappbrowser/pull/97/files#diff-7ae5a9093507568eabbf35c3b0665732

So we can have a configuration like this from our projects:
android/build.gradle

buildscript {
    ext {
        buildToolsVersion = "28.0.3"
        minSdkVersion = 16
        compileSdkVersion = 28
        targetSdkVersion = 28
        supportLibVersion = "28.0.0"
        androidXVersion = "1.+" // Default version for AndroidX dependencies
        androidXAnnotation = "1.1.0"
        androidXBrowser = "1.0.0"
       // Put here other androidX dependencies
    }

Let me know what you think 👍

@SaeedZhiany
Copy link
Author

Thank you for your contributions and this is my opinion about your proposal.

@matt-oakes
Copy link
Member

I've added this to the other PR, but I thought it best to post here too.

I still think having dynamic versions in the library and letting Gradle resolve them as needed is the best solution. I get that non-determinism is an issue, but why not use the same tactic as Yarn and NPM and enable locking? Documentation for Gradle dependency locking is here:

https://docs.gradle.org/current/userguide/dependency_locking.html

@mikehardy
Copy link

I could see that working technically

So there would be a new file with state https://docs.gradle.org/current/userguide/dependency_locking.html#lock_state_location_and_format

...requiring folks to manage that after adding locking directives to the gradles. I'm not excited about externalizing state personally but that's clearly a solution many like via social proof of package-lock/yarn-lock/Podfile.lock and as dependencies chain I can see why

I have not experienced anyone using this in pure native Android land (maybe it's just new to me though?) so I'm curious if culturally and documentation wise we'd be breaking trail.

Would this still handle the case where you could work with either version 28 of support or e.g. 1.0.x of some androidx lib? And maybe need to substitute the name? Or is this just a clean androidx future?

@jdnichollsc
Copy link

Hey guys, check an example of the new installation steps from the last version of my plugin https://github.com/proyecto26/react-native-inappbrowser#mostly-automatic-installation

@SaeedZhiany
Copy link
Author

For the Android Gradle Plugin issue, @dulmandakh has a different solution. he proposed his solution with a few PRs in the repositories below that can be used as samples to other maintainers to change their libraries.

@brodybits, please take a look at those PRs, if you find them useful, maybe it's better to start changing your template.

cc
@mikehardy, @matt-oakes, @jdnichollsc

brodycj pushed a commit to brodycj/create-react-native-module that referenced this issue Sep 16, 2019
from generated code

in order to avoid multiple Android build tools Gradle plugin versions
in React Native projects

as discussed in react-native-community/discussions-and-proposals#151
brodycj pushed a commit to brodycj/create-react-native-module that referenced this issue Sep 16, 2019
from the generated code in `android/build.gradle`

in order to avoid multiple Android build tools Gradle plugin versions
in React Native projects

as discussed in react-native-community/discussions-and-proposals#151
@SaeedZhiany
Copy link
Author

To keep this discussion up to date

@friederbluemle has another amazing solution for the gradle plugin version issue that is much simpler than @dulmandakh's approach, you can read his idea from this comment

@brodycj
Copy link

brodycj commented Sep 18, 2019

@friederbluemle has another amazing solution for the gradle plugin version issue that is much simpler than @dulmandakh's approach, you can read his idea from this comment

which is on the PR in facebook/react-native#25569 but I think should have been here instead:

[...]
I actually found an alternate solution, without moving any Gradle related files into the root of a project.

Essentially, it's just two lines, wrapping the Android Gradle plugin dependency in the buildscripts section:

if (project == rootProject) {
    // ... (dependency here)
}

With this, the dependency is only evaluated (and possibly downloaded) when the project is opened stand-alone (opening the android folder in Android Studio).

When a library project is included as a dependency, then rootProject will be the main application project. -> the statement will evaluate to false, skipping the potentially outdated Android Gradle plugin download.

I opened a bunch of PRs:

react-native-community/react-native-localize#69
react-native-community/react-native-share#588
react-native-community/async-storage#209
FormidableLabs/react-native-app-auth#384
[...]

@brodycj
Copy link

brodycj commented Sep 18, 2019

and quoting some more comments from facebook/react-native#25569 that I think should have been here instead:

by @friederbluemle in facebook/react-native#25569 (comment) (with extra blank lines added to improve readability a little):

[...]
For an application, the only wrapper that will be used is the one that is distributed with the application project itself (in the android folder). Any other Gradle wrappers that might be contained in node_modules will not be used. In fact, these files should not be distributed by library projects in the first place, since it would only result in an increased artifact file size.

By not moving anything into the root directory, it will keep everything related to Android contained in the android folder, and everything related to iOS in the ios folder (just like in a regular RN application project).

What is distributed in the end is controlled by other means. Have a look at the files section of package.json, and npmignore.
[...]

by @friederbluemle in facebook/react-native#25569 (comment) (with a blank line added for the sake of readability):

There are definitely many other library projects that incorrectly distribute the Gradle wrapper (an others) files. Even with the updated react-native-keychain project, the wrapper is still distributed - It's just outside the android folder. In either case, it should not result in any conflicts (just increased download size).

Regarding a quick script that locally fixes up build.gradle files from library projects that have not been updated yet: Great idea! It does not even have to be as complex as wrapping something. Simply removing the whole buildscripts {} block should be fine too. As explained, it's only needed when building/opening the project stand-alone, and can safely be removed from dependencies inside of node_modules (to avoid the extra downloads).

by @friederbluemle in facebook/react-native#25569 (comment):

Quick update to npmignore and files in package.json over at react-native-keychain: oblador/react-native-keychain#248

another comment by @mikehardy in facebook/react-native#25569 (comment):

The (if project == rootProject) idea seems much more "android-native" to me, and conceptually simpler, while apparently meeting the same goals.

@friederbluemle
Copy link

Thanks @SaeedZhiany for keeping this up-to-date.

For reference, here are four PRs with the alternate if (project == rootProject) solution:

zoontek/react-native-localize#69
react-native-share/react-native-share#588
react-native-async-storage/async-storage#209
FormidableLabs/react-native-app-auth#384

Since this satisfies the core problem (unnecessary downloads of/conflicts with different Android Gradle plugin distributions), would you be OK with closing this issue, as well as the related issue/pr over on https://github.com/facebook/react-native/

We can still work on temporary local script to remove the buildscripts {} section separately (or you could open a new issue).

@friederbluemle
Copy link

Also thanks @brodybits for more references 👍

@brodycj
Copy link

brodycj commented Sep 18, 2019

+1 for closing outdated and unwanted PRs such as facebook/react-native#25569 and closed PR in brodycj/create-react-native-module#129

-1 for closing discussions such as this one which I still think need some more work and maybe further consideration from the people involved, including myself

I do think this discussion was started with too many things combined. I think the androidx version thing should have been in a separate discussion.

@friederbluemle
Copy link

Agreed, splitting it into more focussed discussions would be best 👍

@SaeedZhiany
Copy link
Author

@friederbluemle
I had mentioned about only two problems, the AGP and androidX version management (that seems you already read above comments).
the AGP problem can be considered solved with your idea as a final solution, I can edit my first comment in this issue to mark it as resolved and we can continue the discussion about androidX package in this issue.

-1 for closing discussions such as this one which I still think need some more work and maybe further consideration from the people involved, including myself

Why? we can keep this issue opened and add more ideas whenever we found a new one. because the only problem remains in this issue is the AndroidX package management and it can be considered as a new issue thread.

Although I don't see any need, If all of you agreed we can open a new one.

@SaeedZhiany
Copy link
Author

SaeedZhiany commented Sep 18, 2019

However, we need to

  1. write and publish a blog in react-native website to inform other module maintainers about @friederbluemle solution
  2. thinking about how we can integrate the script (that removes the buildscripts {}) into CLI autolink procedure to make the procedure fully automated, so users even don't need to know about AGP issue and its solution.

by these, I agree with splitting androidX issue into a sperate discussion.

Is it necessary to split 1) and 2) too?

@brodycj
Copy link

brodycj commented Sep 18, 2019

by these, I agree with splitting androidX issue into a sperate discussion.

(separate discussion)

+1, I think we have agreement now

Is it necessary to split 1) and 2) too?

I think that would make sense. TBD I am not so sure that I fully understand the points in "2)".

@SaeedZhiany
Copy link
Author

I think we have agreement now

then I'll open a new discussion about AndroidX as soon as I can.

I think that would make sense. TBD I am not so sure that I fully understand the points in "2)".

It's an idea from this comment. for modules that do not apply @friederbluemle approach, we think it would great if we write a local script that can search npm_module directory and wrap the buildscripts with the if statement or simply remove it completely.
I don't know if you heard about new CLI autolink, it's a replacement for react-native link command. if the script is written locally by each user, it can be called in postinstall of package.json. by 2) I mean that maybe we can write the script and integrate it into CLI AutoLink to make it a fully automated procedure

@friederbluemle
Copy link

A script that modifies locally downloaded dependencies in node_modules would be good as a (very) short-term workaround. A better and more sustainable longer term solution is to update the library projects to include the line, so that no additional local steps/postinstall hooks etc are necessary.

@brodycj
Copy link

brodycj commented Sep 18, 2019

@friederbluemle can you do us a favor and open a new issue that describes the solution you proposed in facebook/react-native#25569 (comment), maybe with some clarification, and nothing else?

I think this could really help us understand what is going on in the fix you proposed in brodycj/create-react-native-module#135, among many other places, and enable people to ask questions and understand it without the surrounding noise.

The Gradle stuff is quite new for me and I suspect it would be new for a number of other people.

Yes I did hear of autolink, still don't understand it so well.

I guess a workaround script could help us with a short-term problem, don't have a clue how it would work. Doesn't sound like such a stable solution.

@dulmandakh
Copy link

I'm happy that we found the solution to a very common issue across RN module ecosystem. Now we need to work to spread the solution.

I don't think that AndroidX version sharing is solvable because each and every package has it's own versions, unlike Android Support Library. See facebook/react-native#26481, I'm trying to upgrade AppCompat version to 1.1.0.

@SaeedZhiany
Copy link
Author

A script that modifies locally downloaded dependencies in node_modules would be good as a (very) short-term workaround. A better and more sustainable longer term solution is to update the library projects to include the line, so that no additional local steps/postinstall hooks etc are necessary.

Ok, then, we can put the script file in the blog that users just copy and paste in their project and use it.

@SaeedZhiany
Copy link
Author

I guess a workaround script could help us with a short-term problem, don't have a clue how it would work. Doesn't sound like such a stable solution.

the script is going to do the same work that @friederbluemle proposed but just on locally installed dependencies in npm_modules directory. it can be seen as a workaround for that modules that don't apply the if statement yet. so it's completely a stable solution.

@mikehardy
Copy link

@SaeedZhiany do it like jetifier - make a node module that does the work, document it well, and release it. If it is useful it gets traction, and if it is really useful then after it is stabilized it might be called by react-native cli either as an option or default. Growing a tool on the side like that gives it time to settle (jetifier took 19 releases, FWIW, and that was with a test suite operating on a fair chunk of popular modules - even though it was doing something entirely proven already in other tools with an existing mapping file :-), so that settling period is more important than it seems)

@SaeedZhiany
Copy link
Author

I don't think that AndroidX version sharing is solvable because each and every package has it's own versions, unlike Android Support Library.

yeah, we know about that and it has already a solution but some of us like @matt-oakes seems doesn't like the idea. I don't want to continue the discussion about androidX here and I'll open a new one for that as soon as I can. but you can read about that idea by reading a few first comments in this issue and also this issue

@SaeedZhiany
Copy link
Author

@mikehardy it just a simple script with at most 40 lines of code, I just didn't sure if it's worth to create a repository for it or not. but it's a good idea, we can create a simple repo for it and reference it within the blog. I insist on writing the blog because it has more chance to hit more audiences. it can help us to spread the solution with more speed.

@dulmandakh
Copy link

I don't like the idea of dependency locking, because developers have to create a new lock file every time they install/update/delete NPM packages. Or we must make it invisible, or so simple.

@SaeedZhiany
Copy link
Author

It's just one of the ideas, another solution is hardcode all androidX packages versions into main project gradle file, then all module maintainers should read their required versions from the main project, just like supportLibVersion. but maybe we can do it better with creating a separated file that includes all versions and then importing them in main Gradle file to be accessible for sub-modules. I don't know how we can do it in Gradle because of my low experience in Gradle system, but I'm sure you and @friederbluemle can help us with this. you two has very good experiences with Gradle.

I believe this issue is solvable too and it just needs some more times to find a good solution that satisfies everyone.

@dulmandakh
Copy link

Unlike supportLibVersion, there is no single AndroidX version, but each AndroidX package has its own version. So maintaining versions for AndroidX packages in app template in RN core is not feasible.

If a package depends on appcompat, it can safely remove it because it's already provided by RN.

Hard coding versions in main project gradle file might cause issues in case of package updates. For example, NPM package X requires AndroidX package of Y version of Z, and in the future maintainer wants to use newer version of package Y which has a new API, so a user of NPM package X must update NPM package, but also bump package Y version in main gradle to build it.

Instead package maintainers must use an AndroidX package with a version, and gradle will use latest safe version of it if used by multiple packages. For example, package A requires appcompat version 1.0, and package B requires appcompat version 1.1, then gradle will automatically use only version 1.1.

@mikehardy
Copy link

@SaeedZhiany jetifier started with less than 40 lines of code (and still worked! except all the tiny corner cases we discovered later, and performance, and...) :-) - a good tool deserves it's own repo, IMHO.

I personally don't mind having an ext{} block in my /android/build.gradle file at all 🤷‍♂ - it lets me pin my android dependencies pretty much right where I'd expect, in a file I have to have anyway

@SaeedZhiany
Copy link
Author

Unlike supportLibVersion, there is no single AndroidX version, but each AndroidX package has its own version.

yeah, we discussed about it before and we know about that.

So maintaining versions for AndroidX packages in app template in RN core is not feasible.

it just causes having a big template file, I don't see any other problem that causes infeasibility.

If a package depends on appcompat, it can safely remove it because it's already provided by RN.

I didn't get this statement. what do you mean?

Instead package maintainers must use an AndroidX package with a version, and gradle will use latest safe version of it if used by multiple packages. For example, package A requires appcompat version 1.0, and package B requires appcompat version 1.1, then gradle will automatically use only version 1.1.

Are you mean Gradle automatically downloads only the latest versions of androidX packages required? or downloads all versions used among all npm packages and only uses the latest version?

I think having the versions in the project's Gradle file forces the sub-modules maintainers to keep their code up-to-date whether directy by themself of indirectly by sub-modules users (by creating PR on that sub-modules repository) and this is very good.

@SaeedZhiany
Copy link
Author

side note: I'm on a short trip and I don't have access to my laptop, so typing with mobile is hard for me. can anyone create a new issue for androidX discussion and move all androidX related comment to the new issue, please?

@SaeedZhiany
Copy link
Author

@SaeedZhiany jetifier started with less than 40 lines of code (and still worked! except all the tiny corner cases we discovered later, and performance, and...) :-) - a good tool deserves it's own repo, IMHO.

Then I'll create a repo for it as soon as possible. :)

mikehardy added a commit to mikehardy/react-native-device-info that referenced this issue Sep 20, 2019
This is the outcome of a react-native-community discussion with the goal
of avoiding downloading multiple gradle versions and/or loading multiple
versions at the same time

react-native-community/discussions-and-proposals#151 (comment)
mikehardy added a commit to mikehardy/react-native-device-info that referenced this issue Oct 6, 2019
This is the outcome of a react-native-community discussion with the goal
of avoiding downloading multiple gradle versions and/or loading multiple
versions at the same time

react-native-community/discussions-and-proposals#151 (comment)
mikehardy added a commit to react-native-device-info/react-native-device-info that referenced this issue Oct 6, 2019
This is the outcome of a react-native-community discussion with the goal
of avoiding downloading multiple gradle versions and/or loading multiple
versions at the same time

react-native-community/discussions-and-proposals#151 (comment)
mikehardy added a commit to mikehardy/react-native-device-info that referenced this issue Oct 9, 2019
This is the outcome of a react-native-community discussion with the goal
of avoiding downloading multiple gradle versions and/or loading multiple
versions at the same time

react-native-community/discussions-and-proposals#151 (comment)
@kelset kelset closed this as completed Jun 10, 2021
@react-native-community react-native-community locked and limited conversation to collaborators Jun 10, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
🗣 Discussion This label identifies an ongoing discussion on a subject
Projects
None yet
Development

No branches or pull requests

8 participants