Skip to content

Commit

Permalink
Event name normalization (#42586)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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
  • Loading branch information
dmytrorykun authored and lunaleaps committed Feb 4, 2024
1 parent 0d2c505 commit 45ba042
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export const __INTERNAL_VIEW_CONFIG = {
uiViewClassName: 'EventPropsNativeComponentView',
bubblingEventTypes: {
paperDirectName: {
topPaperDirectName: {
phasedRegistrationNames: {
captured: 'onChangeCapture',
bubbled: 'onChange',
Expand All @@ -308,7 +308,7 @@ export const __INTERNAL_VIEW_CONFIG = {
},
},
paperBubblingName: {
topPaperBubblingName: {
phasedRegistrationNames: {
captured: 'onEventBubblingWithPaperNameCapture',
bubbled: 'onEventBubblingWithPaperName',
Expand All @@ -321,11 +321,11 @@ export const __INTERNAL_VIEW_CONFIG = {
registrationName: 'onEventDirect',
},
paperDirectName: {
topPaperDirectName: {
registrationName: 'onEventDirectWithPaperName',
},
paperBubblingName: {
topPaperBubblingName: {
registrationName: 'onOrientationChange',
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ export const __INTERNAL_VIEW_CONFIG = {
uiViewClassName: 'RCTInterfaceOnlyComponent',
bubblingEventTypes: {
paperChange: {
topPaperChange: {
phasedRegistrationNames: {
captured: 'onChangeCapture',
bubbled: 'onChange',
Expand All @@ -477,7 +477,7 @@ export const __INTERNAL_VIEW_CONFIG = {
},
directEventTypes: {
paperDirectChange: {
topPaperDirectChange: {
registrationName: 'onDirectChange',
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class UIManagerModuleConstantsHelperTest {
val onClickMap: Map<String, String> =
MapBuilder.builder<String, String>().put("onClick", "¯\\_(ツ)_/¯").build()
UIManagerModuleConstantsHelper.normalizeEventTypes(onClickMap)
assertTrue(onClickMap.containsKey("topOnClick"))
assertTrue(onClickMap.containsKey("topClick"))
assertTrue(onClickMap.containsKey("onClick"))
}

Expand All @@ -58,16 +58,16 @@ class UIManagerModuleConstantsHelperTest {
"bubbled", "onColorChanged", "captured", "onColorChangedCapture")))
.build()
UIManagerModuleConstantsHelper.normalizeEventTypes(nestedObjects)
assertTrue(nestedObjects.containsKey("topOnColorChanged"))
var innerMap = nestedObjects["topOnColorChanged"] as? Map<String, Any?>
assertTrue(nestedObjects.containsKey("topColorChanged"))
var innerMap = nestedObjects["topColorChanged"] as? Map<String, Any?>
assertNotNull(innerMap)
assertTrue(innerMap!!.containsKey("phasedRegistrationNames"))
var innerInnerMap = innerMap.get("phasedRegistrationNames") as? Map<String, Any?>
assertNotNull(innerInnerMap)
assertEquals("onColorChanged", innerInnerMap!!.get("bubbled"))
assertEquals("onColorChangedCapture", innerInnerMap.get("captured"))
assertTrue(nestedObjects.containsKey("onColorChanged"))
innerMap = nestedObjects.get("topOnColorChanged") as? Map<String, Any?>
innerMap = nestedObjects.get("topColorChanged") as? Map<String, Any?>
assertNotNull(innerMap)
assertTrue(innerMap!!.containsKey("phasedRegistrationNames"))
innerInnerMap = innerMap.get("phasedRegistrationNames") as? Map<String, Any?>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<char>(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<char>(toupper(prefixedType[0]));
return "top" + prefixedType;
}

std::mutex& EventEmitter::DispatchMutex() {
Expand Down

0 comments on commit 45ba042

Please sign in to comment.