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

Update and fix Gradle setup #384

Merged
merged 2 commits into from
Jan 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .babelrc

This file was deleted.

19 changes: 9 additions & 10 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@

# OSX
#
.DS_Store

# node.js
#
node_modules/
npm-debug.log
yarn-error.log


# Xcode
#
build/
Expand All @@ -29,7 +21,6 @@ DerivedData
*.ipa
*.xcuserstate
project.xcworkspace


# Android/IntelliJ
#
Expand All @@ -38,9 +29,17 @@ build/
.gradle
local.properties
*.iml
android/gradle/
android/gradlew
android/gradlew.bat

# node.js
#
node_modules/
npm-debug.log
yarn-error.log

# BUCK
buck-out/
\.buckd/
*.keystore

10 changes: 10 additions & 0 deletions android/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
*.iml
.DS_Store
.gradle/
.idea/
.npmignore
build/
gradle/
gradlew
gradlew.bat
local.properties
38 changes: 23 additions & 15 deletions android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@ def safeExtGet(prop, fallback) {
}

buildscript {
repositories {
maven {
url 'https://maven.google.com/'
name 'Google'
// 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.
if (project == rootProject) {
repositories {
google()
jcenter()
}
dependencies {
classpath 'com.android.tools.build:gradle:3.5.3'
}
jcenter()
}

dependencies {
classpath 'com.android.tools.build:gradle:3.2.1'
}
}

apply plugin: 'com.android.library'

android {
compileSdkVersion safeExtGet('compileSdkVersion', 27)
buildToolsVersion safeExtGet('buildToolsVersion', '28.0.2')

compileSdkVersion safeExtGet('compileSdkVersion', 28)
buildToolsVersion safeExtGet('buildToolsVersion', '28.0.3')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

defaultConfig {
minSdkVersion safeExtGet('minSdkVersion', 16)
targetSdkVersion safeExtGet('targetSdkVersion', 26)
Expand All @@ -37,14 +37,22 @@ android {
}

repositories {
mavenLocal()
maven {
// All of React Native (JS, Obj-C sources, Android binaries) is installed from npm
url "$rootDir/../node_modules/react-native/android"
}
maven {
url 'https://maven.google.com/'
name 'Google'
// Android JSC is installed from npm
url "$rootDir/../node_modules/jsc-android/dist"
}
google()
jcenter()
}

dependencies {
implementation 'com.facebook.react:react-native:'+safeExtGet('reactNativeVersion', '+')
//noinspection GradleDynamicVersion
implementation 'com.facebook.react:react-native:+' // From node_modules
implementation 'net.openid:appauth:0.7.1'
implementation 'androidx.browser:browser:1.2.0'
}
2 changes: 0 additions & 2 deletions android/gradle.properties

This file was deleted.

Binary file removed android/gradle/wrapper/gradle-wrapper.jar
Binary file not shown.
6 changes: 0 additions & 6 deletions android/gradle/wrapper/gradle-wrapper.properties

This file was deleted.

160 changes: 0 additions & 160 deletions android/gradlew

This file was deleted.

90 changes: 0 additions & 90 deletions android/gradlew.bat

This file was deleted.

3 changes: 3 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
presets: ['module:metro-react-native-babel-preset'],
};
6 changes: 6 additions & 0 deletions ios/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
*/project.xcworkspace/
*/xcuserdata/
.DS_Store
.npmignore
Pods/
build/
Loading