-
Notifications
You must be signed in to change notification settings - Fork 989
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
Integrate Firebase for Android and iOS #21983
base: develop
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (83)
|
e49478b
to
ca8441e
Compare
5307db3
to
be2a8c2
Compare
4453b01
to
f58e51a
Compare
fb74c25
to
6c73939
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass self-review comments 📑
I've attempted to review some the confusing portions of the code, but there are still places involving the logic that I have not commented yet. I'll return for another pass on these areas soon.
.env.jenkins
Outdated
FLAG_WALLET_CONNECT_ENABLED=1 | ||
API_LOGGING_ENABLED=1 | ||
GOOGLE_FREE=0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're adding a environment variable for configuring the build to avoid bundling Google dependencies for Firebase and Google Play Services.
build-fdroid: ##@build Build release for F-Droid | ||
@scripts/google-free.sh | ||
@scripts/build-android.sh | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're adding an additional script step for preparing an Android build for FDroid.
@@ -1 +1 @@ | |||
2.32.0 | |||
2.32.21983 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version change is temporary and should be removed before merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version change is used for creating a testable release builds via the PR number, and as a best practice we change the app version number so we don't conflicts with upcoming releases in the build pipeline.
android/app/build.gradle
Outdated
/** | ||
* Exclude google library so we can publish to external android stores | ||
*/ | ||
def googleFree = project.env.get("GOOGLE_FREE", 0) == '1' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're accessing the environment variable from the Android build system (Gradle), and we use this flag to configure the build's dependencies.
android/app/build.gradle
Outdated
if (googleFree) { | ||
implementation(project(':react-native-firebase_app')) { | ||
exclude group: 'com.google.firebase' | ||
exclude group: 'com.google.gms' | ||
} | ||
|
||
implementation(project(':react-native-firebase_messaging')) { | ||
exclude group: 'com.google.firebase' | ||
exclude group: 'com.google.gms' | ||
} | ||
} else { | ||
implementation 'com.google.firebase:firebase-messaging:24.1.0' | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're attempting to exclude Firebase and Google dependencies from the app when attempting to build a FDroid build.
scripts/build-android.sh
Outdated
# Used by Clojure at compile time for remove import of firebase for fdroid release | ||
if [[ -n "${READER_FEATURES}" ]]; then | ||
append_env_export 'READER_FEATURES' | ||
fi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're integrating with ShadowCLJS during the Android build to forward the GOOGLE_FREE
flag as a reader conditional. This allows us to conditionally include Firebase dependencies in JavaScript bundle.
#!/usr/bin/env bash | ||
|
||
set -e | ||
|
||
# Used by Clojure to condition code and Gradle to make dependencies optional | ||
sed -i -e '$aGOOGLE_FREE=1' .env.release | ||
|
||
# remove firebase (uses google dependencies) and google-services.json for the fdroid-build | ||
yarn remove @react-native-firebase/app | ||
yarn remove @react-native-firebase/messaging | ||
rm android/app/google-services.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're defining a cleanup script for the FDroid builds that removes Firebase dependencies and related files before continuing with building Android app for FDroid.
shadow-cljs.edn
Outdated
:reader-features #{:mobile | ||
#shadow/env ["READER_FEATURES" :as :keyword :default | ||
:cljs]}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're configuring ShadowCLJS mobile build with the extra reader conditional flags.
src/status_im/feature_flags.cljs
Outdated
::profile-pictures-visibility (enabled-in-env? :FLAG_PROFILE_PICTURES_VISIBILITY_ENABLED) | ||
::settings.import-all-keypairs (enabled-in-env? :FLAG_WALLET_SETTINGS_IMPORT_ALL_KEYPAIRS) | ||
::settings.remote-notifications (enabled-in-env? :FLAG_NOTIFICATION_SETTINGS_REMOTE_NOTIFICATIONS) | ||
::wallet.add-watched-address (enabled-in-env? :FLAG_ADD_WATCHED_ADDRESS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're adding the feature flag for the remote notifications toggle.
src/react_native/firebase.cljc
Outdated
(ns react-native.firebase | ||
(:require | ||
#?@(:google-free | ||
[[react-native.firebase.firebase-fdroid]] | ||
:cljs | ||
[[react-native.firebase.firebase-google]]))) | ||
|
||
#?(:google-free | ||
[(def request-remote-token react-native.firebase.firebase-fdroid/request-remote-token)] | ||
:cljs | ||
[(def request-remote-token react-native.firebase.firebase-google/request-remote-token)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we're using the ShadowCLJS reader conditionals to avoid requiring the Firebase bindings and dependencies when building for FDroid.
d455ee1
to
9173de4
Compare
…when enabling remote notifications
…istration for ios
9173de4
to
a13ab31
Compare
resolves #22090
resolves #22174
Summary
Review notes
make clean
before rebuilding the app.Testing notes
Platforms
Areas that may be impacted
Functional
Steps to test
WIP
status: ready