From a71932ca3e3d476e200d957ae5b246b8c1c1d34f Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Tue, 8 Aug 2023 20:13:49 +0100 Subject: [PATCH] [LOCAL] Fabric Interop - Properly dispatch integer commands (#38527) (#38835) resolved: https://github.com/facebook/react-native/pull/38527 --- .../react/fabric/FabricUIManager.java | 47 +++++++++++---- .../react/fabric/FabricUIManagerTest.kt | 57 +++++++++++++++++++ .../fakes/FakeBatchEventDispatchedListener.kt | 17 ++++++ .../uiapp/component/MyLegacyViewManager.java | 20 +++++-- 4 files changed, 126 insertions(+), 15 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt 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 49acc4c358371a..c0df88da2c6cd1 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 @@ -53,7 +53,6 @@ import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.react.common.mapbuffer.ReadableMapBuffer; import com.facebook.react.config.ReactFeatureFlags; -import com.facebook.react.fabric.events.EventBeatManager; import com.facebook.react.fabric.events.EventEmitterWrapper; import com.facebook.react.fabric.events.FabricEventEmitter; import com.facebook.react.fabric.interop.InteropEventEmitter; @@ -61,6 +60,7 @@ import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.fabric.mounting.SurfaceMountingManager; import com.facebook.react.fabric.mounting.SurfaceMountingManager.ViewEvent; +import com.facebook.react.fabric.mounting.mountitems.DispatchCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchIntCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchStringCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.IntBufferBatchMountItem; @@ -79,6 +79,7 @@ import com.facebook.react.uimanager.UIManagerHelper; import com.facebook.react.uimanager.ViewManagerPropertyUpdater; import com.facebook.react.uimanager.ViewManagerRegistry; +import com.facebook.react.uimanager.events.BatchEventDispatchedListener; import com.facebook.react.uimanager.events.EventCategoryDef; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.react.uimanager.events.EventDispatcherImpl; @@ -168,7 +169,7 @@ public void onFabricCommitEnd(DevToolsReactPerfLogger.FabricCommitPoint commitPo @NonNull private final MountItemDispatcher mMountItemDispatcher; @NonNull private final ViewManagerRegistry mViewManagerRegistry; - @NonNull private final EventBeatManager mEventBeatManager; + @NonNull private final BatchEventDispatchedListener mBatchEventDispatchedListener; @NonNull private final CopyOnWriteArrayList mListeners = new CopyOnWriteArrayList<>(); @@ -208,16 +209,16 @@ public void executeItems(Queue items) { }; public FabricUIManager( - ReactApplicationContext reactContext, - ViewManagerRegistry viewManagerRegistry, - EventBeatManager eventBeatManager) { + @NonNull ReactApplicationContext reactContext, + @NonNull ViewManagerRegistry viewManagerRegistry, + @NonNull BatchEventDispatchedListener batchEventDispatchedListener) { mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext); mReactApplicationContext = reactContext; mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor); mMountItemDispatcher = new MountItemDispatcher(mMountingManager, new MountItemDispatchListener()); mEventDispatcher = new EventDispatcherImpl(reactContext); - mEventBeatManager = eventBeatManager; + mBatchEventDispatchedListener = batchEventDispatchedListener; mReactApplicationContext.addLifecycleEventListener(this); mViewManagerRegistry = viewManagerRegistry; @@ -385,7 +386,7 @@ public void stopSurface(final int surfaceID) { @Override public void initialize() { mEventDispatcher.registerEventEmitter(FABRIC, new FabricEventEmitter(this)); - mEventDispatcher.addBatchEventDispatchedListener(mEventBeatManager); + mEventDispatcher.addBatchEventDispatchedListener(mBatchEventDispatchedListener); if (ENABLE_FABRIC_PERF_LOGS) { mDevToolsReactPerfLogger = new DevToolsReactPerfLogger(); mDevToolsReactPerfLogger.addDevToolsReactPerfLoggerListener(FABRIC_PERF_LOGGER); @@ -424,7 +425,7 @@ public void onCatalystInstanceDestroy() { // memory immediately. mDispatchUIFrameCallback.stop(); - mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager); + mEventDispatcher.removeBatchEventDispatchedListener(mBatchEventDispatchedListener); mEventDispatcher.unregisterEventEmitter(FABRIC); mReactApplicationContext.unregisterComponentCallbacks(mViewManagerRegistry); @@ -1039,8 +1040,16 @@ public void dispatchCommand( final int reactTag, final String commandId, @Nullable final ReadableArray commandArgs) { - mMountItemDispatcher.dispatchCommandMountItem( - new DispatchStringCommandMountItem(surfaceId, reactTag, commandId, commandArgs)); + if (ReactFeatureFlags.unstable_useFabricInterop) { + // For Fabric Interop, we check if the commandId is an integer. If it is, we use the integer + // overload of dispatchCommand. Otherwise, we use the string overload. + // and the events won't be correctly dispatched. + mMountItemDispatcher.dispatchCommandMountItem( + createDispatchCommandMountItemForInterop(surfaceId, reactTag, commandId, commandArgs)); + } else { + mMountItemDispatcher.dispatchCommandMountItem( + new DispatchStringCommandMountItem(surfaceId, reactTag, commandId, commandArgs)); + } } @Override @@ -1200,6 +1209,24 @@ public void didDispatchMountItems() { } } + /** + * Util function that takes care of handling commands for Fabric Interop. If the command is a + * string that represents a number (say "42"), it will be parsed as an integer and the + * corresponding dispatch command mount item will be created. + */ + /* package */ DispatchCommandMountItem createDispatchCommandMountItemForInterop( + final int surfaceId, + final int reactTag, + final String commandId, + @Nullable final ReadableArray commandArgs) { + try { + int commandIdInteger = Integer.parseInt(commandId); + return new DispatchIntCommandMountItem(surfaceId, reactTag, commandIdInteger, commandArgs); + } catch (NumberFormatException e) { + return new DispatchStringCommandMountItem(surfaceId, reactTag, commandId, commandArgs); + } + } + private class DispatchUIFrameCallback extends GuardedFrameCallback { private volatile boolean mIsMountingEnabled = true; diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt new file mode 100644 index 00000000000000..f8827eb7225547 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt @@ -0,0 +1,57 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.fabric + +import com.facebook.react.bridge.ReactApplicationContext +import com.facebook.react.uimanager.ViewManagerRegistry +import com.facebook.react.uimanager.events.BatchEventDispatchedListener +import com.facebook.testutils.fakes.FakeBatchEventDispatchedListener +import com.facebook.testutils.shadows.ShadowSoLoader +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(shadows = [ShadowSoLoader::class]) +class FabricUIManagerTest { + + private lateinit var reactContext: ReactApplicationContext + private lateinit var viewManagerRegistry: ViewManagerRegistry + private lateinit var batchEventDispatchedListener: BatchEventDispatchedListener + private lateinit var underTest: FabricUIManager + + @Before + fun setup() { + reactContext = ReactApplicationContext(RuntimeEnvironment.getApplication()) + viewManagerRegistry = ViewManagerRegistry(emptyList()) + batchEventDispatchedListener = FakeBatchEventDispatchedListener() + underTest = FabricUIManager(reactContext, viewManagerRegistry, batchEventDispatchedListener) + } + + @Test + fun createDispatchCommandMountItemForInterop_withValidString_returnsStringEvent() { + val command = underTest.createDispatchCommandMountItemForInterop(11, 1, "anEvent", null) + + // DispatchStringCommandMountItem is package private so we can `as` check it. + val className = command::class.java.name.substringAfterLast(".") + assertEquals("DispatchStringCommandMountItem", className) + } + + @Test + fun createDispatchCommandMountItemForInterop_withValidInt_returnsIntEvent() { + val command = underTest.createDispatchCommandMountItemForInterop(11, 1, "42", null) + + // DispatchIntCommandMountItem is package private so we can `as` check it. + val className = command::class.java.name.substringAfterLast(".") + assertEquals("DispatchIntCommandMountItem", className) + } +} diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt new file mode 100644 index 00000000000000..9f3dd1d78398a6 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt @@ -0,0 +1,17 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.testutils.fakes + +import com.facebook.react.uimanager.events.BatchEventDispatchedListener + +/** A fake [BatchEventDispatchedListener] for testing that does nothing. */ +class FakeBatchEventDispatchedListener : BatchEventDispatchedListener { + override fun onBatchEventDispatched() { + // do nothing + } +} diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java index a192837166ffa1..e2f34691472d88 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java @@ -27,7 +27,7 @@ public class MyLegacyViewManager extends SimpleViewManager { public static final String REACT_CLASS = "RNTMyLegacyNativeView"; - private static final Integer COMMAND_CHANGE_BACKGROUND_COLOR = 42; + private static final int COMMAND_CHANGE_BACKGROUND_COLOR = 42; private final ReactApplicationContext mCallerContext; public MyLegacyViewManager(ReactApplicationContext reactContext) { @@ -71,8 +71,7 @@ public final Map getExportedViewConstants() { @Override public final Map getExportedCustomBubblingEventTypeConstants() { - Map eventTypeConstants = new HashMap(); - eventTypeConstants.putAll( + return new HashMap<>( MapBuilder.builder() .put( "onColorChanged", @@ -81,18 +80,29 @@ public final Map getExportedCustomBubblingEventTypeConstants() { MapBuilder.of( "bubbled", "onColorChanged", "captured", "onColorChangedCapture"))) .build()); - return eventTypeConstants; } @Override public void receiveCommand( @NonNull MyNativeView view, String commandId, @Nullable ReadableArray args) { - if (commandId.contentEquals(COMMAND_CHANGE_BACKGROUND_COLOR.toString())) { + if (commandId.contentEquals("changeBackgroundColor")) { int sentColor = Color.parseColor(args.getString(0)); view.setBackgroundColor(sentColor); } } + @SuppressWarnings("deprecation") // We intentionally want to test against the legacy API here. + @Override + public void receiveCommand( + @NonNull MyNativeView view, int commandId, @Nullable ReadableArray args) { + switch (commandId) { + case COMMAND_CHANGE_BACKGROUND_COLOR: + int sentColor = Color.parseColor(args.getString(0)); + view.setBackgroundColor(sentColor); + break; + } + } + @Nullable @Override public Map getCommandsMap() {