[LOCAL] Fabric Interop - Properly dispatch integer commands (#38527) (#38835)

resolved: #38527

Author: cortinico

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

@@ -53,14 +53,14 @@
 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;
 import com.facebook.react.fabric.mounting.MountItemDispatcher;
 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<UIManagerListener> mListeners = new CopyOnWriteArrayList<>();
@@ -208,16 +209,16 @@ public void executeItems(Queue<MountItem> 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;

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)
+  }
+}

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
+  }
+}

packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java

@@ -27,7 +27,7 @@
 public class MyLegacyViewManager extends SimpleViewManager<MyNativeView> {
 
   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<String, Object> getExportedViewConstants() {
 
   @Override
   public final Map<String, Object> getExportedCustomBubblingEventTypeConstants() {
-    Map<String, Object> eventTypeConstants = new HashMap<String, Object>();
-    eventTypeConstants.putAll(
+    return new HashMap<>(
         MapBuilder.<String, Object>builder()
             .put(
                 "onColorChanged",
@@ -81,18 +80,29 @@ public final Map<String, Object> 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<String, Integer> getCommandsMap() {