Update and fix Gradle setup #384

friederbluemle · 2019-09-18T03:34:43Z

Description

Android Gradle plugin

This wraps the Android Gradle plugin dependency in buildscripts section in android/build.gradle in a conditional:

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

A small change with a big impact:
The Android Gradle plugin is only required when opening the project stand-alone, not when it is included as a dependency. By doing this, the project opens correctly in Android Studio, and it can also be consumed as a native module dependency from an application project without affecting the app project (avoiding unnecessary downloads/conflicts/etc).

Please see related discussion in #381 - This PR replaces and closes #381.

Gradle wrapper

Similarly the Gradle wrapper (and related scripts) are only required for stand-alone editing/building of the android folder. It should not be distributed. Android Studio will automatically generate a wrapper, or alternatively one can be generated on the command line on-the-fly (see below).

Missing dependency

The dependencies section was missing a reference to androidx.browser:browser:1.0.0, causing a compile error in RNAppAuthModule.

Missing Android debug keystore in example

Due to an oversight, the debug.keystore file was ignored - This is already fixed on react-native's master branch: https://github.com/facebook/react-native/blob/master/template/_gitignore#L43
I re-added the keystore to fix the build error in the AndroidExample project.

.npmignore config

In a separate commit, this PR also includes a small update to the npmignore config of the project. By excluding the Gradle wrapper jar and scripts a significant size reduction of the distribution artifact to less than a third of the previous value was achieved:

Before:

npm notice === Tarball Details ===
npm notice name:          react-native-app-auth
npm notice version:       5.0.0-rc1
npm notice filename:      react-native-app-auth-5.0.0-rc1.tgz
npm notice package size:  74.6 kB
npm notice unpacked size: 157.0 kB
npm notice total files:   26

After:

npm notice name:          react-native-app-auth
npm notice version:       5.0.0-rc1
npm notice filename:      react-native-app-auth-5.0.0-rc1.tgz
npm notice package size:  22.8 kB
npm notice unpacked size: 96.1 kB
npm notice total files:   21

Steps to verify

What's required for testing (prerequisites)?

Android Studio and/or Gradle installed locally.

What are the steps to reproduce (after prerequisites)?

Open android in Android Studio:

studio android/

Or:

Generate a Gradle wrapper on the fly, and build on command line:

cd android/
gradle wrapper --gradle-version 5.4.1 --distribution-type all
./gradlew build

Successfully verified and tested on:

Android Device (Xiaomi Mi A2)
Android Emulator (Pixel 3a)
iOS Device (iPhone 8)
iOS Simulator (iPhone X)

_{^{Once approved, please use a normal GitHub merge (i.e. NO rebase/squash merge) to integrate the commit(s) from the PR head branch. The changes are broken up into meaningful, atomic commits, and my branch should already be up-to-date with the latest base branch. If it isn't, or if you want me to change anything, please let me know, and I will update the branch as soon as possible. Thank you!}}

dulmandakh · 2019-09-19T03:34:31Z

android/build.gradle

    }
 }

 apply plugin: 'com.android.library'

 android {
    compileSdkVersion safeExtGet('compileSdkVersion', 27)
-    buildToolsVersion safeExtGet('buildToolsVersion', '28.0.2')
-
+    buildToolsVersion safeExtGet('buildToolsVersion', '28.0.3')


it's not needed anymore, so it's better to remove it.

You're correct, with more recent versions of the Android Gradle plugin this is no longer required - However: At some point (about a year ago), adding this would actually cause a warning in the build log. Also Android Studio templates did not have this line anymore. Then something changed: After another update, the line was added again in the Android Studio template (it's even still present in the latest stable version 3.5.0). There is also no more warning, as far as I can tell. Do you happen to know more details by any chance?
Also - Since the version of the Android Gradle plugin is not determined by the library project, but by the app project, there is a chance that the app still uses an older version that requires this line (although today this chance is very low, or even non existent). Removing it could break it, while keeping it there can do no harm (other than possibly a warning). Thoughts?

