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

[iOS][Android][face-detector][ads-admob][firebase-core][firebase-analytics] Upgrade native libraries #12125

Conversation

bbarthec
Copy link
Contributor

@bbarthec bbarthec commented Mar 5, 2021

Why

We have to upgrade ads-admob, because of new Google's polices (more info here) and this upgrade triggers other cocoapods dependencies updates.

Major update refers to expo-face-detector, because it has to be upgraded from old Firebase/MLVision library to new GoogleMLKit/FaceDetection library.

supersedes #12126
closes https://linear.app/expo/issue/ENG-177/upgrade-google-admob
closes #11941
closes #11934

How

iOS

Upgrades native libraries:

  • upgraded firebase_sdk_version from 6.14.0 to 7.7.0 in firebase-* modules - followed changelog and nothing important for us changed
  • upgraded Google-Mobile-Ads-SDK from 7.55.1 to 7.69.0 in ads-admob module - nothing important changed according to changelog
  • migrated from Firebase/MLVision to GoogleMLKit/FaceDetection@2.1.0 in face-detector module - followed migration plan & changes

Refrained from upgrading Google-Mobile-Ads-SDK to the newest version 8.1.0 as it seems too big for this PR.

Had to backport library upgrades to older SDKs as of cocoapods not accepting multiple versions of single library.

Android

To achieve ads-admob parity on both platform and because of https://developers.google.com/admob/android/rel-notes#19.3.0 (Android 11 is supported since 19.4.0) I had to update Android native dependency as well. I has to upgrade this library code to achieve feature parity on both platforms.
Additionally I've adjusted event names emitted by this module (now these are much more simplified compared with the previous ones).

Test Plan

  • No detectable regression observed.
  • Compiled Expo GO and launched both ncl and test-suite with UNVERSIONED SDK and checked affected modules - every screen/test works.
  • Failed to compile bare-expo, but the problem seems to live in Flipper-related code, so it's out of scope of this PR and has to be fixed separately after this PR lands.

@bbarthec bbarthec requested review from ccheever, cruzach and brentvatne and removed request for IjzerenHein, EvanBacon and ccheever March 5, 2021 19:28
@bbarthec bbarthec force-pushed the @bbarthec/face-detector/ios/upgrade-firebase-to-google-ml-kit branch from 7c81b62 to 8f9571f Compare March 5, 2021 19:39
@brentvatne brentvatne force-pushed the @bbarthec/face-detector/ios/upgrade-firebase-to-google-ml-kit branch from 8f9571f to b73c4b2 Compare March 7, 2021 01:33
@brentvatne
Copy link
Member

@bbarthec - looks like we need to update changelog for ci to pass here

…coapods] Upgrade native libraries & temporarily ignore verioned ABIs

- upgrade firebase_sdk_version to 7.7.0
- upgrade Google-Mobile-Ads-SDK to 7.69.0
- migrate from Firebase/MLVision to GoogleMLKit/FaceDetection
- fixed warning for ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES
- removed ABI38_0_0 yoga reference from OTHER_SWIFT_FLAGS
- update module code by replacing Firebase references with GoogleMLKit ones
…vent names

- updated com.google.android.gms:play-services-ads from 17.x.x to 19.4.0
- updated event names to resemble new RewardedAd lifecycle
- updated docs for RewardedAd
@bbarthec bbarthec force-pushed the @bbarthec/face-detector/ios/upgrade-firebase-to-google-ml-kit branch from b73c4b2 to 09b80d8 Compare March 8, 2021 18:10
@bbarthec bbarthec changed the title [iOS][face-detector][ads-admob][firebase-core][firebase-analytics] Upgrade native libraries [iOS][Android][face-detector][ads-admob][firebase-core][firebase-analytics] Upgrade native libraries Mar 8, 2021
@bbarthec bbarthec requested a review from byCedric March 8, 2021 18:27
@cristian-milea
Copy link

cristian-milea commented Apr 23, 2021

@bbarthec , Thanks for upgrading AdMob SDK for expo. Do we need to change anything else do make it work? I changed the event names but still get Error Domain=com.google.admob Code=1 \"Request Error: No ad to show.\" UserInfo={NSLocalizedDescription=Request Error: No ad to show.

-- using expo 41

@cruzach
Copy link
Contributor

cruzach commented Apr 26, 2021

@cristian-milea you shouldn't need to make any other changes. That error comes from Admob itself, so you should search for that error message in their documentation and see what causes they point to. I have seen that error when the ad unit ID is just too new (it takes some time)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants