[NoQA] feat: enable bridgeless #48160

WoLewicki · 2024-08-28T10:55:04Z

Details

PR enabling bridgeless mode in the App. This PR can affects the App as a whole, so should require full regression testing.

PRs for introduced patches:

react-native-quick-sqlite: https://github.com/margelo/react-native-quick-sqlite/pull/64
react-native-pager-view: feat: remove usage of currentBridge callstack/react-native-pager-view#881
react-native-firebase: We have a dirty workaround and Meta is already working on a better solution after I reported the problem to them. We won't make a PR with it since it's better to wait for it to be fixed in core. (issue in RN repo: [New Architecture][iOS][Bridgeless] Native module methods not visible in JS facebook/react-native#46779)
lottie-react-native: The changes are already available on rc: feat: add experimental bridgeless support lottie-react-native/lottie-react-native#1160
rnmapbox/maps: fix: events in bridgeless mode rnmapbox/maps#3656
react-native-modal is not maintained from what I know and there are plans for providing our own, faster implementation of the library.
react-native-vision-camera: fix: events in bridgeless mode mrousavy/react-native-vision-camera#3240

Fixed Issues

$ #48163
PROPOSAL:

Tests

Verify that no errors appear in the JS console

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

melvin-bot · 2024-10-04T13:02:27Z

@fedirjh @roryabraham One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

github-actions · 2024-10-04T13:37:50Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/48160/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/48160/index.html

Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/48160/NewExpensify.dmg	https://48160.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

roryabraham · 2024-10-04T16:14:46Z

react-native-keyboard-controller: The code is already there from 1.13.0 so it just needs a bump and we can remove the patch then.

So let's do this before we merge this PR, rather than creating a patch that has to be cleaned up after

roryabraham · 2024-10-04T16:17:38Z

Created #50253

roryabraham · 2024-10-04T16:18:02Z

react-native-firebase: We have a dirty workaround and Meta is already working on a better solution after I reported the problem to them. We won't make a PR with it since it's better to wait for it to be fixed in core.

Can you link to an upstream issue plz? 🙇🏼

WoLewicki · 2024-10-07T11:39:52Z

Can you link to an upstream issue plz? 🙇🏼

Done.

fedirjh · 2024-10-07T14:23:48Z

@WoLewicki On iOS native, I cannot change the profile avatar or select an attachment in composer

CleanShot.2024-10-07.at.15.12.39.mp4

CleanShot.2024-10-07.at.14.56.32.mp4

When creating a distance request, the composer is shinking :

CleanShot.2024-10-07.at.15.09.10.mp4

I got the warning several times when I created a new expenses

fedirjh · 2024-10-07T15:12:39Z

@WoLewicki On android, when sending a new message, the app crashes with this error:

fedirjh · 2024-10-07T15:17:42Z

On Android: When you try to create a new expense, the App crashes with this error, there were other warnings errors as well :

fedirjh · 2024-10-07T15:22:06Z

On Android: When you create a scan expense, the App crashes with these errors:

fedirjh · 2024-10-07T15:43:23Z

When you navigate to troubleshoot page, the app will crash with this error :

WoLewicki · 2024-10-18T13:09:00Z

I wouldn't expect enormous gains, but some at least due to Static View Configs.

mountiny · 2024-10-21T10:42:36Z

I can run it now. Running here https://github.com/Expensify/App/actions/runs/11438433039

WoLewicki · 2024-10-21T11:33:24Z

Idk why but it seems to have failed @mountiny. I just merged the newest main, maybe it will work now?

mountiny · 2024-10-22T03:32:41Z

https://github.com/Expensify/App/actions/runs/11452730936

hannojg · 2024-10-22T12:46:06Z

Hm seems like the action to run the e2e tests for a PR is broken. We'll take a look!

WoLewicki · 2024-10-22T13:21:13Z

I merged newest main, is it ready now?

kirillzyusko · 2024-10-22T13:51:35Z

I merged newest main, is it ready now?

@WoLewicki yes, tests should pass now (well, apk-manipulation stages shouldn't fail). However I see that your PR contains "merge" commits. In #48251 I re-worked pipeline, where baseline is taken as last merge commit (and at this point of time I'm not sure which merge commit will be taken in your PR - will it compare with main or with one of merge commits in PR). I think it's just better to run e2e pipeline and check that 👀

Additionally (just want to warn in advance) I think tests will not pass, because they are currently broken on main as well (unfortunately). I'm currently working on this PR: #51248
It works, but it's draft changes and I want to improve some aspects 🙂

If your PR can wait few days, then it would be simply amazing. Alternatively I can run tests locally to check performance benefits (since e2e pipeline is not fully working at the moment).

WoLewicki · 2024-10-22T13:58:41Z

I think having any feedback from this PR regarding performance is valuable, so even running it locally could be enough for now.

If your PR can wait few days, then it would be simply amazing

I'd merge this PR nonetheless since it is the way forward either way and it would help me with pushing RN 0.76 and expo update. But I don't have a strong opinion on that tbh.

kirillzyusko · 2024-10-22T14:06:43Z

Okay @WoLewicki , give me ~1.5hr and I'll post my results here 😊

kirillzyusko · 2024-10-22T15:57:30Z

Okay, I can not run tests locally, because I'm getting:

2024-10-22 17:53:35.734  3133-3375  AndroidRuntime          com.expensify.chat.e2edelta          E  FATAL EXCEPTION: mqt_native_modules (Ask Gemini)
                                                                                                    Process: com.expensify.chat.e2edelta, PID: 3133
                                                                                                    com.facebook.react.common.JavascriptException: Error: Exception in HostFunction: Could not enqueue microtask because they are disabled in this runtime, js engine: hermes, stack:
                                                                                                    setImmediate@1:527723
                                                                                                    setDriver@1:1660584
                                                                                                    LocalForage@1:1659118
                                                                                                    anonymous@1:1640327
                                                                                                    s@1:1635136
                                                                                                    e@1:1634879
                                                                                                    anonymous@1:1634781
                                                                                                    anonymous@1:1634629
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:1634119
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:1606115
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:1603178
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:1402528
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:206788
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:206603
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193495
                                                                                                    metroRequire@1:192777
                                                                                                    anonymous@1:204066
                                                                                                    loadModuleImplementation@1:194267
                                                                                                    guardedLoadModule@1:193452
                                                                                                    metroRequire@1:192777
                                                                                                    global@1:191704
                                                                                                    
                                                                                                    	at com.facebook.react.modules.core.ExceptionsManagerModule.reportException(SourceFile:76)
                                                                                                    	at com.facebook.jni.NativeRunnable.run(Native Method)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:959)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:100)
                                                                                                    	at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(SourceFile:1)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:232)
                                                                                                    	at android.os.Looper.loop(Looper.java:317)
                                                                                                    	at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(SourceFile:38)
                                                                                                    	at java.lang.Thread.run(Thread.java:1012)