Play Store requires 64bit support, so developers must use at least RN 0.59. And to have proper AndroidX support, developers use RN 0.60. So there is a zero chance to an app will use older version.

I understand. But do you know what's the reason this was reverted in the official template (i.e. the line reappeared, and it's no longer a warning)?

dulmandakh · 2019-09-19T23:24:03Z

Also, I think that we should have gradle wrapper since it maybe be used as a root project.

kadikraman · 2019-10-05T15:41:26Z

Hi @dulmandakh and @friederbluemle - thanks so much, and I apologise for the delay in getting back you you.

I've looked at both implementations and I'm tempted to go with this one. Are you both in agreement with this or is there more to be done in your opinion @dulmandakh ?

friederbluemle · 2019-10-06T20:00:04Z

Hi @kadikraman - Thanks for the review! 👍
I rebased the branch, and also updated the Android Gradle plugin to the now latest version (3.5.1). As explained, it will only be used when the project is opened stand-alone (it won't affect app projects using this library).

kadikraman · 2019-10-07T12:23:45Z

package.json

    "android",
-    "index.js",


Could you add these files back, please? In particular, removing the index.js would break deploying new versions (since only the files listed here will get pushed to npm)

index.js is set as main in package.json. Just like a bunch of other files, it is always included in the packed version, and there no need to manually specify it. You can just do a quick npm pack --dry-run locally to see that it is included even though it's not in the files list.

kadikraman · 2019-10-07T12:25:23Z

package.json

-    "react": "^16.1.1",
-    "react-native": "^0.50.3"
+    "react": "16.8.6",
+    "react-native": "0.60.3"


Are there any major breaking changes between 60.3, 60.2 and 60.1? If not, I'd prefer this to be ^0.60.

There are no major breaking changes that I know of (in the versions you mentioned). However, there no way to guarantee that. The reason I chose 0.60.3 here is to align it with the example project (we can update it independently in a separate PR). The problem with using ^0.60 here is that RN does not strictly follow semver (yet?). ^ fixes the major version only, so it would happily update it to 0.61.* (or 0.62.x once release etc). And those version might contain breaking changes (as well as accidentally become out-of-sync with a matching react version).

friederbluemle

In order to fix the failing CI (with the updated and fixed RN version), I also had to update and align a couple of other files/devDependencies (fixing the jest test runner). Please let me know if you prefer these changes to be done separately (as a prerequisite to this PR).

friederbluemle · 2019-11-05T19:50:37Z

Hi @kadikraman - Just a friendly reminder to please take another quick look at these changes and merge if everything is clear. I rebased the branch again onto the latest master branch. Thanks! 🙏

friederbluemle · 2019-11-13T18:58:05Z

Updated to the latest Android Gradle plugin 3.5.2.

friederbluemle · 2019-12-01T04:18:53Z

Rebased and resolved conflicts.

kadikraman · 2020-01-07T15:17:38Z

Hi @friederbluemle thanks for everything - could you please merge in the latest master (or give me push access to your fork, I've done it locally before realising I couldn't push 🤦‍♀ )

Hoping to get this merged this week.

friederbluemle · 2020-01-07T17:06:06Z

Hi @kadikraman - Thanks for getting back here, and happy new year!

I've rebased the branch and resolved the conflicts. One of the three commits (Re-add missing Android debug.keystore) was no longer applicable (since Example/AndroidExample was removed), so the commit got removed too.

To fix the following compile error, I had to update the default compileSdkVersion from 27 to 28:

appcompat-1.0.2/res/values-v28/values-v28.xml:5:5-8:13: AAPT: error: resource android:attr/dialogCornerRadius not found.

The project now builds without issues both on command line and in Android Studio. Both yarn test and yarn lint pass as well.

kadikraman · 2020-01-10T16:42:05Z

Released in v5.0.0 🎉

This was referenced Sep 18, 2019

fix gradle setup #381

Closed

Update build.gradle facebook/react-native#25569

Closed

friederbluemle force-pushed the update-project branch from 13f02a6 to 6694f68 Compare September 18, 2019 03:54

brodycj mentioned this pull request Sep 18, 2019

Add gradle plugin and androidX versions into project-level gradle react-native-community/discussions-and-proposals#151

Closed

dulmandakh reviewed Sep 19, 2019

View reviewed changes

brodycj mentioned this pull request Sep 19, 2019

[...] followup to android build.gradle template setup updates brodycj/create-react-native-module#143

Open

8 tasks

friederbluemle force-pushed the update-project branch 2 times, most recently from 68b3f15 to b1ac61c Compare October 6, 2019 19:58

kadikraman reviewed Oct 7, 2019

View reviewed changes

friederbluemle commented Oct 7, 2019

View reviewed changes

friederbluemle force-pushed the update-project branch from b1ac61c to d51fe72 Compare October 7, 2019 17:15

friederbluemle force-pushed the update-project branch from d51fe72 to f922518 Compare November 5, 2019 18:59

friederbluemle force-pushed the update-project branch from f922518 to b2f8cc1 Compare November 13, 2019 18:57

friederbluemle force-pushed the update-project branch from b2f8cc1 to cbe650c Compare December 1, 2019 04:18

friederbluemle added 2 commits January 7, 2020 08:28

Update .npmignore config

bec8ff3

Update and fix Gradle setup

f67ce6a

friederbluemle force-pushed the update-project branch from cbe650c to f67ce6a Compare January 7, 2020 17:06

kadikraman approved these changes Jan 8, 2020

View reviewed changes

kadikraman merged commit 6a7878d into FormidableLabs:master Jan 8, 2020

friederbluemle deleted the update-project branch January 8, 2020 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update and fix Gradle setup #384

Update and fix Gradle setup #384

friederbluemle commented Sep 18, 2019 •

edited

Loading

dulmandakh Sep 19, 2019

friederbluemle Sep 19, 2019

dulmandakh Sep 19, 2019

friederbluemle Sep 22, 2019

dulmandakh commented Sep 19, 2019

kadikraman commented Oct 5, 2019

friederbluemle commented Oct 6, 2019

kadikraman Oct 7, 2019

friederbluemle Oct 7, 2019

kadikraman Oct 7, 2019

friederbluemle Oct 7, 2019 •

edited

Loading

friederbluemle left a comment •

edited

Loading

friederbluemle commented Nov 5, 2019

friederbluemle commented Nov 13, 2019

friederbluemle commented Dec 1, 2019

kadikraman commented Jan 7, 2020

friederbluemle commented Jan 7, 2020

kadikraman commented Jan 10, 2020

Update and fix Gradle setup #384

Update and fix Gradle setup #384

Conversation

friederbluemle commented Sep 18, 2019 • edited Loading

Description

Android Gradle plugin

Gradle wrapper

Missing dependency

Missing Android debug keystore in example

.npmignore config

Steps to verify

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

dulmandakh Sep 19, 2019

Choose a reason for hiding this comment

friederbluemle Sep 19, 2019

Choose a reason for hiding this comment

dulmandakh Sep 19, 2019

Choose a reason for hiding this comment

friederbluemle Sep 22, 2019

Choose a reason for hiding this comment

dulmandakh commented Sep 19, 2019

kadikraman commented Oct 5, 2019

friederbluemle commented Oct 6, 2019

kadikraman Oct 7, 2019

Choose a reason for hiding this comment

friederbluemle Oct 7, 2019

Choose a reason for hiding this comment

kadikraman Oct 7, 2019

Choose a reason for hiding this comment

friederbluemle Oct 7, 2019 • edited Loading

Choose a reason for hiding this comment

friederbluemle left a comment • edited Loading

Choose a reason for hiding this comment

friederbluemle commented Nov 5, 2019

friederbluemle commented Nov 13, 2019

friederbluemle commented Dec 1, 2019

kadikraman commented Jan 7, 2020

friederbluemle commented Jan 7, 2020

kadikraman commented Jan 10, 2020

friederbluemle commented Sep 18, 2019 •

edited

Loading

friederbluemle Oct 7, 2019 •

edited

Loading

friederbluemle left a comment •

edited

Loading