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

feat: bump up fabric example react-native Android #3957

Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 1 addition & 3 deletions android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ android {
exclude "**/libreact_render*.so"
}

buildDir 'buildOutput_' + configStringPath
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary really necessary ? The idea of this path configuration is to avoid cleaning project after enabling or disabling a build option...

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 think it's necessary for using New Architecture by react-native.
If I don't remove this buildDir 'buildOutput_' + configStringPath settings, I get this error when I gradle sync in Android Studio.
Screenshot 2024-07-01 at 10 04 29 PM
Screenshot 2024-07-01 at 10 04 37 PM

I think react-native's settings are the main issue, but I think it would be easier to follow their pre-setup path for codegen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that screenshot means, react-native is finding out ./android/build/generated/source/codegen/jni/ but we only have wrong path because buildDir setting. (such as buildOutput_6777172ada0eb850223426c62199b5ab)
Screenshot 2024-07-01 at 10 06 15 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what is the main purpose of set up buildDir 'buildOutput_' + configStringPath in build.gradle?

Copy link
Member

Choose a reason for hiding this comment

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

We use it in order to make "clean" build each time we change some android config (eg. extensions like IMA or exo-player)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I have a plan for resolving this issue.

If buildDir is the only reason that we cannot merge this PR, my idea is like this.

First, revert this commit 62ec8d7

I think we can test Android Fabric without removing buildDir in build.gradle.
And let's consider buildDir later when Fabric implementation is almost done for Android.

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've pushed revert commit
Revert "feat: 🔥 turn off buildDir setting in android/build.gradle for…

Can you have a check my opinions? @freeboub

Copy link
Member

Choose a reason for hiding this comment

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

Sound good to me
btw now I understand why you want to remove it - I will do some research if we can "clean" build each time we change some android config in other way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KrzysztofMoch happy that you got my situation!
Your opinion would be another way to resolve this issue 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping @freeboub
Can you check this thread please?
I think we don't need to delay about this PR.


