New architecture support #550

j-piasecki · 2022-08-04T15:03:26Z

Description

This PR adds Fabric integration to the FlashList. Here are some more important highlights:

Android:
- The events on Fabric need to be sent by the FabricUIManager so I made an Extension file for Fabric and for Paper (the correct one is added to the source set in build.gradle)
iOS:
- I've merged CellContainer with CellContainerComponentView as keeping a CellContainer in every CellContainerComponentView didn't look like a very good idea performance-wise
- In the case of AutoLayoutView, I stayed with wrapping it with the Fabric Component (AutoLayoutViewComponentView). I didn't want to accidentally break the functionality converting in to obj-c. It may be possible that they can be merged via inheritance but I'm not that knowledgeable in interop between swift and obj-c and couldn't get it to work
- Event dispatchers on Fabric are auto-generated so I added a field to AutoLayoutView containing a closure, which in turn is set by the AutoLayoutViewComponentView. This way the code in Swift has access to the generated obj-c++ method.
Fixture app
- I've added patches for safe-area-context and screens to make sure they work with RN 0.72 on the new architecture
- New architecture is now enabled by default in the app, which may not be desired. Let me know how you would like it handled.

Reviewers’ hat-rack 🎩

[ ]

Screenshots or videos (if needed)

Checklist

I have added a changelog entry following the Keep a Changelog guidelines.

hirbod · 2022-08-14T15:50:52Z

So Fabric is actually making the performance worse? I was always hoping that Fabric is here to change that :D

fortmarek

Thanks @j-piasecki!

Do we have an idea why Fabric could be slower?

fortmarek · 2022-08-18T07:36:15Z

RNFlashList.podspec

+    s.dependency "RCTTypeSafety"
+    s.dependency "ReactCommon/turbomodule/core"
+  else
+    s.platforms = { :ios => "9.0", :tvos => "9.0" }


This line has a different syntax than this. Also, given React Native only supports 12.4 as of now, can't we just target 11.0 versions on both Paper and Fabric?

I updated it, so that it matches the main branch.

fortmarek · 2022-08-18T07:38:12Z

android/src/main/kotlin/com/shopify/reactnative/flash_list/AutoLayoutViewManager.kt

