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(ci): Expo #12244

Open
wants to merge 59 commits into
base: main
Choose a base branch
from
Open

feat(ci): Expo #12244

wants to merge 59 commits into from

Conversation

andrepimenta
Copy link
Member

@andrepimenta andrepimenta commented Nov 8, 2024

Description

Problem

Right now, setting up the environment for running the MetaMask Mobile app is a major pain and a point of huge friction for developers looking to build a feature on the app. It takes hours for the first setup because of the so many tools needed to be installed which are prone to errors and problems (many times difficult to understand and debug) which causes devs, especially devs with less experience on mobile, to give up on developing on the MM Mobile app. This problem is also taking time from the Mobile team that has to help devs to debug issues and setup the app. Furthermore, every time a developer wants to run the Mobile app (after first setup), they need to compile the native part of the app which takes several minutes and is also prone to many issues with incompatibilities with the system.

Solution implemented

The work on this PR will allow major improvements in contributor experience in the MetaMask Mobile app. By integrating expo development builds, there is no more need for compiling the native part of the app meaning, devs can just install an APK on their device or simulator and load the javascript bundle. This makes the first setup and consequently runs several times faster.
The experience will be much similar to how a webapp works, but in this case the browser will be the development build, bundling and loading the javascript files is exactly the same. This means that for web/extension devs, the process will be much more familiar, and not only that but much much faster to setup and easier to understand.

Improvements in time to setup/run

Time for first setup goes from 3-4 hours to 1-2 minutes
Time for running the app goes from 4-5 mins to 1 min (and 8s for subsequently runs)

Why so much faster?

Before, the setup included everything here: https://github.com/MetaMask/metamask-mobile/blob/main/docs/readme/environment.md. Included installing several tools and apps such as xcode and android studio. This was always followed by errors and problems caused by the tools or the computer environment that were hard to understand and debug.
Now, devs can just download and install an APK and run one command yarn expo:start and that is it.

Steps I took to integrate expo

Commit 1 c36edda

  • First step is to install expo modules that allows expo compatible modules to be installed and used: npx install-expo-modules@latest
  • [TODO] This increases iOS min version to 13.0, we should check if we're ok with this change
  • npx expo run:ios doesn’t run out of the box, it has to be integrated with our build.sh in order to build or dependencies. This is because it tries to run pod install but we actually install pods with bundle execute pod install.
  • Replaced sourcecode.c.objc with sourcecode.cpp.objcpp as explained in Xcode build fails Property 'reactDelegate' not found on object of type 'AppDelegate *' expo/expo#20850 (comment) because of property 'reactDelegate' not found on object of type 'AppDelegate *' error.

Commit 2 abcb412

  • Minimizer fixes (had to replace #import <UIKit/UIKit.h> with #import <objc/runtime.h> in RCTMinimizer.m

Commit 3 b181a1f

  • Had to install npx expo install expo-build-properties to include the maven repos to fix an Android error. The expo build properties will be useful to implement (CNG)[https://docs.expo.dev/workflow/continuous-native-generation/]

Commit 4 f586844

  • The magic really starts to happen once "expo-dev-client": "~2.4.13"is installed
  • Ran into a bunch of errors, one of which I had to update "react-native-reanimated": "~3.3.0"
  • iOS stopped working with no error info, took me almost a week to pinpoint the error as it could be anywhere. Had to create a new react native app, install expo and compare every single file. Finally narrowed down the problem to return [super application:application didFinishLaunchingWithOptions:launchOptions];
  • Still the expo deeplinks were not working

Commit 5 820321f

  • Fixed the expo deeplinks by adding [EXDevLauncherController.sharedInstance onDeepLink:url options:options]

Commit 6 d482e09

  • Added build. commands

Commit 7 - 20

  • Fixing unit testing and E2E: Expo doesn't support detox out of the box as the initial screen will now be the Expo screen which is something detox is not expecting, so I had to implement a workaround by launching the app and skipping the Expo screen by calling a deeplink that makes the bundle start loading. Also included other minor fixes.

Commit 21 and on

  • Implementing deploying Dev Builds on our CI/CD

✅ Android working
✅ iOS working

TODOs

  • iOS min version got increased to 13.0, we should check if we're ok with this change
  • Tsconfig got updated automatically, we should check if we agree
  • Test QA builds
  • Figure out where to host the development builds
  • Update the repo README

Acceptance criteria

  • All app functionality works as before (signing in, transactions, browser, etc.)
  • Deeplinks work as before
  • Updating from a previous build works without lost data
  • All dev tools (e.g. Flipper) still work
  • Vault backup still works
  • Icons still work (expo introduced a compatibility layer around our icon library)

Related issues

Fixes: #11548

Manual testing steps

These are subject to change!

  1. If you have the development build installed, you can just do yarn watch and scan the QR code
    OR
  2. yarn setup:expo
  3. yarn start:android or yarn start:ios

Screenshots/Recordings

Before

Video so long I don't want to put it here 😂

After

expo.metamask.recording.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link

socket-security bot commented Nov 8, 2024

Report is too large to display inline.
View full report↗︎

Next steps

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@andrepimenta andrepimenta added team-mobile-platform Run Smoke E2E Triggers smoke e2e on Bitrise labels Nov 11, 2024
Copy link
Contributor

github-actions bot commented Nov 11, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 97dfdba
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/2de0f5af-9a6c-44d7-8f45-92d37b84b998

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@andrepimenta andrepimenta added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Nov 12, 2024
Copy link
Contributor

github-actions bot commented Nov 12, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 7f8a041
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d3245996-265a-4c42-88ee-79eb36c24c5b

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@andrepimenta andrepimenta added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Nov 12, 2024
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@andrepimenta andrepimenta marked this pull request as ready for review November 26, 2024 17:58
@andrepimenta andrepimenta requested review from a team as code owners November 26, 2024 17:58
@gauthierpetetin gauthierpetetin added the team-tiger Tiger team (for tech debt reduction + performance improvements) label Nov 27, 2024
Copy link

sonarcloud bot commented Nov 27, 2024

@andrepimenta andrepimenta requested a review from a team as a code owner November 27, 2024 23:17
Copy link

Report too large to display inline

View full report↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform team-tiger Tiger team (for tech debt reduction + performance improvements)
Projects
Status: Needs dev review
Development

Successfully merging this pull request may close these issues.

Research: Explore building and hosting native binary using Expo EAS build
3 participants