status-mobile icon indicating copy to clipboard operation
status-mobile copied to clipboard

Video assets which are iOS only should not end up in Android bundle

Open siddarthkay opened this issue 1 year ago • 6 comments

Problem

The video asset files used for onboarding animation are ~ 30 MB and they end up in the Android APK. The component which uses those assets is not rendered in Android but maybe because its a runtime check those assets make their way into the bundle anyway.

We should find a way to make sure those video assets are included in iOS only so that we can shave off 30 MB from the Android APK.

Related links :

  • https://reactnative.dev/docs/images#images-from-hybrid-apps-resources
  • https://reactnative.dev/docs/platform-specific-code#platform-specific-extensions

siddarthkay avatar May 11 '24 16:05 siddarthkay

Checking my initial assumption that these mp4 files make their way into the bundle due to a runtime check. I added stricter platform checks to only reference the video component which requires the mp4s only on iOS platform like this :


diff --git a/src/status_im/common/parallax/blacklist.cljs b/src/status_im/common/parallax/blacklist.cljs
index 3e9a1da5a..0b74d2586 100644
--- a/src/status_im/common/parallax/blacklist.cljs
+++ b/src/status_im/common/parallax/blacklist.cljs
@@ -1,6 +1,7 @@
 (ns status-im.common.parallax.blacklist
   (:require
-    [native-module.core :as native-module]))
+    [native-module.core :as native-module]
+    [react-native.platform :as platform]))

 (def ^:private device-id (:device-id (native-module/get-device-model-info)))

@@ -12,7 +13,4 @@

 (def ^:private minimum-device-code 11)

-(def blacklisted?
-  (-> device-id
-      get-model-code
-      (< minimum-device-code)))
+(def blacklisted? platform/android?)
diff --git a/src/status_im/contexts/onboarding/enable_biometrics/view.cljs b/src/status_im/contexts/onboarding/enable_biometrics/view.cljs
index a8f51f638..d8b0f746e 100644
--- a/src/status_im/contexts/onboarding/enable_biometrics/view.cljs
+++ b/src/status_im/contexts/onboarding/enable_biometrics/view.cljs
@@ -10,6 +10,7 @@
     [status-im.contexts.onboarding.enable-biometrics.style :as style]
     [status-im.navigation.state :as state]
     [utils.i18n :as i18n]
+    [react-native.platform :as platform]
     [utils.re-frame :as rf]))


@@ -55,9 +56,10 @@
     [rn/view
      {:position :absolute
       :top      12}
-     [parallax/video
-      {:layers  (:biometrics resources/parallax-video)
-       :stretch stretch}]]))
+     (when platform/ios?
+       [parallax/video
+        {:layers  (:biometrics resources/parallax-video)
+         :stretch stretch}])]))

 (defn enable-biometrics-simple
   []
diff --git a/src/status_im/contexts/onboarding/enable_notifications/view.cljs b/src/status_im/contexts/onboarding/enable_notifications/view.cljs
index f423944f9..5e6db2bae 100644
--- a/src/status_im/contexts/onboarding/enable_notifications/view.cljs
+++ b/src/status_im/contexts/onboarding/enable_notifications/view.cljs
@@ -10,6 +10,7 @@
     [status-im.contexts.onboarding.enable-notifications.style :as style]
     [status-im.contexts.shell.jump-to.utils :as shell.utils]
     [taoensso.timbre :as log]
+    [react-native.platform :as platform]
     [utils.i18n :as i18n]
     [utils.re-frame :as rf]))

@@ -58,9 +59,10 @@
 (defn enable-notifications-parallax
   []
   (let [stretch (if rn/small-screen? -40 -25)]
-    [parallax/video
-     {:layers  (:notifications resources/parallax-video)
-      :stretch stretch}]))
+    (when platform/ios?
+      [parallax/video
+         {:layers  (:notifications resources/parallax-video)
+          :stretch stretch}])))

 (defn enable-notifications-simple
   []
diff --git a/src/status_im/contexts/onboarding/generating_keys/view.cljs b/src/status_im/contexts/onboarding/generating_keys/view.cljs
index f15a96d68..5b2a6a888 100644
--- a/src/status_im/contexts/onboarding/generating_keys/view.cljs
+++ b/src/status_im/contexts/onboarding/generating_keys/view.cljs
@@ -126,13 +126,14 @@
 (defn parallax-page
   [insets]
   [:<>
-   [parallax/video
-    {:stretch           -20
-     :container-style   {:top  40
-                         :left 20}
-     :layers            (:generate-keys resources/parallax-video)
-     :disable-parallax? true
-     :enable-looping?   false}]
+   (when platform/ios?
+     [parallax/video
+      {:stretch           -20
+       :container-style   {:top  40
+                           :left 20}
+       :layers            (:generate-keys resources/parallax-video)
+       :disable-parallax? true
+       :enable-looping?   false}])
    [:f> f-page-title insets]])

 (defn f-simple-page

However after analysing a release APK from this build I could still see the mp4s as part of the res folder. Which means that as long as there is a js/require in the code that pulls in these resources they will make it to both the android and iOS bundles. ref ->

https://github.com/status-im/status-mobile/blob/f50ca4606f0e92ffc45900e468139242aa44fb3e/src/status_im/common/resources.cljs#L132-L142

siddarthkay avatar May 12 '24 05:05 siddarthkay

One idea which i fiddled with a little bit was to store the video assets in native directories of iOS and Android respectively. The iOS one would have the original video but the Android one would have a fake mp4 file of 4bytes. so when we'd require them on Android we'd get the savings we want since we don't use those UI components which require videos anyways.

This isn't very straightforward since react-native-transparent-video library only accepts a source attribute and doesn't accept uris -> https://github.com/status-im/react-native-transparent-video/blob/main/src/index.tsx#L6-L9 cc @briansztamfater

I did write a patch to include uri as part of props but I did not get the video to show up on iOS so more research is indeed needed in this space to achieve the goal of these heavy video assets not showing up in Android bundle

siddarthkay avatar May 13 '24 09:05 siddarthkay

@siddarthkay I think 30MB just for onboarding animations (regardless of platform) is a bit too much

Maybe we can also try to optimize the size of these files

OmarBasem avatar May 13 '24 11:05 OmarBasem

a bit too much

indeed 30 MB of animations are a lot.

in other news : I managed to get rid of this 30 MB on Android side.

credits to @jakubgs for this recommendation I wrote a gradle task which parses the release bundle and swaps the heavy mp4 files with fake ones.

note : The fake ones would contain the same name as the original one to make sure that the js bundle does not complain of missing assets.

So now the android APK would contain mp4 files which would weight 4 bytes each.

This should land us savings of ~ 30 MB on Android APK.

I'm still testing things out, will send a PR for this today.

siddarthkay avatar May 14 '24 06:05 siddarthkay

This approach should have no impact on iOS which then requires no change on Clojurescript side as well which is nicer considering the goal here is to reduce APK size.

siddarthkay avatar May 14 '24 06:05 siddarthkay

wrote a gradle task which parses the release bundle and swaps the heavy mp4 files with fake ones.

I later realised that this logic should live in one of the phases and it was better if it was executed right after the buildPhase which prepares the APK in our existing nix derivation.

siddarthkay avatar May 14 '24 11:05 siddarthkay