From b32c6c2cc1bc566a85a883901dbf5e23b5a75b61 Mon Sep 17 00:00:00 2001 From: Dmitry Rykun Date: Mon, 29 Jan 2024 07:01:26 -0800 Subject: [PATCH] Event name normalization (#42586) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42586 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. The Android that we have is incomplete, it is missing the [*2. If it starts with `on` -> replace `on` with `top`*]. This means that some events names that worked with the old renderer would not be compatible with the new renderer + Static ViewConfigs. Specifically every event names that start with `on`. This diff implements event name normalization on Android. Changelog: [Internal] - Update event normalization algorithm to match SVCs. Reviewed By: cortinico Differential Revision: D50604571 fbshipit-source-id: cef34d8baa2cf31f641be423a16bca7ea9fa20c4 --- .../GenerateViewConfigJs-test.js.snap | 8 ++++---- .../components/GenerateViewConfigJs.js | 4 ++-- .../GenerateViewConfigJs-test.js.snap | 4 ++-- .../UIManagerModuleConstantsHelper.java | 10 +++++++++- .../UIManagerModuleConstantsHelperTest.kt | 8 ++++---- .../react/renderer/core/EventEmitter.cpp | 20 +++++++++++++------ 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/packages/react-native-codegen/e2e/__tests__/components/__snapshots__/GenerateViewConfigJs-test.js.snap b/packages/react-native-codegen/e2e/__tests__/components/__snapshots__/GenerateViewConfigJs-test.js.snap index fcfb25d973f431..d74bb73cc85ea4 100644 --- a/packages/react-native-codegen/e2e/__tests__/components/__snapshots__/GenerateViewConfigJs-test.js.snap +++ b/packages/react-native-codegen/e2e/__tests__/components/__snapshots__/GenerateViewConfigJs-test.js.snap @@ -294,7 +294,7 @@ export const __INTERNAL_VIEW_CONFIG = { uiViewClassName: 'EventPropsNativeComponentView', bubblingEventTypes: { - paperDirectName: { + topPaperDirectName: { phasedRegistrationNames: { captured: 'onChangeCapture', bubbled: 'onChange', @@ -308,7 +308,7 @@ export const __INTERNAL_VIEW_CONFIG = { }, }, - paperBubblingName: { + topPaperBubblingName: { phasedRegistrationNames: { captured: 'onEventBubblingWithPaperNameCapture', bubbled: 'onEventBubblingWithPaperName', @@ -321,11 +321,11 @@ export const __INTERNAL_VIEW_CONFIG = { registrationName: 'onEventDirect', }, - paperDirectName: { + topPaperDirectName: { registrationName: 'onEventDirectWithPaperName', }, - paperBubblingName: { + topPaperBubblingName: { registrationName: 'onOrientationChange', }, }, diff --git a/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js b/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js index c8d96823129b78..f09b38c7885f52 100644 --- a/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js +++ b/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js @@ -197,7 +197,7 @@ function generateBubblingEventInfo( ) { return j.property( 'init', - j.identifier(nameOveride || normalizeInputEventName(event.name)), + j.identifier(normalizeInputEventName(nameOveride || event.name)), j.objectExpression([ j.property( 'init', @@ -221,7 +221,7 @@ function generateDirectEventInfo( ) { return j.property( 'init', - j.identifier(nameOveride || normalizeInputEventName(event.name)), + j.identifier(normalizeInputEventName(nameOveride || event.name)), j.objectExpression([ j.property( 'init', diff --git a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateViewConfigJs-test.js.snap b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateViewConfigJs-test.js.snap index f7728272a897cb..8212d94634c79d 100644 --- a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateViewConfigJs-test.js.snap +++ b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateViewConfigJs-test.js.snap @@ -472,7 +472,7 @@ export const __INTERNAL_VIEW_CONFIG = { uiViewClassName: 'RCTInterfaceOnlyComponent', bubblingEventTypes: { - paperChange: { + topPaperChange: { phasedRegistrationNames: { captured: 'onChangeCapture', bubbled: 'onChange', @@ -481,7 +481,7 @@ export const __INTERNAL_VIEW_CONFIG = { }, directEventTypes: { - paperDirectChange: { + topPaperDirectChange: { registrationName: 'onDirectChange', }, }, 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 88d5576a77d851..098b7e15fa976e 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 c4e004242858cc..2f3e0af7fe335b 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 8c3f9d83bf67f0..bd9a7c0e39a21b 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp @@ -16,18 +16,26 @@ 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 - * necessary (e.g. "layout" becames "topLayout"). + * Replaces "on" with "top" if present. Or capitalizes the first letter and adds + * "top" prefix. E.g. "eventName" becomes "topEventName", "onEventName" also + * becomes "topEventName". */ 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() {