From 738f2e701a48dbe03534430fe0a9c2e0510e51b4 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Tue, 12 Sep 2023 04:16:59 -0700 Subject: [PATCH] Stop sending customCoalesceKey in new renderer (#39406) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39406 `customCoalesceKey` is not currently used in Fabric, and we do not plan on leveraging this to do event unique'ing on Android. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D49145587 fbshipit-source-id: 6b4e76f47217e017b3a37cc0cc7081d5847eeeb3 --- .../react/fabric/FabricUIManager.java | 27 +++---------------- .../fabric/events/EventEmitterWrapper.java | 8 +++--- .../fabric/events/FabricEventEmitter.java | 3 +-- .../mounting/SurfaceMountingManager.java | 12 ++------- .../uimanager/events/ReactEventEmitter.java | 3 +-- .../jni/react/fabric/EventEmitterWrapper.cpp | 4 +-- .../jni/react/fabric/EventEmitterWrapper.h | 5 +--- .../fabric/events/TouchEventDispatchTest.kt | 9 +++---- 8 files changed, 16 insertions(+), 55 deletions(-) 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 1fbb9069eb8..943e6ec08a4 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 @@ -909,30 +909,13 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @Override public void receiveEvent(int reactTag, String eventName, @Nullable WritableMap params) { - receiveEvent(View.NO_ID, reactTag, eventName, params); + receiveEvent(View.NO_ID, reactTag, eventName, false, params, EventCategoryDef.UNSPECIFIED); } @Override public void receiveEvent( int surfaceId, int reactTag, String eventName, @Nullable WritableMap params) { - receiveEvent(surfaceId, reactTag, eventName, false, 0, params); - } - - public void receiveEvent( - int surfaceId, - int reactTag, - String eventName, - boolean canCoalesceEvent, - int customCoalesceKey, - @Nullable WritableMap params) { - receiveEvent( - surfaceId, - reactTag, - eventName, - canCoalesceEvent, - customCoalesceKey, - params, - EventCategoryDef.UNSPECIFIED); + receiveEvent(surfaceId, reactTag, eventName, false, params, EventCategoryDef.UNSPECIFIED); } /** @@ -955,7 +938,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { int reactTag, String eventName, boolean canCoalesceEvent, - int customCoalesceKey, @Nullable WritableMap params, @EventCategoryDef int eventCategory) { if (ReactBuildConfig.DEBUG && surfaceId == View.NO_ID) { @@ -975,8 +957,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // access to the event emitter later when the view is mounted. For now just save the event // in the view state and trigger it later. mMountingManager.enqueuePendingEvent( - reactTag, - new ViewEvent(eventName, params, eventCategory, canCoalesceEvent, customCoalesceKey)); + reactTag, new ViewEvent(eventName, params, eventCategory, canCoalesceEvent)); } else { // This can happen if the view has disappeared from the screen (because of async events) FLog.d(TAG, "Unable to invoke event: " + eventName + " for reactTag: " + reactTag); @@ -985,7 +966,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } if (canCoalesceEvent) { - eventEmitter.dispatchUnique(eventName, params, customCoalesceKey); + eventEmitter.dispatchUnique(eventName, params); } else { eventEmitter.dispatch(eventName, params, eventCategory); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/EventEmitterWrapper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/EventEmitterWrapper.java index 27cb19b3534..bec63670d22 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/EventEmitterWrapper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/EventEmitterWrapper.java @@ -39,8 +39,7 @@ public class EventEmitterWrapper { private native void dispatchEvent( @NonNull String eventName, @NonNull NativeMap params, @EventCategoryDef int category); - private native void dispatchUniqueEvent( - @NonNull String eventName, @NonNull NativeMap params, int customCoalesceKey); + private native void dispatchUniqueEvent(@NonNull String eventName, @NonNull NativeMap params); /** * Invokes the execution of the C++ EventEmitter. @@ -65,12 +64,11 @@ public class EventEmitterWrapper { * @param eventName {@link String} name of the event to execute. * @param params {@link WritableMap} payload of the event */ - public synchronized void dispatchUnique( - @NonNull String eventName, @Nullable WritableMap params, int customCoalesceKey) { + public synchronized void dispatchUnique(@NonNull String eventName, @Nullable WritableMap params) { if (!isValid()) { return; } - dispatchUniqueEvent(eventName, (NativeMap) params, customCoalesceKey); + dispatchUniqueEvent(eventName, (NativeMap) params); } public synchronized void destroy() { 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 6674681fc40..d7544fb173c 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 @@ -51,8 +51,7 @@ public class FabricEventEmitter implements RCTModernEventEmitter { Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricEventEmitter.receiveEvent('" + eventName + "')"); try { - mUIManager.receiveEvent( - surfaceId, reactTag, eventName, canCoalesceEvent, customCoalesceKey, params, category); + mUIManager.receiveEvent(surfaceId, reactTag, eventName, canCoalesceEvent, params, category); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java index be7bf13bbf6..b34ec84bb0d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java @@ -1130,8 +1130,7 @@ public class SurfaceMountingManager { private void dispatchEvent(EventEmitterWrapper eventEmitter, ViewEvent viewEvent) { if (viewEvent.canCoalesceEvent()) { - eventEmitter.dispatchUnique( - viewEvent.getEventName(), viewEvent.getParams(), viewEvent.getCustomCoalesceKey()); + eventEmitter.dispatchUnique(viewEvent.getEventName(), viewEvent.getParams()); } else { eventEmitter.dispatch( viewEvent.getEventName(), viewEvent.getParams(), viewEvent.getEventCategory()); @@ -1391,7 +1390,6 @@ public class SurfaceMountingManager { public static class ViewEvent { private final String mEventName; private final boolean mCanCoalesceEvent; - private final int mCustomCoalesceKey; private final @EventCategoryDef int mEventCategory; private @Nullable WritableMap mParams; @@ -1399,13 +1397,11 @@ public class SurfaceMountingManager { String eventName, @Nullable WritableMap params, @EventCategoryDef int eventCategory, - boolean canCoalesceEvent, - int customCoalesceKey) { + boolean canCoalesceEvent) { mEventName = eventName; mParams = params; mEventCategory = eventCategory; mCanCoalesceEvent = canCoalesceEvent; - mCustomCoalesceKey = customCoalesceKey; } public String getEventName() { @@ -1416,10 +1412,6 @@ public class SurfaceMountingManager { return mCanCoalesceEvent; } - public int getCustomCoalesceKey() { - return mCustomCoalesceKey; - } - public @EventCategoryDef int getEventCategory() { return mEventCategory; } 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 22eb199e25a..8372d0e3bc7 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 @@ -62,8 +62,7 @@ public class ReactEventEmitter implements RCTModernEventEmitter { @Override public void receiveEvent( int surfaceId, int targetTag, String eventName, @Nullable WritableMap event) { - // The two additional params here, `canCoalesceEvent` and `customCoalesceKey`, have no - // meaning outside of Fabric. + // We assume this event can't be coalesced. `customCoalesceKey` has no meaning in Fabric. receiveEvent(surfaceId, targetTag, eventName, false, 0, event, EventCategoryDef.UNSPECIFIED); } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.cpp index 5517304057c..9d37fa1f08c 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.cpp @@ -30,9 +30,7 @@ void EventEmitterWrapper::dispatchEvent( void EventEmitterWrapper::dispatchUniqueEvent( std::string eventName, - NativeMap* payload, - int customCoalesceKey) { - // TODO: customCoalesceKey currently unused + NativeMap* payload) { // It is marginal, but possible for this to be constructed without a valid // EventEmitter. In those cases, make sure we noop/blackhole events instead of // crashing. diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.h index 87cb2dfb5d0..21a8230e524 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventEmitterWrapper.h @@ -28,10 +28,7 @@ class EventEmitterWrapper : public jni::HybridClass { SharedEventEmitter eventEmitter; void dispatchEvent(std::string eventName, NativeMap* params, int category); - void dispatchUniqueEvent( - std::string eventName, - NativeMap* params, - int customCoalesceKey); + void dispatchUniqueEvent(std::string eventName, NativeMap* params); }; } // namespace facebook::react 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 5e1994f8579..e2ae05a4dd4 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 @@ -490,8 +490,7 @@ class TouchEventDispatchTest { } val argument = ArgumentCaptor.forClass(WritableMap::class.java) verify(uiManager, times(4)) - .receiveEvent( - anyInt(), anyInt(), anyString(), anyBoolean(), anyInt(), argument.capture(), anyInt()) + .receiveEvent(anyInt(), anyInt(), anyString(), anyBoolean(), argument.capture(), anyInt()) Assert.assertEquals(startMoveEndExpectedSequence, argument.allValues) } @@ -502,8 +501,7 @@ class TouchEventDispatchTest { } val argument = ArgumentCaptor.forClass(WritableMap::class.java) verify(uiManager, times(6)) - .receiveEvent( - anyInt(), anyInt(), anyString(), anyBoolean(), anyInt(), argument.capture(), anyInt()) + .receiveEvent(anyInt(), anyInt(), anyString(), anyBoolean(), argument.capture(), anyInt()) Assert.assertEquals(startMoveCancelExpectedSequence, argument.allValues) } @@ -514,8 +512,7 @@ class TouchEventDispatchTest { } val argument = ArgumentCaptor.forClass(WritableMap::class.java) verify(uiManager, times(6)) - .receiveEvent( - anyInt(), anyInt(), anyString(), anyBoolean(), anyInt(), argument.capture(), anyInt()) + .receiveEvent(anyInt(), anyInt(), anyString(), anyBoolean(), argument.capture(), anyInt()) Assert.assertEquals(startPointerMoveUpExpectedSequence, argument.allValues) }