-    }
+    override fun getExportedCustomDirectEventTypeConstants() = mutableMapOf(
+            "onBlankAreaEvent" to mutableMapOf("registrationName" to "onBlankAreaEvent"),
+            "topOnBlankAreaEvent" to mutableMapOf("registrationName" to "onBlankAreaEvent"),


Why do we need this? topOnBlankAreaEvent does not exist, afaict?

Without it I was getting Error: Unsupported top level event type "topOnBlankAreaEvent" dispatched error. It's solved similarly in Screens and Gesture Handler.

fortmarek · 2022-08-18T07:39:29Z

android/src/paper/java/com/facebook/react/viewmanagers/AutoLayoutViewManagerDelegate.java

@@ -0,0 +1,46 @@
+/**
+* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).


is the generator always creating code for paper as well? 🤔

No, it's not. I copied those files without modifying them. Do you think those comments should be removed?

fortmarek · 2022-08-18T07:40:34Z

fabricfixture/.buckconfig

@@ -0,0 +1,6 @@
+


Is there a reason for having buck-related files and code inside the fixture app?

Not really, the fabricfixture was the result of react-native init. I found some problems with the app in the meantime so I set it up from scratch based on the fixture app.

fortmarek · 2022-08-18T07:45:07Z

fabricfixture/.bundle/config

@@ -0,0 +1,2 @@
+BUNDLE_PATH: "vendor/bundle"


Since we're creating a standalone app for the Fabric fixture – has the maintenance burden suffered on your side?

Have you considered looking at the react-native-test-app from Microsoft, which is now e.g. used in react-native-blur?

I haven't looked at react-native-test-app yet.

As for the standalone app for Fabric - I think it's currently the better approach. Most of the libraries don't support Fabric yet and having this layer of separation helps developing for both architectures. We also found that limiting the outside dependencies may be beneficial. As for now, every release of React Native (since 0.68) introduced some (at least for us) breaking changes - this caused problems when adapting our libraries to the newest versions, as we needed to fix problems in the app's dependencies first. Recently we've had that exact problem between Reanimated, Screens and Gesture Handler, because the example apps are dependant on our other libraries and we're considering simplifying them.

fortmarek · 2022-08-18T08:00:23Z

ios/Sources/AutoLayoutView.swift

@@ -46,7 +51,15 @@ import UIKit
    /// Tracks where first pixel is drawn in the visible window
    private var lastMinBound: CGFloat = 0

-    override func layoutSubviews() {
+    private func getSubviews() -> [UIView] {


nit (if this can be overridden – or we can name this property differently):

Suggested change

private func getSubviews() -> [UIView] {

private override var subviews: [UIView] {

I tried to override it but it's being used internally by UIKit and was causing exceptions. I renamed it to viewsToLayout, is that an ok name?

fortmarek · 2022-08-18T08:02:27Z

ios/Sources/AutoLayoutView.swift

@@ -266,6 +289,7 @@ import UIKit
    }

    private func footer() -> UIView? {
-        return superview?.subviews.first(where:{($0 as? CellContainer)?.index == -1})
+        // TODO: Investigate hierarchy here


What exactly do you mean?

That was a reminder for me 😅. I've updated it now.

fortmarek · 2022-08-18T08:04:51Z

ios/Sources/AutoLayoutViewComponentView.mm

+    _autoLayoutView.onBlankAreaEventHandler = ^(CGFloat start, CGFloat end) {
+      AutoLayoutViewComponentView *strongSelf = weakSelf;
+      if (strongSelf->_eventEmitter != nullptr) {
+        std::dynamic_pointer_cast<const facebook::react::AutoLayoutViewEventEmitter>(strongSelf->_eventEmitter)
+          ->onBlankAreaEvent(facebook::react::AutoLayoutViewEventEmitter::OnBlankAreaEvent{
+            .offsetStart = (int) floor(start),
+            .offsetEnd = (int) floor(end),
+          });
+      }


Can you link me some additional resources? This seems quite complicated for sending an event, so would love to learn more.

I was relying on how it's done in Screens. The weakSelf is to make sure I don't cause a retain cycle doing this.

fortmarek · 2022-08-18T08:06:28Z

src/fabric/AutoLayoutViewNativeComponent.js

+  renderAheadOffset?: Double,
+  enableInstrumentation?: boolean,
+  disableAutoLayout?: boolean,
+  onBlankAreaEvent?: DirectEventHandler<BlankAreaEvent>, // This type is not parsable by the TS codegen, once it is, let's rewrite this file to TS


Do you know if this is tracked somewhere? Would really like to move to TS, at least eventually ...

reactwg/react-native-new-architecture#27

fortmarek · 2022-08-18T08:07:03Z

src/fabric/CellContainerNativeComponent.ts

@@ -0,0 +1,19 @@
+// @ts-ignore TODO: remove once there is a .d.ts file with definitions


Is there an issue for this somewhere?

Possibly the same as above. Other that that - I don't know.

fortmarek · 2022-08-18T08:09:51Z

@j-piasecki – can we point to the iOS branch for now? This PR has bundled changes with iOS which I have, mistakenly, reviewed as well 😞

We could also bundle these two PRs together into this one (I think that's fine as long as you squash some commits, so it's easier to follow along commit-by-commit)

j-piasecki · 2022-08-18T13:25:18Z

Pointing this to the iOS branch would mean moving the PR to my fork, so I think we can bundle them together. I'll get to it soon.

j-piasecki · 2022-08-22T10:30:35Z

As for your question about the performance - at the moment the main suspect is the JS Fabric renderer as the performance improves significantly in release mode.

fortmarek · 2022-08-24T10:38:04Z

@j-piasecki the posted performance number are from release mode or debug mode?

j-piasecki · 2022-08-24T10:56:52Z

@fortmarek They are from release mode.

naqvitalha · 2022-09-05T19:20:34Z

@j-piasecki Have you been able to look into why UI thread FPS is getting impacted with Fabric enabled? I think that's important before we call FlashList fabric ready. Let me know if I can help.

j-piasecki · 2022-09-06T09:40:03Z

I have looked into it briefly, but I don't have anything concrete yet.

j-piasecki · 2022-09-21T12:36:46Z

I have an update - the terrible performance on Android in debug mode is related to Hermes. It seems that the Hermes gets built in debug mode which slows it down quite a bit. After replacing the libhermes.so inside the APK file with the one extracted from the app built in release mode (or from the debug build of an app in the old arch) the performance gets much better.

If I understand correctly, there is a plan to add support for Prefab to Hermes in React Native 0.71, which could mean that this particular problem will solve itself with the next React Native release.

cortinico · 2022-09-21T13:17:15Z

Sorry for jumping on this. I've been watching this as I have a side project app where I'm eager to try FlashList with Fabric.

I have an update - the terrible performance on Android in debug mode is related to Hermes. It seems that the Hermes gets built in debug mode which slows it down quite a bit. After replacing the libhermes.so inside the APK file with the one extracted from the app built in release mode (or from the debug build of an app in the old arch) the performance gets much better.

Thanks for investigating this.

Technically you should not be blocked by 0.71.x in any form.
We're already shipping two .aar for Hermes, the hermes-engine-debug.aar and hermes-engine-release.aar. They contain libhermes.so files which are optimized for Debug/Release.

If I understand correctly, there is a plan to add support for Prefab to Hermes in React Native 0.71, which could mean that this particular problem will solve itself with the next React Native release.

Correct that we're actively looking into this. I'm wondering why this is affecting FlashList. Are you rebundling Hermes or doing anything related? If so, I'm curious to hear more as, in theory, libraries should behave transparently to the underlying JS engine.

j-piasecki · 2022-09-21T13:46:09Z

I may have misunderstood what using prefabs will mean. At the moment Hermes is being built from source, would that still be the case with prefabs?

I think that this could be a potential cause of performance problems. I've just improved the fps of the benchmark in debug mode (the same that the numbers in the PR description are from) by about 10 times by using ndk 21.4.7075529 (via Rosetta on M1 Pro) instead of ndk 24.0.8215888.

cortinico · 2022-09-21T15:54:13Z

I may have misunderstood what using prefabs will mean. At the moment Hermes is being built from source, would that still be the case with prefabs?

The way how hermes is built should be transparent to the end user and also to this library. The fact that we ship libhermes.so or you build it on your computer should make no difference.

You can still pick between Debug and Release when building your app, and Hermes will follow accordingly.

I think that this could be a potential cause of performance problems. I've just improved the fps of the benchmark in debug mode (the same that the numbers in the PR description are from) by about 10 times by using ndk 21.4.7075529 (via Rosetta on M1 Pro) instead of ndk 24.0.8215888.

This is really odd and, unless NDK 24 changed the default compilation flags, should not happen.
As a rule of thumb I would suggest you run all of your benchmarks against the same NDK version/variant otherwise is really hard to track down potential regression and especially understand if they're related to Fabric or not.

tomekzaw · 2022-09-22T08:10:15Z

Hey @cortinico, sorry for jumping in here. Unfortunately, the issue with libhermes.so also affects not only performance but also the way how C++ exceptions are handled between shared library boundaries. This results in app crashes on Android when building with NDK 24.

Try creating a fresh RN 0.70 app, enable Fabric and add throw new Error('error'); somewhere in JS code, and build the app for Android. If you build the app on Intel-based Mac with NDK 21, the error appears in the red box as it should. However, if you build the app on M1-based Mac with NDK 24, the app crashes.

The last message in logcat is JSI rethrowing JS exception: [...]. It looks like libhermes.so throws a jsi::JSError
(https://github.com/facebook/hermes/blob/98670b0ed40b9edd5fd405dc5a28119ad0aff35f/API/hermes/hermes.cpp#L2278-L2280) but for some reason catch (jsi::JSError &e) (or even catch (...)) is not able to catch the exception and the app crashes. (On iOS the exception can be caught with catch (...) or catch (std::exception &e) but catch (jsi::JSError &e) doesn't work as probably due to duplicated symbols, but this sounds like a separate issue.)

We are highly affected by this bug in Reanimated (when worklet throws JS error the app crashes on Android with Fabric enabled if using NDK 24). The crash is gone when building with NDK 21 (or just replacing libhermes.so in app-debug.apk).

cortinico · 2022-09-22T08:44:45Z

Hey @cortinico, sorry for jumping in here. Unfortunately, the issue with libhermes.so also affects not only performance but also the way how C++ exceptions are handled between shared library boundaries. This results in app crashes on Android when building with NDK 24.

Thanks for flagging this @tomekzaw. The issue is quite concerning to me and I'd like to investigate further. Can you open an issue on Reanimated or on React Native and let's keep the convo there. I don't want to sidetrack this PR for FlashList 👍

j-piasecki · 2022-09-22T10:36:05Z

The way how hermes is built should be transparent to the end user and also to this library. The fact that we ship libhermes.so or you build it on your computer should make no difference.

I understand, and while it should make no difference, it seems like the tools used to build hermes may have a substantial effect on its performance. I feel like it's important to keep it in mind, especially now that hermes is built from source and we cannot guarantee the setup. It may lead to hard-to-explain performance problems, like the one I encountered with FlashList.

Also, it may be clear from your answer but I cannot infer whether the hermes will still be built locally in future 😅.

We're already shipping two .aar for Hermes, the hermes-engine-debug.aar and hermes-engine-release.aar. They contain libhermes.so files which are optimized for Debug/Release.

In that case, may I ask what's the reasoning for building it locally even though there is one already built available?

cortinico · 2022-09-22T10:53:29Z

Also, it may be clear from your answer but I cannot infer whether the hermes will still be built locally in future 😅.

Yes, we're looking into this at the moment. Most likely will land on the next major release of React Native

In that case, may I ask what's the reasoning for building it locally even though there is one already built available?

Sorry if I was unclear. What I meant that, regardless if you're building Hermes locally or getting a prebuilt, Hermes is 'variant aware'. So there is a hermes-engine-release.aar (either downloaded or prepared locally) which contains a version of Hermes optimized for Release.

To give you more context, the situation is a bit more complicate in terms of prebuilts as:

Users on Old Architecture don't need to compile any C++ code. Therefore they can use the prebuilt version of hermes-engine-release|debug.aar we ship for them (see bundled hermes docs here: https://reactnative.dev/architecture/bundled-hermes)
Users on New Architecture need to compile C++ code introduced by the new arch. Therefore they can't (yet) use the prebuilt version of hermes-engine-release|debug.aar as that's missing headers and other machinery to let it compile successfully.
- That's what we're looking into solving at the moment so every user, regardless of the arch, can use a prebuilt version of hermes on Android.

naqvitalha · 2022-09-23T04:09:35Z

@cortinico @j-piasecki My main worry with fabric right now is that we're not able to maintain 60fps on the UI thread. It seems like drops in JS thread directly impact UI thread in the new architecture.
Do we have any insights into it? It's incredibly difficult to maintain 60fps on the JS thread.
In the old arch few drops in JS thread did not make a difference while in the new arch these drops interrupt scroll. Even minor drops are noticeable.
I can look into this sometime in the future but I'd love to hear some ideas.

j-piasecki · 2022-09-23T12:02:49Z

I don't have any concrete ideas as to what may be the cause of it yet. What I can say is that after I overloaded the JS thread to the point where recycling the views in the list takes a few seconds, the scrolling itself remained smooth. Will investigate this more.

## Description This PR improves developer experience of error handling in worklets by showing a RedBox with full error message rather than crashing the whole app on Android. **Note:** Android with Fabric enabled works only if building with NDK 21 via `yarn react-native run-android [--active-arch-only]`, crashes when building with NDK 24 from Android Studio, see Shopify/flash-list#550 (comment) for details (this is not an issue with Reanimated) <table> <thead> <tr> <th rowspan="2">Scenario</th> <th colspan="2">Before</th> <th colspan="2">After</th> </tr> <tr> <th>Paper</th> <th>Fabric</th> <th>Paper</th> <th>Fabric</th> </thead> <tbody> <tr> <td>worklet</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>nested worklet</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useAnimatedStyle</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useDerivedValue</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useFrameCallback</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>GestureDetector</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useAnimatedGestureHandler</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useAnimatedScrollHandler</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useScrollViewOffset</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> </tbody> </table> ## Test code and steps to reproduce ### Building with NDK 21 on M1-based Mac https://github.com/software-mansion/react-native-reanimated/blob/48af341d51ca289dc007fdfd81d124ae0c267523/FabricExample/android/build.gradle#L11-L17 ```diff - ndkVersion = "24.0.8215888" + ndkVersion = "21.4.7075529" ``` `ERROR: Unknown host CPU architecture: arm64` ```console code ~/Library/Android/sdk/ndk/21.4.7075529/ndk-build ``` ```diff #!/bin/sh DIR="$(cd "$(dirname "$0")" && pwd)" -$DIR/build/ndk-build "$@" +arch -x86_64 $DIR/build/ndk-build "$@" ``` ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [ ] Updated documentation - [ ] Ensured that CI passes

This PR improves developer experience of error handling in worklets by showing a RedBox with full error message rather than crashing the whole app on Android. **Note:** Android with Fabric enabled works only if building with NDK 21 via `yarn react-native run-android [--active-arch-only]`, crashes when building with NDK 24 from Android Studio, see Shopify/flash-list#550 (comment) for details (this is not an issue with Reanimated) <table> <thead> <tr> <th rowspan="2">Scenario</th> <th colspan="2">Before</th> <th colspan="2">After</th> </tr> <tr> <th>Paper</th> <th>Fabric</th> <th>Paper</th> <th>Fabric</th> </thead> <tbody> <tr> <td>worklet</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>nested worklet</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useAnimatedStyle</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useDerivedValue</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useFrameCallback</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>GestureDetector</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useAnimatedGestureHandler</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useAnimatedScrollHandler</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> <tr> <td>useScrollViewOffset</td> <td align="center">💥</td> <td align="center">💥</td> <td align="center">✅</td> <td align="center">✅</td> </tr> </tbody> </table> https://github.com/software-mansion/react-native-reanimated/blob/48af341d51ca289dc007fdfd81d124ae0c267523/FabricExample/android/build.gradle#L11-L17 ```diff - ndkVersion = "24.0.8215888" + ndkVersion = "21.4.7075529" ``` `ERROR: Unknown host CPU architecture: arm64` ```console code ~/Library/Android/sdk/ndk/21.4.7075529/ndk-build ``` ```diff DIR="$(cd "$(dirname "$0")" && pwd)" -$DIR/build/ndk-build "$@" +arch -x86_64 $DIR/build/ndk-build "$@" ``` - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [ ] Updated documentation - [ ] Ensured that CI passes

j-piasecki · 2022-10-26T13:40:58Z

I think the stuttering may be caused by the text. Here's a part of a profiler session on Samsung Galaxy A53 (Release build):

And focused on one update command:

The duration of the FabricUIManager::mountViews call in the screenshot above is over 6ms, which is often enough to cause a visible frame drop.
You should be able to get the same results using Android profiler and recording a System Trace. Keep in mind that I added some more systrace sections in IntBufferBatchMountItem, SurfaceMountingManager and in relevant methods in ReactTextViewManager and TextLayoutManager.

j-piasecki · 2022-11-04T14:18:41Z

After editing the benchmark to render rectangles in place of text I've got the following results:

Fabric:

	Avg FPS	Min FPS	Max FPS	Max blank area	Cumulative blank area
Nokia 1 Plus	33.7	19.8	45.9	1007	47899
Samsung Galaxy A53	117.3	111	120.2	0	0
Poco F3	117.9	110.4	120.4	0	0

Paper:

	Avg FPS	Min FPS	Max FPS	Max blank area	Cumulative blank area
Nokia 1 Plus	35.2	19.3	50.3	781	42654
Samsung Galaxy A53	116.2	106.8	120.2	8	8
Poco F3	119.4	112.4	121.2	0	0

Those numbers seem much closer than those from the original post.

j-piasecki · 2024-06-07T15:42:33Z

I was able to get rid of a few hacks around view hierarchy on iOS by modifying methods responsible for mounting children, so the native hierarchy is the same as on the old architecture.

WoLewicki · 2024-06-07T17:08:11Z

android/src/main/kotlin/com/shopify/reactnative/flash_list/AutoLayoutViewManager.kt

-        ).build();
-    }
+    override fun getExportedCustomDirectEventTypeConstants() = mutableMapOf(
+        "onBlankAreaEvent" to mutableMapOf("registrationName" to "onBlankAreaEvent"),


Why do you need here both onBlankAreaEvent and topOnBlankAreaEvent?

To be honest, I don't remember 😅. It's been a while since I've written this, but it doesn't seem to be causing any issues when I removed topOnBlankAreaEvent.

naqvitalha

@j-piasecki This is good to go. Just a few comments. It looks like E2E tests are breaking. We can proceed with the merge if you can address the comments and the E2E tests. Huge thanks for your incredible work on this PR!

naqvitalha · 2024-06-11T19:45:41Z

ios/Sources/AutoLayoutView.swift

@@ -274,6 +282,13 @@ import UIKit
    }

    private func footer() -> UIView? {
-        return superview?.subviews.first(where:{($0 as? CellContainer)?.index == -1})
+        // On the new arch, AutoLayoutView is wrapped with AutoLayoutViewComponentView, so we need to go up one more level


Can you explain why do we need the extra view?

On the new architecture a lot of the code needed by views is implemented inside RCTViewComponentView which is written in ObjC++. Ideally to create a new view component on the new architecture, one would inherit after it but it has two problems:

it's not implemented on the old architecture, and would require some tricks to make it work

I don't think it's possible for a Swift class to inherit after ObjC++ class

Instead of that, it's possible to create a wrapper view that inherits after RCTViewComponentView (in this case AutoLayoutViewComponentView) that will handle props and events of the underlying view that's already been implemented.

naqvitalha · 2024-06-11T19:46:14Z

fixture/react-native/patches/react-native-safe-area-context+4.10.1.patch

@@ -0,0 +1,288 @@
+diff --git a/node_modules/react-native-safe-area-context/android/src/main/jni/CMakeLists.txt b/node_modules/react-native-safe-area-context/android/src/main/jni/CMakeLists.txt


Do we need these patches now? Can we just upgrade the dependencies?

They are necessary to address the breaking changes between different versions of React Native.

Can we just upgrade the dependencies?

In this case, it would need to be a downgrade since both screens and safe-area-context are on the newest versions, which are adapted for React Native 0.74.

Upgrading the version of RN used in the fixture app would also help (upgrading other dependencies like reanimated and gesture-handler would also be necessary) but I'm not sure whether that's desirable or even in scope of this PR.

j-piasecki · 2024-06-13T11:24:56Z

I've changed the fixture app to have the new arch disabled by default - I couldn't get Detox to work on the new arch, it always hangs on waiting until the app is ready. It seems like it's not tested on the new arch - https://wix.github.io/Detox/docs/next/introduction/environment-setup/

j-piasecki · 2024-06-17T07:59:49Z

I cannot reproduce failing e2e tests locally. It's possible that's because of e5d3bde, but in case it doesn't help, is it possible to get the screenshots from the tests where it failed to match the snapshot from the github action?

naqvitalha · 2024-06-17T16:52:20Z

@j-piasecki I'm gonna look into the e2e tests shortly.

naqvitalha

LGTM, thanks a lot @j-piasecki for this PR!

T-Damer · 2024-06-19T03:39:21Z

LFG 🚀

Hosam-hsm · 2024-06-19T04:20:19Z

Is this published?

jfalih · 2024-06-21T10:09:02Z

isn't published yet?

mensonones · 2024-06-27T21:47:58Z

@Hosam-hsm @jfalih

For tests follow steps

package.json: "@shopify/flash-list": "git+https://github.com/Shopify/flash-list.git"
Using: import FlashList from '@shopify/flash-list/src/FlashList';

github-actions bot added cla-needed and removed cla-needed labels Aug 4, 2022

fortmarek reviewed Aug 18, 2022

View reviewed changes

fortmarek mentioned this pull request Aug 18, 2022

Fabric support android #356

Closed

1 task

j-piasecki force-pushed the @jpiasecki/fabric-android branch from 8462508 to 22fbea7 Compare August 22, 2022 07:47

j-piasecki mentioned this pull request Aug 22, 2022

Fabric support on iOS #549

Closed

1 task

j-piasecki changed the title ~~Fabric support on Android~~ Fabric support Aug 23, 2022

tomekzaw mentioned this pull request Sep 22, 2022

Fix Android crash when worklet throws error software-mansion/react-native-reanimated#3558

Merged

6 tasks

j-piasecki added 7 commits June 7, 2024 12:24

Update podspec

9c36e0a

Setup ios code

ade08ac

Fix AutoLayout subviews and footer

a77dc81

Implement onBlankAreaEvent on iOS

c12786f

Update comments

2c7f3c7

Implement Android

ddb6c6a

Mound children under AutoLayoutView

eb5cfad

j-piasecki reopened this Jun 7, 2024

j-piasecki force-pushed the @jpiasecki/fabric-android branch from f16ec5d to eb5cfad Compare June 7, 2024 15:33

j-piasecki changed the title ~~Fabric support~~ New architecture support Jun 7, 2024

Update changelog

8c212ee

WoLewicki reviewed Jun 7, 2024

View reviewed changes

j-piasecki added 2 commits June 10, 2024 10:36

Improve event handling

c7528cd

Remove context extension

44302e9

naqvitalha reviewed Jun 11, 2024

View reviewed changes

Disable new arch by default

25abbb7

Update podfile.lock

e5d3bde

Merge branch 'main' into @jpiasecki/fabric-android

03815c9

naqvitalha approved these changes Jun 18, 2024

View reviewed changes

update podfile

c740e67

naqvitalha enabled auto-merge (squash) June 18, 2024 21:30

naqvitalha merged commit 029fd40 into Shopify:main Jun 18, 2024
11 checks passed

		@@ -0,0 +1,46 @@
		/**
		* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).

	private func getSubviews() -> [UIView] {
	private override var subviews: [UIView] {

		@@ -0,0 +1,19 @@
		// @ts-ignore TODO: remove once there is a .d.ts file with definitions

		@@ -0,0 +1,288 @@
		diff --git a/node_modules/react-native-safe-area-context/android/src/main/jni/CMakeLists.txt b/node_modules/react-native-safe-area-context/android/src/main/jni/CMakeLists.txt

New architecture support #550

New architecture support #550

Conversation

j-piasecki commented Aug 4, 2022 • edited Loading

Description

Reviewers’ hat-rack 🎩

Screenshots or videos (if needed)

Checklist

hirbod commented Aug 14, 2022

fortmarek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fortmarek commented Aug 18, 2022

j-piasecki commented Aug 18, 2022

j-piasecki commented Aug 22, 2022

fortmarek commented Aug 24, 2022

j-piasecki commented Aug 24, 2022

naqvitalha commented Sep 5, 2022

j-piasecki commented Sep 6, 2022

j-piasecki commented Sep 21, 2022

cortinico commented Sep 21, 2022

j-piasecki commented Sep 21, 2022

cortinico commented Sep 21, 2022

tomekzaw commented Sep 22, 2022

cortinico commented Sep 22, 2022

j-piasecki commented Sep 22, 2022

cortinico commented Sep 22, 2022

naqvitalha commented Sep 23, 2022

j-piasecki commented Sep 23, 2022

j-piasecki commented Oct 26, 2022

j-piasecki commented Nov 4, 2022

j-piasecki commented Jun 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naqvitalha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-piasecki commented Jun 13, 2024

j-piasecki commented Jun 17, 2024

naqvitalha commented Jun 17, 2024

naqvitalha left a comment

Choose a reason for hiding this comment

T-Damer commented Jun 19, 2024

Hosam-hsm commented Jun 19, 2024

jfalih commented Jun 21, 2024

mensonones commented Jun 27, 2024

j-piasecki commented Aug 4, 2022 •

edited

Loading