Clarify docs and deprecation on Event methods (#42600)

Summary: Pull Request resolved: #42600 Disentangled the logic for emitting touches to JS on Android. `receiveTouches` should never be used as a public API, as TouchEvents can be dispatched just like any other event, and receiveTouches is an internal helper for `TouchEvent`. Changelog: [Android][Removed] Updated migrated guidance for EventEmitter and reduced visibility of internal TouchesHelper methods Reviewed By: cortinico Differential Revision: D52907393 fbshipit-source-id: a8207039c863ab23a1d93dd2d2f28e8a274c8ecf
facebook · Jan 25, 2024 · c9764c5 · c9764c5
1 parent 49f6ffc
commit c9764c5
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 66 deletions.
diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api
@@ -5587,13 +5587,8 @@ public final class com/facebook/react/uimanager/events/TouchEventType : java/lan
 }
 
 public class com/facebook/react/uimanager/events/TouchesHelper {
-	public static final field CHANGED_TOUCHES_KEY Ljava/lang/String;
 	public static final field TARGET_KEY Ljava/lang/String;
-	public static final field TARGET_SURFACE_KEY Ljava/lang/String;
-	public static final field TOUCHES_KEY Ljava/lang/String;
 	public fun <init> ()V
-	public static fun sendTouchEvent (Lcom/facebook/react/uimanager/events/RCTModernEventEmitter;Lcom/facebook/react/uimanager/events/TouchEvent;)V
-	public static fun sendTouchesLegacy (Lcom/facebook/react/uimanager/events/RCTEventEmitter;Lcom/facebook/react/uimanager/events/TouchEvent;)V
 }
 
 public class com/facebook/react/uimanager/layoutanimation/LayoutAnimationController {

diff --git a/...ative/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java b/...ative/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java
@@ -16,7 +16,6 @@
 import com.facebook.react.uimanager.events.EventCategoryDef;
 import com.facebook.react.uimanager.events.RCTModernEventEmitter;
 import com.facebook.react.uimanager.events.TouchEvent;
-import com.facebook.react.uimanager.events.TouchesHelper;
 import com.facebook.systrace.Systrace;
 
 public class FabricEventEmitter implements RCTModernEventEmitter {
@@ -63,11 +62,14 @@ public void receiveTouches(
       @NonNull String eventName,
       @NonNull WritableArray touches,
       @NonNull WritableArray changedIndices) {
-    throw new IllegalStateException("EventEmitter#receiveTouches is not supported by Fabric");
+    throw new UnsupportedOperationException(
+        "EventEmitter#receiveTouches is not supported by Fabric");
   }
 
   @Override
   public void receiveTouches(TouchEvent event) {
-    TouchesHelper.sendTouchEvent(this, event);
+    // Calls are expected to go via TouchesHelper
+    throw new UnsupportedOperationException(
+        "EventEmitter#receiveTouches is not supported by Fabric");
   }
 }
diff --git a/...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java b/...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java
@@ -10,7 +10,6 @@
 import androidx.annotation.Nullable;
 import com.facebook.react.bridge.WritableMap;
 import com.facebook.react.common.SystemClock;
-import com.facebook.react.uimanager.IllegalViewOperationException;
 
 /**
  * A UI event that can be dispatched to JS.
@@ -158,20 +157,10 @@ public boolean match(int viewTag, String eventName) {
    */
   @Deprecated
   public void dispatch(RCTEventEmitter rctEventEmitter) {
-    WritableMap eventData = getEventData();
-    if (eventData == null) {
-      throw new IllegalViewOperationException(
-          "Event: you must return a valid, non-null value from `getEventData`, or override `dispatch` and `dispatchModern`. Event: "
-              + getEventName());
-    }
-    rctEventEmitter.receiveEvent(getViewTag(), getEventName(), eventData);
+    rctEventEmitter.receiveEvent(getViewTag(), getEventName(), getEventData());
   }
 
-  /**
-   * Can be overridden by classes to make migrating to RCTModernEventEmitter support easier. If this
-   * class returns null, the RCTEventEmitter interface will be used instead of
-   * RCTModernEventEmitter. In the future, returning null here will be an error.
-   */
+  /** Can be overridden by classes when no custom logic for dispatching is needed. */
   @Nullable
   protected WritableMap getEventData() {
     return null;
@@ -193,23 +182,19 @@ protected int getEventCategory() {
    *
    * @see #dispatch
    */
-  @Deprecated
   public void dispatchModern(RCTModernEventEmitter rctEventEmitter) {
     if (getSurfaceId() != -1) {
-      WritableMap eventData = getEventData();
-      if (eventData != null) {
-        rctEventEmitter.receiveEvent(
-            getSurfaceId(),
-            getViewTag(),
-            getEventName(),
-            canCoalesce(),
-            getCoalescingKey(),
-            eventData,
-            getEventCategory());
-        return;
-      }
+      rctEventEmitter.receiveEvent(
+          getSurfaceId(),
+          getViewTag(),
+          getEventName(),
+          canCoalesce(),
+          getCoalescingKey(),
+          getEventData(),
+          getEventCategory());
+    } else {
+      dispatch(rctEventEmitter);
     }
-    dispatch(rctEventEmitter);
   }
 
   public interface EventAnimationDriverMatchSpec {

diff --git a/...ative/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTEventEmitter.java b/...ative/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTEventEmitter.java
@@ -13,16 +13,19 @@
 import com.facebook.react.bridge.WritableArray;
 import com.facebook.react.bridge.WritableMap;
 
-/** Deprecated in favor of RCTModernEventEmitter, which extends this interface. */
+/**
+ * Paper JS interface to emit events from native to JS.
+ *
+ * <p>Deprecated in favor of RCTModernEventEmitter, which works with both the old and new renderer.
+ */
 @DoNotStrip
 @Deprecated
 public interface RCTEventEmitter extends JavaScriptModule {
   /**
-   * Deprecated in favor of RCTModernEventEmitter.receiveEvent.
-   *
    * @param targetReactTag react tag of the view that receives the event
    * @param eventName name of event
    * @param event event params
+   * @deprecated Use RCTModernEventEmitter.receiveEvent instead
    */
   @Deprecated
   void receiveEvent(int targetReactTag, String eventName, @Nullable WritableMap event);
@@ -33,6 +36,8 @@ public interface RCTEventEmitter extends JavaScriptModule {
    * @param eventName JS event name
    * @param touches active pointers data
    * @param changedIndices indices of changed pointers
+   * @deprecated Dispatch the TouchEvent using {@link EventDispatcher} instead
    */
+  @Deprecated
   void receiveTouches(String eventName, WritableArray touches, WritableArray changedIndices);
 }
diff --git a/...ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTModernEventEmitter.java b/...ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTModernEventEmitter.java
@@ -31,5 +31,7 @@ void receiveEvent(
       @Nullable WritableMap event,
       @EventCategoryDef int category);
 
+  /** @deprecated Dispatch the TouchEvent using {@link EventDispatcher} instead */
+  @Deprecated
   void receiveTouches(TouchEvent event);
 }
diff --git a/...ive/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java b/...ive/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java
@@ -23,12 +23,11 @@ class ReactEventEmitter implements RCTModernEventEmitter {
 
   private static final String TAG = "ReactEventEmitter";
 
-  @Nullable
-  private RCTModernEventEmitter mFabricEventEmitter =
-      null; /* Corresponds to a Fabric EventEmitter */
+  /** Corresponds to {@link com.facebook.react.fabric.events.FabricEventEmitter} */
+  @Nullable private RCTModernEventEmitter mFabricEventEmitter = null;
 
-  @Nullable
-  private RCTEventEmitter mRCTEventEmitter = null; /* Corresponds to a Non-Fabric EventEmitter */
+  /** Corresponds to (Paper) EventEmitter, which is a JS interface */
+  @Nullable private RCTEventEmitter mDefaultEventEmitter = null;
 
   private final ReactApplicationContext mReactContext;
 
@@ -43,12 +42,12 @@ public void register(@UIManagerType int uiManagerType, RCTModernEventEmitter eve
 
   public void register(@UIManagerType int uiManagerType, RCTEventEmitter eventEmitter) {
     assert uiManagerType == UIManagerType.DEFAULT;
-    mRCTEventEmitter = eventEmitter;
+    mDefaultEventEmitter = eventEmitter;
   }
 
   public void unregister(@UIManagerType int uiManagerType) {
     if (uiManagerType == UIManagerType.DEFAULT) {
-      mRCTEventEmitter = null;
+      mDefaultEventEmitter = null;
     } else {
       mFabricEventEmitter = null;
     }
@@ -78,7 +77,7 @@ public void receiveTouches(
     int reactTag = touches.getMap(0).getInt(TARGET_KEY);
     @UIManagerType int uiManagerType = ViewUtil.getUIManagerType(reactTag);
     if (uiManagerType == UIManagerType.DEFAULT && getDefaultEventEmitter() != null) {
-      mRCTEventEmitter.receiveTouches(eventName, touches, changedIndices);
+      mDefaultEventEmitter.receiveTouches(eventName, touches, changedIndices);
     }
   }
 
@@ -88,9 +87,9 @@ public void receiveTouches(TouchEvent event) {
     @UIManagerType
     int uiManagerType = ViewUtil.getUIManagerType(event.getViewTag(), event.getSurfaceId());
     if (uiManagerType == UIManagerType.FABRIC && mFabricEventEmitter != null) {
-      mFabricEventEmitter.receiveTouches(event);
+      TouchesHelper.sendTouchEvent(mFabricEventEmitter, event);
     } else if (uiManagerType == UIManagerType.DEFAULT && getDefaultEventEmitter() != null) {
-      TouchesHelper.sendTouchesLegacy(mRCTEventEmitter, event);
+      TouchesHelper.sendTouchesLegacy(mDefaultEventEmitter, event);
     } else {
       ReactSoftExceptionLogger.logSoftException(
           TAG,
@@ -111,17 +110,17 @@ public void receiveTouches(TouchEvent event) {
    */
   @Nullable
   private RCTEventEmitter getDefaultEventEmitter() {
-    if (mRCTEventEmitter == null) {
+    if (mDefaultEventEmitter == null) {
       if (mReactContext.hasActiveReactInstance()) {
-        mRCTEventEmitter = mReactContext.getJSModule(RCTEventEmitter.class);
+        mDefaultEventEmitter = mReactContext.getJSModule(RCTEventEmitter.class);
       } else {
         ReactSoftExceptionLogger.logSoftException(
             TAG,
             new ReactNoCrashSoftException(
                 "Cannot get RCTEventEmitter from Context, no active Catalyst instance!"));
       }
     }
-    return mRCTEventEmitter;
+    return mDefaultEventEmitter;
   }
 
   @Override
@@ -144,7 +143,7 @@ public void receiveEvent(
           event,
           category);
     } else if (uiManagerType == UIManagerType.DEFAULT && getDefaultEventEmitter() != null) {
-      mRCTEventEmitter.receiveEvent(targetReactTag, eventName, event);
+      mDefaultEventEmitter.receiveEvent(targetReactTag, eventName, event);
     } else {
       ReactSoftExceptionLogger.logSoftException(
           TAG,

diff --git a/...act-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchEvent.java b/...act-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchEvent.java
@@ -189,6 +189,9 @@ public void dispatch(RCTEventEmitter rctEventEmitter) {
   @Override
   public void dispatchModern(RCTModernEventEmitter rctEventEmitter) {
     if (verifyMotionEvent()) {
+      // TouchesHelper.sendTouchEvent can be inlined here post Fabric rollout
+      // For now, we go via the event emitter, which will decide whether the legacy or modern
+      // event path is required
       rctEventEmitter.receiveTouches(this);
     }
   }

diff --git a/...-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.java b/...-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.java
@@ -17,10 +17,11 @@
 
 /** Class responsible for generating catalyst touch events based on android {@link MotionEvent}. */
 public class TouchesHelper {
-  public static final String TARGET_SURFACE_KEY = "targetSurface";
-  public static final String TARGET_KEY = "target";
-  public static final String CHANGED_TOUCHES_KEY = "changedTouches";
-  public static final String TOUCHES_KEY = "touches";
+  @Deprecated public static final String TARGET_KEY = "target";
+
+  private static final String TARGET_SURFACE_KEY = "targetSurface";
+  private static final String CHANGED_TOUCHES_KEY = "changedTouches";
+  private static final String TOUCHES_KEY = "touches";
   private static final String PAGE_X_KEY = "pageX";
   private static final String PAGE_Y_KEY = "pageY";
   private static final String TIMESTAMP_KEY = "timestamp";
@@ -80,7 +81,8 @@ private static WritableMap[] createPointersArray(TouchEvent event) {
    * @param rctEventEmitter Event emitter used to execute JS module call
    * @param touchEvent native touch event to read pointers count and coordinates from
    */
-  public static void sendTouchesLegacy(RCTEventEmitter rctEventEmitter, TouchEvent touchEvent) {
+  /* package */ static void sendTouchesLegacy(
+      RCTEventEmitter rctEventEmitter, TouchEvent touchEvent) {
     TouchEventType type = touchEvent.getTouchEventType();
 
     WritableArray pointers =
@@ -114,7 +116,7 @@ public static void sendTouchesLegacy(RCTEventEmitter rctEventEmitter, TouchEvent
    * @param eventEmitter emitter to dispatch event to
    * @param event the touch event to extract data from
    */
-  public static void sendTouchEvent(RCTModernEventEmitter eventEmitter, TouchEvent event) {
+  /* package */ static void sendTouchEvent(RCTModernEventEmitter eventEmitter, TouchEvent event) {
     Systrace.beginSection(
         Systrace.TRACE_TAG_REACT_JAVA_BRIDGE,
         "TouchesHelper.sentTouchEventModern(" + event.getEventName() + ")");

diff --git a/...ive/ReactAndroid/src/test/java/com/facebook/react/fabric/events/TouchEventDispatchTest.kt b/...ive/ReactAndroid/src/test/java/com/facebook/react/fabric/events/TouchEventDispatchTest.kt
@@ -11,10 +11,18 @@ import android.util.DisplayMetrics
 import android.view.MotionEvent
 import android.view.MotionEvent.PointerCoords
 import android.view.MotionEvent.PointerProperties
-import com.facebook.react.bridge.*
+import com.facebook.react.bridge.Arguments
+import com.facebook.react.bridge.JavaOnlyArray
+import com.facebook.react.bridge.JavaOnlyMap
+import com.facebook.react.bridge.ReactTestHelper
+import com.facebook.react.bridge.ReadableMap
+import com.facebook.react.bridge.WritableArray
+import com.facebook.react.bridge.WritableMap
 import com.facebook.react.fabric.FabricUIManager
+import com.facebook.react.modules.core.ReactChoreographer
 import com.facebook.react.uimanager.DisplayMetricsHolder
 import com.facebook.react.uimanager.ViewManagerRegistry
+import com.facebook.react.uimanager.events.EventDispatcher
 import com.facebook.react.uimanager.events.TouchEvent
 import com.facebook.react.uimanager.events.TouchEventCoalescingKeyHelper
 import com.facebook.react.uimanager.events.TouchEventType
@@ -28,12 +36,12 @@ import org.junit.runner.RunWith
 import org.mockito.ArgumentCaptor
 import org.mockito.ArgumentMatchers.*
 import org.mockito.MockedStatic
+import org.mockito.Mockito.mock
 import org.mockito.Mockito.mockStatic
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.robolectric.RobolectricTestRunner
-import org.robolectric.RuntimeEnvironment
 import org.robolectric.annotation.Config
 
 @RunWith(RobolectricTestRunner::class)
@@ -452,9 +460,10 @@ class TouchEventDispatchTest {
                       buildGesture(SURFACE_ID, TARGET_VIEW_ID, 1f, 3f, GESTURE_START_TIME, 0),
                       buildGesture(SURFACE_ID, TARGET_VIEW_ID, 2f, 1f, GESTURE_START_TIME, 1))))
 
-  private lateinit var eventEmitter: FabricEventEmitter
+  private lateinit var eventDispatcher: EventDispatcher
   private lateinit var uiManager: FabricUIManager
   private lateinit var arguments: MockedStatic<Arguments>
+  private lateinit var reactChoreographer: MockedStatic<ReactChoreographer>
 
   @Before
   fun setUp() {
@@ -469,24 +478,33 @@ class TouchEventDispatchTest {
 
     // We use a real FabricUIManager here as it's harder to mock with both static and non-static
     // methods.
-    val reactContext = ReactApplicationContext(RuntimeEnvironment.getApplication())
+    val reactContext = ReactTestHelper.createCatalystContextForTest()
     val viewManagerRegistry = ViewManagerRegistry(emptyList())
     val batchEventDispatchedListener = FakeBatchEventDispatchedListener()
     uiManager =
         spy(FabricUIManager(reactContext, viewManagerRegistry, batchEventDispatchedListener))
+    uiManager.initialize()
 
-    eventEmitter = FabricEventEmitter(uiManager)
+    eventDispatcher = uiManager.getEventDispatcher()
+
+    // Ignore scheduled choreographer work
+    val reactChoreographerMock = mock(ReactChoreographer::class.java)
+    reactChoreographer = mockStatic(ReactChoreographer::class.java)
+    reactChoreographer
+        .`when`<ReactChoreographer> { ReactChoreographer.getInstance() }
+        .thenReturn(reactChoreographerMock)
   }
 
   @After
   fun tearDown() {
     arguments.close()
+    reactChoreographer.close()
   }
 
   @Test
   fun testFabric_startMoveEnd() {
     for (event in startMoveEndSequence) {
-      event.dispatchModern(eventEmitter)
+      eventDispatcher.dispatchEvent(event)
     }
     val argument = ArgumentCaptor.forClass(WritableMap::class.java)
     verify(uiManager, times(4))
@@ -497,7 +515,7 @@ class TouchEventDispatchTest {
   @Test
   fun testFabric_startMoveCancel() {
     for (event in startMoveCancelSequence) {
-      event.dispatchModern(eventEmitter)
+      eventDispatcher.dispatchEvent(event)
     }
     val argument = ArgumentCaptor.forClass(WritableMap::class.java)
     verify(uiManager, times(6))
@@ -508,7 +526,7 @@ class TouchEventDispatchTest {
   @Test
   fun testFabric_startPointerUpCancel() {
     for (event in startPointerMoveUpSequence) {
-      event.dispatchModern(eventEmitter)
+      eventDispatcher.dispatchEvent(event)
     }
     val argument = ArgumentCaptor.forClass(WritableMap::class.java)
     verify(uiManager, times(6))