enable new architecture for android PR,Release builds only #20950
+44
−27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
23cd279
to
66a694b
Compare
flexsurfer
approved these changes
Aug 2, 2024
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
66a694b
to
2bc249f
Compare
7 tasks
Jenkins BuildsClick to see older builds (120)
|
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
2 times, most recently
from
6a054ad
to
136001c
Compare
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
136001c
to
cbc193d
Compare
siddarthkay
changed the title
jakubgs
approved these changes
Aug 2, 2024
|
@status-im/mobile-qa : This PR enables new architecture in react-native for android builds. It was already enabled for iOS right after montenegro offsite. We don't expect it to cause any platform specific issues on Android but never the less a smoke test should be good enough ( only when you have the time, I know you're busy with release testing ). |
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
cbc193d
to
489a3d8
Compare
This was referenced Aug 4, 2024
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
2 times, most recently
from
a0ad5c8
to
734b022
Compare
ilmotta
approved these changes
Aug 6, 2024
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
734b022
to
c0755ac
Compare
vkjr
approved these changes
Aug 6, 2024
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
c0755ac
to
1e0a62b
Compare
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
2 times, most recently
from
fe1e3bc
to
f400776
Compare
|
@mariia-skrypnyk : nothing new on this one, I'll ping you when I have more news on this one. |
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
baa9627
to
de25cbf
Compare
|
In debug mode I did notice a bunch of abort signals originating from reanimated library : I'll try to see if they're fixed in a newer version of reanimated or try to temporarily patch them out. |
|
Finally I'm able to reproduce this issue in debug variant. This gives us good hints that the bug indeed originates due to hole view library and at his component cc @status-im/mobile : can you please find some alternative approach or a temp fix for this issue? |
|
The root cause could probably be float to int conversion here -> https://github.com/ibitcy/react-native-hole-view/blob/b4b60b508cb43ce739eeb3f2abede491d21155af/android/src/main/java/com/ibitcy/react_native_hole_view/RNHoleView.kt#L80-L177 A simple fix could be to try-catch the exceptions first and if possible fix it or maybe even mute it. |
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
3 times, most recently
from
5b51e12
to
dd62fb0
Compare
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
3 times, most recently
from
f426fb4
to
c13c955
Compare
|
The number @status-im/mobile-qa : this fixes issue 1 |
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
c13c955
to
7661360
Compare
Hey @siddarthkay! Unfortunately ISSUE 1 is still reproducible for me in the latest PR build Status-debug-logs - 2024-09-16T123354.319.zip telegram-cloud-document-2-5422360167834475167.mp4 |
|
Hi @pavloburykh : Thanks for checking! I'll try to reproduce again and fix 🙏🏻 |
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
7661360
to
7fb37e4
Compare
ISSUE 3 Bottom sheets are closed by itself after being opened within 'Profiles on device' screenSteps:
Actual result: Bottom sheets are closed by itself after being opened telegram-cloud-document-2-5422360167834475199.mp4 |
ISSUE 4 Broken layout of Create Profile screen when creating additional profile from 'Profiles on device' screenSteps:
Actual result:
telegram-cloud-document-2-5422360167834475207.mp4 |
|
Hey @siddarthkay ! I have moved PR moved Contributor column. Please feel free to ping me and move PR back to In Testing once issues are resolved. Thank you. |
siddarthkay
force-pushed
the
try-to-enable-new-architecture-for-android
branch
from
609ce50
to
e8dd70b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In this PR we use
unstable_reactLegacyComponentNamesto bridge components that are not compatible with Fabric.We also enable
hermesandnew architecturefor Android but only for PR and Release builds.Review notes
Pls test local dev environment with this branch
Testing notes
Pls test everything
Platforms
status: ready
fixes: #18138