From c9764c5a17b246972928affa9d8d0162eff47ade Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 25 Jan 2024 07:33:17 -0800 Subject: [PATCH] Clarify docs and deprecation on Event methods (#42600) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../ReactAndroid/api/ReactAndroid.api | 5 --- .../fabric/events/FabricEventEmitter.java | 8 ++-- .../react/uimanager/events/Event.java | 39 ++++++------------- .../uimanager/events/RCTEventEmitter.java | 11 ++++-- .../events/RCTModernEventEmitter.java | 2 + .../uimanager/events/ReactEventEmitter.java | 27 +++++++------ .../react/uimanager/events/TouchEvent.java | 3 ++ .../react/uimanager/events/TouchesHelper.java | 14 ++++--- .../fabric/events/TouchEventDispatchTest.kt | 34 ++++++++++++---- 9 files changed, 77 insertions(+), 66 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index a6cbbc6d8d7..d78986c2d98 100644 --- 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 ()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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java index d7544fb173c..d5601632c0c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/FabricEventEmitter.java @@ -16,7 +16,6 @@ import com.facebook.react.uimanager.common.ViewUtil; 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 class FabricEventEmitter implements RCTModernEventEmitter { @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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java index e8448813dbd..ba933b49f4e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java @@ -10,7 +10,6 @@ package com.facebook.react.uimanager.events; 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 abstract class Event { */ @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 @@ public abstract class Event { * * @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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTEventEmitter.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTEventEmitter.java index 49234af84d6..c51ed518ba5 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTEventEmitter.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTEventEmitter.java @@ -13,16 +13,19 @@ import com.facebook.react.bridge.JavaScriptModule; 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. + * + *

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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTModernEventEmitter.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTModernEventEmitter.java index d5939b74d8e..4acb3351da4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTModernEventEmitter.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/RCTModernEventEmitter.java @@ -31,5 +31,7 @@ public interface RCTModernEventEmitter extends RCTEventEmitter { @Nullable WritableMap event, @EventCategoryDef int category); + /** @deprecated Dispatch the TouchEvent using {@link EventDispatcher} instead */ + @Deprecated void receiveTouches(TouchEvent event); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java index 312239ed89f..aba13e8b9c6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/ReactEventEmitter.java +++ b/packages/react-native/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 @@ class ReactEventEmitter implements RCTModernEventEmitter { 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 @@ class ReactEventEmitter implements RCTModernEventEmitter { 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 @@ class ReactEventEmitter implements RCTModernEventEmitter { @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,9 +110,9 @@ class ReactEventEmitter implements RCTModernEventEmitter { */ @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, @@ -121,7 +120,7 @@ class ReactEventEmitter implements RCTModernEventEmitter { "Cannot get RCTEventEmitter from Context, no active Catalyst instance!")); } } - return mRCTEventEmitter; + return mDefaultEventEmitter; } @Override @@ -144,7 +143,7 @@ class ReactEventEmitter implements RCTModernEventEmitter { 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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchEvent.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchEvent.java index 91cf784260c..16aceaf3ef6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchEvent.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchEvent.java @@ -189,6 +189,9 @@ public class TouchEvent extends Event { @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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.java index 03a23c628ce..922b15ce4fa 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.java @@ -17,10 +17,11 @@ import com.facebook.systrace.Systrace; /** 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 @@ public class TouchesHelper { * @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 class TouchesHelper { * @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/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/events/TouchEventDispatchTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/events/TouchEventDispatchTest.kt index e2ae05a4dd4..4264834dd20 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/events/TouchEventDispatchTest.kt +++ b/packages/react-native/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 + private lateinit var reactChoreographer: MockedStatic @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.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))