sourceSets {
main {
java {
Expand Down Expand Up @@ -212,7 +210,7 @@ dependencies {
if (ExoplayerDependencies["useExoplayerRtsp"]) {
implementation "androidx.media3:media3-exoplayer-rtsp:$media3_version"
}

// For ad insertion using the Interactive Media Ads SDK with ExoPlayer
if (ExoplayerDependencies["useExoplayerIMA"]) {
implementation "androidx.media3:media3-exoplayer-ima:$media3_version"
Expand Down
2 changes: 1 addition & 1 deletion examples/FabricExample/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
root: true,
extends: '@react-native-community',
extends: '@react-native',
parser: '@typescript-eslint/parser',
plugins: ['@typescript-eslint'],
overrides: [
Expand Down
3 changes: 3 additions & 0 deletions examples/FabricExample/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,6 @@ buck-out/
# Ruby / CocoaPods
/ios/Pods/
/vendor/bundle/

# testing
/coverage
5 changes: 3 additions & 2 deletions examples/FabricExample/Gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
source 'https://rubygems.org'

# You may use http://rbenv.org/ or https://rvm.io/ to install and use this version
ruby '2.7.5'
ruby ">= 2.6.10"

gem 'cocoapods', '~> 1.11', '>= 1.11.2'
gem 'cocoapods', '~> 1.13'
gem 'activesupport', '>= 6.1.7.3', '< 7.1.0'
37 changes: 22 additions & 15 deletions examples/FabricExample/Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
GEM
remote: https://rubygems.org/
specs:
CFPropertyList (3.0.6)
CFPropertyList (3.0.7)
base64
nkf
rexml
activesupport (7.0.4.3)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 1.6, < 2)
minitest (>= 5.1)
tzinfo (~> 2.0)
addressable (2.8.4)
public_suffix (>= 2.0.2, < 6.0)
addressable (2.8.7)
public_suffix (>= 2.0.2, < 7.0)
algoliasearch (1.27.5)
httpclient (~> 2.8, >= 2.8.3)
json (>= 1.5.1)
atomos (0.1.3)
base64 (0.2.0)
claide (1.1.0)
cocoapods (1.12.0)
cocoapods (1.15.2)
addressable (~> 2.8)
claide (>= 1.0.2, < 2.0)
cocoapods-core (= 1.12.0)
cocoapods-core (= 1.15.2)
cocoapods-deintegrate (>= 1.0.3, < 2.0)
cocoapods-downloader (>= 1.6.0, < 2.0)
cocoapods-downloader (>= 2.1, < 3.0)
cocoapods-plugins (>= 1.0.0, < 2.0)
cocoapods-search (>= 1.0.0, < 2.0)
cocoapods-trunk (>= 1.6.0, < 2.0)
Expand All @@ -32,8 +35,8 @@ GEM
molinillo (~> 0.8.0)
nap (~> 1.0)
ruby-macho (>= 2.3.0, < 3.0)
xcodeproj (>= 1.21.0, < 2.0)
cocoapods-core (1.12.0)
xcodeproj (>= 1.23.0, < 2.0)
cocoapods-core (1.15.2)
activesupport (>= 5.0, < 8)
addressable (~> 2.8)
algoliasearch (~> 1.0)
Expand All @@ -44,7 +47,7 @@ GEM
public_suffix (~> 4.0)
typhoeus (~> 1.0)
cocoapods-deintegrate (1.0.5)
cocoapods-downloader (1.6.3)
cocoapods-downloader (2.1)
cocoapods-plugins (1.0.0)
nap
cocoapods-search (1.0.1)
Expand All @@ -57,27 +60,30 @@ GEM
escape (0.0.4)
ethon (0.16.0)
ffi (>= 1.15.0)
ffi (1.15.5)
ffi (1.17.0)
fourflusher (2.3.1)
fuzzy_match (2.0.4)
gh_inspector (1.1.3)
httpclient (2.8.3)
i18n (1.12.0)
concurrent-ruby (~> 1.0)
json (2.6.3)
json (2.7.2)
minitest (5.18.0)
molinillo (0.8.0)
nanaimo (0.3.0)
nap (1.1.0)
netrc (0.11.0)
nkf (0.2.0)
public_suffix (4.0.7)
rexml (3.2.5)
rexml (3.2.9)
strscan
ruby-macho (2.5.1)
typhoeus (1.4.0)
strscan (3.1.0)
typhoeus (1.4.1)
ethon (>= 0.9.0)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
xcodeproj (1.22.0)
xcodeproj (1.24.0)
CFPropertyList (>= 2.3.3, < 4.0)
atomos (~> 0.1.3)
claide (>= 1.0.2, < 2.0)
Expand All @@ -89,7 +95,8 @@ PLATFORMS
ruby

DEPENDENCIES
cocoapods (~> 1.11, >= 1.11.2)
activesupport (>= 6.1.7.3, < 7.1.0)
cocoapods (~> 1.13)

RUBY VERSION
ruby 2.7.5p203
Expand Down
3 changes: 3 additions & 0 deletions examples/FabricExample/__tests__/App-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import 'react-native';
import React from 'react';
import App from '../App';

// Note: import explicitly to use the types shipped with jest.
import {it} from '@jest/globals';

// Note: test renderer must be required after react-native.
import renderer from 'react-test-renderer';

Expand Down
1 change: 0 additions & 1 deletion examples/FabricExample/_node-version

This file was deleted.

59 changes: 6 additions & 53 deletions examples/FabricExample/android/app/build.gradle
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
apply plugin: "com.android.application"
apply plugin: "org.jetbrains.kotlin.android"
apply plugin: "com.facebook.react"

import com.android.build.OutputFile
Expand All @@ -13,8 +14,8 @@ react {
// root = file("../")
// The folder where the react-native NPM package is. Default is ../node_modules/react-native
// reactNativeDir = file("../node-modules/react-native")
// The folder where the react-native Codegen package is. Default is ../node_modules/react-native-codegen
// codegenDir = file("../node-modules/react-native-codegen")
// The folder where the react-native Codegen package is. Default is ../node_modules/@react-native/codegen
// codegenDir = file("../node_modules/@react-native/codegen")
// The cli.js file which is the React Native CLI entrypoint. Default is ../node_modules/react-native/cli.js
// cliFile = file("../node_modules/react-native/cli.js")

Expand Down Expand Up @@ -52,14 +53,6 @@ react {
// hermesFlags = ["-O", "-output-source-map"]
}

/**
* Set this to true to create four separate APKs instead of one,
* one for each native architecture. This is useful if you don't
* use App Bundles (https://developer.android.com/guide/app-bundle/)
* and want to have separate APKs to upload to the Play Store.
*/
def enableSeparateBuildPerCPUArchitecture = false

/**
* Set this to true to Run Proguard on Release builds to minify the Java bytecode.
*/
Expand All @@ -78,20 +71,11 @@ def enableProguardInReleaseBuilds = false
*/
def jscFlavor = 'org.webkit:android-jsc:+'

/**
* Private function to get the list of Native Architectures you want to build.
* This reads the value from reactNativeArchitectures in your gradle.properties
* file and works together with the --active-arch-only flag of react-native run-android.
*/
def reactNativeArchitectures() {
def value = project.getProperties().get("reactNativeArchitectures")
return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"]
}

android {
ndkVersion rootProject.ext.ndkVersion

compileSdkVersion rootProject.ext.compileSdkVersion
buildToolsVersion rootProject.ext.buildToolsVersion
compileSdk rootProject.ext.compileSdkVersion

namespace "net.video.fabricexample"
defaultConfig {
Expand All @@ -102,14 +86,6 @@ android {
versionName "1.0"
}

splits {
abi {
reset()
enable enableSeparateBuildPerCPUArchitecture
universalApk false // If true, also generate a universal APK
include (*reactNativeArchitectures())
}
}
signingConfigs {
debug {
storeFile file('debug.keystore')
Expand All @@ -130,36 +106,13 @@ android {
proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
}
}

// applicationVariants are e.g. debug, release
applicationVariants.all { variant ->
variant.outputs.each { output ->
// For each separate APK per architecture, set a unique version code as described here:
// https://developer.android.com/studio/build/configure-apk-splits.html
// Example: versionCode 1 will generate 1001 for armeabi-v7a, 1002 for x86, etc.
def versionCodes = ["armeabi-v7a": 1, "x86": 2, "arm64-v8a": 3, "x86_64": 4]
def abi = output.getFilter(OutputFile.ABI)
if (abi != null) { // null for the universal-debug, universal-release variants
output.versionCodeOverride =
defaultConfig.versionCode * 1000 + versionCodes.get(abi)
}

}
}
}

dependencies {
// The version of react-native is set by the React Native Gradle Plugin
implementation("com.facebook.react:react-android")
implementation("com.facebook.react:flipper-integration")

implementation("androidx.swiperefreshlayout:swiperefreshlayout:1.0.0")

debugImplementation("com.facebook.flipper:flipper:${FLIPPER_VERSION}")
debugImplementation("com.facebook.flipper:flipper-network-plugin:${FLIPPER_VERSION}") {
exclude group:'com.squareup.okhttp3', module:'okhttp'
}

debugImplementation("com.facebook.flipper:flipper-fresco-plugin:${FLIPPER_VERSION}")
if (hermesEnabled.toBoolean()) {
implementation("com.facebook.react:hermes-android")
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,5 @@
<application
android:usesCleartextTraffic="true"
tools:targetApi="28"
tools:ignore="GoogleAppIndexingWarning">
<activity android:name="com.facebook.react.devsupport.DevSettingsActivity" android:exported="false" />
</application>
tools:ignore="GoogleAppIndexingWarning"/>
</manifest>

This file was deleted.

Loading
Loading