Update and fix Gradle setup #588

friederbluemle · 2019-09-18T03:33:35Z

Overview

Android Gradle plugin

This wraps the Android Gradle plugin dependency in the buildscripts section of 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).

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).

UPDATE 2019-11-19: Due to CI changes since this PR was opened, the Gradle wrapper has temporarily been added back. It will be removed in a follow-up PR that also includes other simplifications to the CI config.

Support lib conflict

There was also an appcompat-v7 leftover in build.gradle which caused the following error in Android Studio:

ERROR: Manifest merger failed : Attribute application@appComponentFactory value=(androidx.core.app.CoreComponentFactory) from [androidx.core:core:1.0.1] AndroidManifest.xml:22:18-86
	is also present at [com.android.support:support-compat:28.0.0] AndroidManifest.xml:22:18-91 value=(android.support.v4.app.CoreComponentFactory).
	Suggestion: add 'tools:replace="android:appComponentFactory"' to <application> element at manifestMerger6643979123070804112.xml:7:5-9:19 to override.

Since this project has already been converted to AndroidX, and React Native 0.60+ pulls in the androidx dependencies, this line is no longer needed.

.npmignore config

In a separate commit, this PR also includes a small update to the npmignore config of the project. By defining precisely which files get added to the distributed archive, along with excluding things like the Gradle wrapper jar a significant size reduction of the distribution artifact to almost a third of the previous value was achieved:

Before:

npm notice === Tarball Details ===
npm notice name:          react-native-share
npm notice version:       2.0.0
npm notice filename:      react-native-share-2.0.0.tgz
npm notice package size:  91.1 kB
npm notice unpacked size: 224.2 kB
npm notice total files:   65

After:

npm notice name:          react-native-share
npm notice version:       2.0.0
npm notice filename:      react-native-share-2.0.0.tgz
npm notice package size:  35.9 kB
npm notice unpacked size: 159.3 kB
npm notice total files:   57

Test Plan

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!}}

jgcmarins

It looks great!
What do you think @mikehardy?

jgcmarins · 2019-09-19T03:52:13Z

.npmignore

-jsconfig.json
-commitlint.config.js
-changelog.js
-assets/*


I think we should keep this and not publish them to npm

Correct, it should not be published. Please also check the updated files section in package.json - It works in tandem with npmignore, and declares files that are included.

friederbluemle · 2019-10-23T19:05:49Z

Rebased onto latest master branch.

jgcmarins

it looks good
it would be cool to test against our example

friederbluemle · 2019-10-23T23:26:18Z

@jgcmarins - Thanks for your comment and review.
I tested this locally. Can you give more instructions on what you were thinking in terms of testing?
The CI error seems unrelated:

This job has been blocked because resource-class:large is not available on your plan. Please upgrade to continue building.

jgcmarins · 2019-10-23T23:39:40Z

I was thinking of testing against our example

The CI error seems unrelated

yes, it's unrelated

MateusAndrade · 2019-10-30T02:44:45Z

@jgcmarins do tou think you are good to go with this PR? I was seeing that the CI are broken, but this dont looks related to this

jgcmarins · 2019-10-30T03:38:28Z

Yes
I think if it's following React Native best practices for gradle, it's ok to be merged

mikehardy · 2019-10-30T20:21:33Z

ios/.npmignore

+*/xcuserdata/
+.DS_Store
+.npmignore
+Pods/


@friederbluemle just a question - I notice you put sub-.npmignore files in here and android and I just recently published a react-native-device-info package with accidental inclusions of stuff here, even though I attempted to npmignore them at the top level. Have you seen that before? Is that why you have layered npmignore going on in this PR?