I don't know why it happens 🤷‍♂️ I think we can proceed with merging and in a meantime I will try to fix e2e pipeline to make sure we can run tests independently to a local environment 👀

For transparency: builds from main also fail with the same error. I think it's a problem with my local setup 🤔

kirillzyusko · 2024-10-23T17:20:52Z

After merging these 3 PRs we can run test against the PR:

Highly desirable to merge #51182 as well, but it can be optional

mountiny

Lets give this one a go

OSBotify · 2024-10-23T17:33:24Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2024-10-25T02:08:08Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.0.54-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

github-actions · 2024-10-29T09:13:48Z

🚀 Deployed to production by https://github.com/Beamanator in version: 9.0.54-11 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	skipped 🚫
🍎🔄 iOS HybridApp 🍎🔄	skipped 🚫

github-actions · 2024-10-29T11:38:05Z

🚀 Deployed to production by https://github.com/Beamanator in version: 9.0.54-11 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	skipped 🚫
🍎🔄 iOS HybridApp 🍎🔄	skipped 🚫

feat: enable bridgeless

e7c526e

mountiny added the Ready To Build label Aug 28, 2024

mountiny assigned WoLewicki Aug 28, 2024

mountiny self-requested a review August 28, 2024 11:39

This comment has been minimized.

Sign in to view

j-piasecki and others added 6 commits September 20, 2024 13:39

Merge branch 'main' into @wolewicki/enable-bridgeless

f35a62d

Merge branch 'main' into @wolewicki/enable-bridgeless

972716b

fix: make ios work properly

23984a6

fix: make bridgeless work on Android

815041f

chore: revert rnconfig patch from this pr

ce7ce2c

Merge branch 'main' into @wolewicki/enable-bridgeless

5d70325

WoLewicki marked this pull request as ready for review October 4, 2024 13:02

WoLewicki requested a review from a team as a code owner October 4, 2024 13:02

melvin-bot bot requested review from fedirjh and roryabraham and removed request for a team October 4, 2024 13:02

roryabraham mentioned this pull request Oct 4, 2024

[HOLD for payment 2024-10-16] Bump react-native-keyboard-controller #50253

Closed

WoLewicki added 2 commits October 7, 2024 14:07

Merge branch 'main' into @wolewicki/enable-bridgeless

e1157c6

chore: merge current main

effd960

Merge branch 'main' into @wolewicki/enable-bridgeless

