From 0f4234f45009be2c54dfd8a71466676034c4d6d9 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Thu, 21 Mar 2024 16:18:28 -0700 Subject: [PATCH] Fix InteropUIBlockListener to support react-native-view-shot on Bridgeless (#43594) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43594 I've been migrating `react-native-view-shot` to Fabric by using the `InteropUiBlockListener` and I've realized that the interop layer doesn't work well. 1. FabricUIManager needs to implement `UIBlockViewResolver` in order for the interop layer to work correctly. 2. We need to hook `addUIBlock` to the `didDispatchMountItems` callback otherwise the UIBlocks won't be executed at all. Changelog: [Android] [Fixed] - Fix InteropUIBlockListener to support react-native-view-shot on Bridgeless Reviewed By: javache Differential Revision: D55187939 fbshipit-source-id: d048b4b5eed77fa856fdfac17c0df5f23fd44844 --- .../ReactAndroid/api/ReactAndroid.api | 2 +- .../react/fabric/FabricUIManager.java | 3 +- .../interop/InteropUiBlockListener.kt | 10 +++- .../interop/InteropUiBlockListenerTest.kt | 48 +++++++++++++++++++ 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 4d72f0ea722..d4bb2e9a365 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -2516,7 +2516,7 @@ public class com/facebook/react/fabric/FabricSoLoader { public static fun staticInit ()V } -public class com/facebook/react/fabric/FabricUIManager : com/facebook/react/bridge/LifecycleEventListener, com/facebook/react/bridge/UIManager { +public class com/facebook/react/fabric/FabricUIManager : com/facebook/react/bridge/LifecycleEventListener, com/facebook/react/bridge/UIManager, com/facebook/react/fabric/interop/UIBlockViewResolver { public static final field ENABLE_FABRIC_LOGS Z public static final field ENABLE_FABRIC_PERF_LOGS Z public static final field IS_DEVELOPMENT_ENVIRONMENT Z diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index f091faaf87c..bd76cd15dbd 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -57,6 +57,7 @@ import com.facebook.react.fabric.events.EventEmitterWrapper; import com.facebook.react.fabric.events.FabricEventEmitter; import com.facebook.react.fabric.internal.interop.InteropUIBlockListener; import com.facebook.react.fabric.interop.UIBlock; +import com.facebook.react.fabric.interop.UIBlockViewResolver; import com.facebook.react.fabric.mounting.MountItemDispatcher; import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.fabric.mounting.SurfaceMountingManager; @@ -99,7 +100,7 @@ import java.util.concurrent.atomic.AtomicBoolean; */ @SuppressLint("MissingNativeLoadLibrary") @DoNotStripAny -public class FabricUIManager implements UIManager, LifecycleEventListener { +public class FabricUIManager implements UIManager, LifecycleEventListener, UIBlockViewResolver { public static final String TAG = FabricUIManager.class.getSimpleName(); // The IS_DEVELOPMENT_ENVIRONMENT variable is used to log extra data when running fabric in a diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListener.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListener.kt index 1db876d5aa1..3c3f7cfd2d4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListener.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListener.kt @@ -37,6 +37,9 @@ internal class InteropUIBlockListener : UIManagerListener { } override fun willMountItems(uiManager: UIManager) { + if (beforeUIBlocks.isEmpty()) { + return + } beforeUIBlocks.forEach { if (uiManager is UIBlockViewResolver) { it.execute(uiManager) @@ -46,6 +49,9 @@ internal class InteropUIBlockListener : UIManagerListener { } override fun didMountItems(uiManager: UIManager) { + if (afterUIBlocks.isEmpty()) { + return + } afterUIBlocks.forEach { if (uiManager is UIBlockViewResolver) { it.execute(uiManager) @@ -54,9 +60,9 @@ internal class InteropUIBlockListener : UIManagerListener { afterUIBlocks.clear() } - override fun willDispatchViewUpdates(uiManager: UIManager) = Unit + override fun didDispatchMountItems(uiManager: UIManager) = didMountItems(uiManager) - override fun didDispatchMountItems(uiManager: UIManager) = Unit + override fun willDispatchViewUpdates(uiManager: UIManager) = willMountItems(uiManager) override fun didScheduleMountItems(uiManager: UIManager) = Unit } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListenerTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListenerTest.kt index d875aa83ef8..71985256665 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListenerTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/internal/interop/InteropUiBlockListenerTest.kt @@ -43,6 +43,18 @@ class InteropUiBlockListenerTest { assertEquals(1, underTest.afterUIBlocks.size) } + @Test + fun willDispatchViewUpdates_emptiesBeforeUIBlocks() { + val underTest = InteropUIBlockListener() + underTest.prependUIBlock {} + underTest.addUIBlock {} + + underTest.willDispatchViewUpdates(FakeUIManager()) + + assertEquals(0, underTest.beforeUIBlocks.size) + assertEquals(1, underTest.afterUIBlocks.size) + } + @Test fun didMountItems_emptiesAfterUIBlocks() { val underTest = InteropUIBlockListener() @@ -55,6 +67,18 @@ class InteropUiBlockListenerTest { assertEquals(0, underTest.afterUIBlocks.size) } + @Test + fun didDispatchMountItems_emptiesAfterUIBlocks() { + val underTest = InteropUIBlockListener() + underTest.prependUIBlock {} + underTest.addUIBlock {} + + underTest.didDispatchMountItems(FakeUIManager()) + + assertEquals(1, underTest.beforeUIBlocks.size) + assertEquals(0, underTest.afterUIBlocks.size) + } + @Test fun willMountItems_deliversUiManagerCorrectly() { val fakeUIManager = FakeUIManager() @@ -67,6 +91,18 @@ class InteropUiBlockListenerTest { assertEquals(1, fakeUIManager.resolvedViewCount) } + @Test + fun willDispatchViewUpdates_deliversUiManagerCorrectly() { + val fakeUIManager = FakeUIManager() + val underTest = InteropUIBlockListener() + + underTest.prependUIBlock { uiManager -> uiManager.resolveView(0) } + + underTest.willDispatchViewUpdates(fakeUIManager) + + assertEquals(1, fakeUIManager.resolvedViewCount) + } + @Test fun didMountItems_deliversUiManagerCorrectly() { val fakeUIManager = FakeUIManager() @@ -78,4 +114,16 @@ class InteropUiBlockListenerTest { assertEquals(1, fakeUIManager.resolvedViewCount) } + + @Test + fun didDispatchMountItems_deliversUiManagerCorrectly() { + val fakeUIManager = FakeUIManager() + val underTest = InteropUIBlockListener() + + underTest.addUIBlock { uiManager -> uiManager.resolveView(0) } + + underTest.didDispatchMountItems(fakeUIManager) + + assertEquals(1, fakeUIManager.resolvedViewCount) + } }