Hey @mikehardy - Good question. The way NPM handles npmignore, gitignore, and the files field in package.json can be surprising and unintuitive. There are a bunch of special cases if one file exists but not the other, the files field is present or not, if matches are located at the root level or in a subfolder etc.
The reason I put two .npmignore files in the android/ and ios/ folder is because the very precisely ignore only files related to that platform with minimal duplication (for example, you won't find a .gradle/ in the ios/ folder).
Since android/ and ios/ are included using the files field in package.json, it is not possible to just use one .npmignore file at the root level to exclude files from those directories. Hope that makes sense.

Clear as mud :-), but the tools' fault not yours as it is not easy to un-tangle how the different include/excludes work (what priority, whitelist or blacklist, when directives short-circuit other directives etc). For now in RNDI I just do a clean pre-package but I had not thought of nested npmignore files. Thanks for the explanation

Here's a good article that explains some of it in more detail:
https://medium.com/@jdxcode/for-the-love-of-god-dont-use-npmignore-f93c08909d8d

I also opened this one in react-native-device-info with similar changes:
react-native-device-info/react-native-device-info#872

jgcmarins

great job here
thanks

friederbluemle · 2019-11-13T19:01:14Z

Rebased and updated to latest Android Gradle plugin 3.5.2.

@jgcmarins - Could you take another quick look at this and merge if you have a minute? Thanks! 🙏

jgcmarins

Looks great

mikehardy · 2019-11-19T21:55:11Z

android/build.gradle

-          url 'https://maven.google.com'
+    // The Android Gradle plugin is only required when opening the android folder stand-alone.
+    // This avoids unnecessary downloads and potential conflicts when the library is included as a
+    // module dependency in an application project.


@friederbluemle this is not really the place to mention it but this just came through and reminded me - the one place I've seen this style fail is react-native-webview. It depends on kotlin, and that has to be on the classpath or the module build breaks a project build, but most projects (? my project anyway) does not by default have kotlin yet. So until (if?) the react-native template is altered to include kotlin not all modules may be de-duplicated like this. To be clear: not related to this PR, just for information

Thanks for the heads-up @mikehardy 👌

friederbluemle · 2019-11-19T22:00:49Z

@jgcmarins - Rebased again onto the latest master branch. CI is green again too.
I would really appreciate a merge soon. Thanks!

jgcmarins

great job, thanks

jgcmarins · 2019-11-20T12:13:17Z

@friederbluemle should we upgrade example as well?

friederbluemle · 2019-11-20T23:41:33Z

@jgcmarins Thanks for merging! 👍
Yes, latest react-native for example/ here: #637 😄