509a8d7

WoLewicki added 2 commits October 22, 2024 12:46

Merge branch 'main' into @wolewicki/enable-bridgeless

58fb07b

fix: update patch

475a623

Merge branch 'main' into @wolewicki/enable-bridgeless

6a33b0e

fix: prepare bootsplash for hybrid

2ee388f

Merge branch 'main' into @wolewicki/enable-bridgeless

595e6f1

kirillzyusko mentioned this pull request Oct 23, 2024

[NoQA] e2e: properly detect baseline commit (when running from PR) #51351

Merged

48 tasks

mountiny changed the title ~~feat: enable bridgeless~~ [NoQA] feat: enable bridgeless Oct 23, 2024

mountiny approved these changes Oct 23, 2024

View reviewed changes

mountiny merged commit 3ddc1dd into Expensify:main Oct 23, 2024
10 checks passed

github-actions bot mentioned this pull request Oct 25, 2024

Deploy Checklist: New Expensify 2024-10-25 #51448

Closed

49 tasks

kirillzyusko mentioned this pull request Oct 25, 2024

Latest android release builds are instantly crashing due to JS errors #51454

Closed

WoLewicki mentioned this pull request Oct 25, 2024

[CP Staging] fix: revert bridgeless on Android #51489

Merged

50 tasks

This was referenced Oct 31, 2024

[HOLD for payment 2024-11-13] [$250] iOS - Search - Search input field does not auto focus #51482

Closed

Fix - Chat - Compose box stays in the middle when reduced with a several lines message #50520

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NoQA] feat: enable bridgeless #48160

[NoQA] feat: enable bridgeless #48160

WoLewicki commented Aug 28, 2024 •

edited

Loading

This comment has been minimized.

melvin-bot bot commented Oct 4, 2024

github-actions bot commented Oct 4, 2024

roryabraham commented Oct 4, 2024 •

edited

Loading

roryabraham commented Oct 4, 2024

roryabraham commented Oct 4, 2024

WoLewicki commented Oct 7, 2024

fedirjh commented Oct 7, 2024

fedirjh commented Oct 7, 2024

fedirjh commented Oct 7, 2024 •

edited

Loading

fedirjh commented Oct 7, 2024

fedirjh commented Oct 7, 2024

WoLewicki commented Oct 18, 2024

mountiny commented Oct 21, 2024

WoLewicki commented Oct 21, 2024

mountiny commented Oct 22, 2024

hannojg commented Oct 22, 2024

WoLewicki commented Oct 22, 2024

kirillzyusko commented Oct 22, 2024

WoLewicki commented Oct 22, 2024

kirillzyusko commented Oct 22, 2024

kirillzyusko commented Oct 22, 2024 •

edited

Loading

kirillzyusko commented Oct 23, 2024

mountiny left a comment

OSBotify commented Oct 23, 2024

github-actions bot commented Oct 25, 2024

github-actions bot commented Oct 29, 2024

github-actions bot commented Oct 29, 2024

[NoQA] feat: enable bridgeless #48160

[NoQA] feat: enable bridgeless #48160

Conversation

WoLewicki commented Aug 28, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

This comment has been minimized.

melvin-bot bot commented Oct 4, 2024

github-actions bot commented Oct 4, 2024

roryabraham commented Oct 4, 2024 • edited Loading

roryabraham commented Oct 4, 2024

roryabraham commented Oct 4, 2024

WoLewicki commented Oct 7, 2024

fedirjh commented Oct 7, 2024

fedirjh commented Oct 7, 2024

fedirjh commented Oct 7, 2024 • edited Loading

fedirjh commented Oct 7, 2024

fedirjh commented Oct 7, 2024

WoLewicki commented Oct 18, 2024

mountiny commented Oct 21, 2024

WoLewicki commented Oct 21, 2024

mountiny commented Oct 22, 2024

hannojg commented Oct 22, 2024

WoLewicki commented Oct 22, 2024

kirillzyusko commented Oct 22, 2024

WoLewicki commented Oct 22, 2024

kirillzyusko commented Oct 22, 2024

kirillzyusko commented Oct 22, 2024 • edited Loading

kirillzyusko commented Oct 23, 2024

mountiny left a comment

Choose a reason for hiding this comment

OSBotify commented Oct 23, 2024

github-actions bot commented Oct 25, 2024

github-actions bot commented Oct 29, 2024

github-actions bot commented Oct 29, 2024

WoLewicki commented Aug 28, 2024 •

edited

Loading

roryabraham commented Oct 4, 2024 •

edited

Loading

fedirjh commented Oct 7, 2024 •

edited

Loading

kirillzyusko commented Oct 22, 2024 •

edited

Loading