From 2d514622f3df83818e0850bc92df2fab198f0a3f Mon Sep 17 00:00:00 2001 From: Dmitry Rykun Date: Mon, 22 Jan 2024 05:24:13 -0800 Subject: [PATCH] Event name normalization for Fabric interop Summary: Every event name must be normalized. The normalization strategy is: 1. If it starts with `top` -> do nothing. 2. If it starts with `on` -> replace `on` with `top`. 3. Else -> capitalize the first character and prepend `top`. We have it for the old renderer on iOS [here](https://github.com/facebook/react-native/blob/a7586947d719a9cd2344ad346d271e7ca900de87/packages/react-native/React/Base/RCTEventDispatcher.m#L12-L21). This one is also used by the interop layer. Static ViewConfigs being a part of the new renderer [replicate](https://github.com/facebook/react-native/blob/a7586947d719a9cd2344ad346d271e7ca900de87/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js#L164-L172) this behavior to match the old rendered. We've never had proper implementation for this on Android. So some events names that worked with the old renderer would not be compatible with the new renderer + Static ViewConfigs. Specifically every event name now must start with `top`. This diff implements event name normalization on Android. Changelog: [Internal] - Update event normalization algorithm to match SVCs. Differential Revision: D50604571 --- .../uimanager/UIManagerModuleConstantsHelper.java | 10 +++++++++- .../UIManagerModuleConstantsHelperTest.kt | 8 ++++---- .../react/renderer/core/EventEmitter.cpp | 15 +++++++++++---- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java index 88d5576a77d8..098b7e15fa97 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java @@ -193,7 +193,15 @@ private static void validateDirectEventNames( } for (String oldKey : keysToNormalize) { Object value = events.get(oldKey); - String newKey = "top" + oldKey.substring(0, 1).toUpperCase() + oldKey.substring(1); + String baseKey = ""; + if (oldKey.startsWith("on")) { + // Drop "on" prefix. + baseKey = oldKey.substring(2); + } else { + // Capitalize first letter. + baseKey = oldKey.substring(0, 1).toUpperCase() + oldKey.substring(1); + } + String newKey = "top" + baseKey; events.put(newKey, value); } } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelperTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelperTest.kt index c4e004242858..2f3e0af7fe33 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelperTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelperTest.kt @@ -32,7 +32,7 @@ class UIManagerModuleConstantsHelperTest { val onClickMap: Map = MapBuilder.builder().put("onClick", "¯\\_(ツ)_/¯").build() UIManagerModuleConstantsHelper.normalizeEventTypes(onClickMap) - assertTrue(onClickMap.containsKey("topOnClick")) + assertTrue(onClickMap.containsKey("topClick")) assertTrue(onClickMap.containsKey("onClick")) } @@ -58,8 +58,8 @@ class UIManagerModuleConstantsHelperTest { "bubbled", "onColorChanged", "captured", "onColorChangedCapture"))) .build() UIManagerModuleConstantsHelper.normalizeEventTypes(nestedObjects) - assertTrue(nestedObjects.containsKey("topOnColorChanged")) - var innerMap = nestedObjects["topOnColorChanged"] as? Map + assertTrue(nestedObjects.containsKey("topColorChanged")) + var innerMap = nestedObjects["topColorChanged"] as? Map assertNotNull(innerMap) assertTrue(innerMap!!.containsKey("phasedRegistrationNames")) var innerInnerMap = innerMap.get("phasedRegistrationNames") as? Map @@ -67,7 +67,7 @@ class UIManagerModuleConstantsHelperTest { assertEquals("onColorChanged", innerInnerMap!!.get("bubbled")) assertEquals("onColorChangedCapture", innerInnerMap.get("captured")) assertTrue(nestedObjects.containsKey("onColorChanged")) - innerMap = nestedObjects.get("topOnColorChanged") as? Map + innerMap = nestedObjects.get("topColorChanged") as? Map assertNotNull(innerMap) assertTrue(innerMap!!.containsKey("phasedRegistrationNames")) innerInnerMap = innerMap.get("phasedRegistrationNames") as? Map diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp index 8c3f9d83bf67..a06d7ba76569 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp @@ -16,6 +16,10 @@ namespace facebook::react { +static bool hasPrefix(const std::string& str, const std::string& prefix) { + return str.compare(0, prefix.length(), prefix) == 0; +} + // TODO(T29874519): Get rid of "top" prefix once and for all. /* * Capitalizes the first letter of the event type and adds "top" prefix if @@ -23,11 +27,14 @@ namespace facebook::react { */ static std::string normalizeEventType(std::string type) { auto prefixedType = std::move(type); - if (prefixedType.find("top", 0) != 0) { - prefixedType.insert(0, "top"); - prefixedType[3] = static_cast(toupper(prefixedType[3])); + if (facebook::react::hasPrefix(prefixedType, "top")) { + return prefixedType; + } + if (facebook::react::hasPrefix(prefixedType, "on")) { + return "top" + prefixedType.substr(2); } - return prefixedType; + prefixedType[0] = static_cast(toupper(prefixedType[0])); + return "top" + prefixedType; } std::mutex& EventEmitter::DispatchMutex() {