* master-base: (215 commits) Update react-native to 0.61.4 (react-native-share#637) Update and fix Gradle setup (react-native-share#588) set useNativeDriver explicitly for Animations (react-native-share#633) correctly type image source (react-native-share#632) ci(fix): adapt to new workflow (react-native-share#626) fix: added missing "v" to `source` field in podspec (react-native-share#619) chore(readme): mocking with Jest example (react-native-share#610) iOS 13: Resolve promise if ShareSheet is manually dismissed (react-native-share#607) Update README.md fix: remove uncessary tools replace on build gradle chore(lint): fixing lint errors Makes sharing with Snapchat work on Android (react-native-share#464) Fixed validate error Removed react-native Share dependency Only call successCallback when completed is true Change readme.md Update Gradle wrapper to 5.6.2 build(deps): bump eslint-utils from 1.4.0 to 1.4.2 (react-native-share#580) Send email to predefined email address - Example Send email to predefined email address ...

friederbluemle requested review from jgcmarins, MateusAndrade and mikehardy as code owners September 18, 2019 03:33

friederbluemle mentioned this pull request Sep 18, 2019

Update build.gradle facebook/react-native#25569

Closed

friederbluemle force-pushed the update-gradle-setup branch from b69cc2a to 444404f Compare September 18, 2019 03:57

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

jgcmarins 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-gradle-setup branch from 444404f to a30a2d0 Compare October 23, 2019 18:34

jgcmarins previously approved these changes Oct 23, 2019

View reviewed changes

friederbluemle dismissed jgcmarins’s stale review via 5eb4f5f October 30, 2019 18:39

friederbluemle force-pushed the update-gradle-setup branch from a30a2d0 to 5eb4f5f Compare October 30, 2019 18:39

mikehardy reviewed Oct 30, 2019

View reviewed changes

jgcmarins previously approved these changes Oct 31, 2019

View reviewed changes

friederbluemle dismissed jgcmarins’s stale review via 69061d8 November 13, 2019 18:59

friederbluemle force-pushed the update-gradle-setup branch from 5eb4f5f to 69061d8 Compare November 13, 2019 18:59

jgcmarins previously approved these changes Nov 13, 2019

View reviewed changes

friederbluemle added 2 commits November 19, 2019 13:49

Update .npmignore config

c06752a

Update and fix build.gradle setup

eeb7817

friederbluemle dismissed jgcmarins’s stale review via eeb7817 November 19, 2019 21:51

friederbluemle force-pushed the update-gradle-setup branch from 69061d8 to eeb7817 Compare November 19, 2019 21:51

mikehardy reviewed Nov 19, 2019

View reviewed changes

jgcmarins approved these changes Nov 20, 2019

View reviewed changes

jgcmarins merged commit c7287a3 into react-native-share:master Nov 20, 2019

friederbluemle deleted the update-gradle-setup branch November 20, 2019 18:46

MateusAndrade mentioned this pull request Jan 16, 2020

build: setting project version to 3.0.0 #675

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update and fix Gradle setup #588

Update and fix Gradle setup #588

friederbluemle commented Sep 18, 2019 •

edited

Loading

jgcmarins left a comment

jgcmarins Sep 19, 2019

friederbluemle Sep 19, 2019

friederbluemle commented Oct 23, 2019

jgcmarins left a comment

friederbluemle commented Oct 23, 2019

jgcmarins commented Oct 23, 2019

MateusAndrade commented Oct 30, 2019

jgcmarins commented Oct 30, 2019

mikehardy Oct 30, 2019

friederbluemle Oct 30, 2019 •

edited

Loading

mikehardy Oct 30, 2019

friederbluemle Oct 30, 2019

jgcmarins left a comment

friederbluemle commented Nov 13, 2019

jgcmarins left a comment

mikehardy Nov 19, 2019

friederbluemle Nov 19, 2019

friederbluemle commented Nov 19, 2019

jgcmarins left a comment

jgcmarins commented Nov 20, 2019

friederbluemle commented Nov 20, 2019

Update and fix Gradle setup #588

Update and fix Gradle setup #588

Conversation

friederbluemle commented Sep 18, 2019 • edited Loading

Overview

Android Gradle plugin

Gradle wrapper

Support lib conflict

.npmignore config

Test Plan

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

jgcmarins left a comment

Choose a reason for hiding this comment

jgcmarins Sep 19, 2019

Choose a reason for hiding this comment

friederbluemle Sep 19, 2019

Choose a reason for hiding this comment

friederbluemle commented Oct 23, 2019

jgcmarins left a comment

Choose a reason for hiding this comment

friederbluemle commented Oct 23, 2019

jgcmarins commented Oct 23, 2019

MateusAndrade commented Oct 30, 2019

jgcmarins commented Oct 30, 2019

mikehardy Oct 30, 2019

Choose a reason for hiding this comment

friederbluemle Oct 30, 2019 • edited Loading

Choose a reason for hiding this comment

mikehardy Oct 30, 2019

Choose a reason for hiding this comment

friederbluemle Oct 30, 2019

Choose a reason for hiding this comment

jgcmarins left a comment

Choose a reason for hiding this comment

friederbluemle commented Nov 13, 2019

jgcmarins left a comment

Choose a reason for hiding this comment

mikehardy Nov 19, 2019

Choose a reason for hiding this comment

friederbluemle Nov 19, 2019

Choose a reason for hiding this comment

friederbluemle commented Nov 19, 2019

jgcmarins left a comment

Choose a reason for hiding this comment

jgcmarins commented Nov 20, 2019

friederbluemle commented Nov 20, 2019

friederbluemle commented Sep 18, 2019 •

edited

Loading

friederbluemle Oct 30, 2019 •

edited

Loading