Fix up UIManagerModuleConstantsHelper + test (facebook#45877)

Summary: Pull Request resolved: facebook#45877 A couple fixes: - Test was mixing up event names - refactored to make the code [hopefully] clearer - Code potentially recreates `events` if it's a singleton map, but that was lost due to it not being returned Changelog: [Internal] Minor fix to UIManagerModuleConstantsHelper Reviewed By: cortinico Differential Revision: D60609294 fbshipit-source-id: 3c82ba30b9401674e678585b1612324f885c9ae1
rshest · Aug 2, 2024 · 8d4fd07 · 8d4fd07
1 parent a696d2e
commit 8d4fd07
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 30 deletions.
diff --git a/...actAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java b/...actAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelper.java
@@ -53,7 +53,7 @@ public static Map<String, Object> getDefaultExportableEventTypes() {
   }
 
   private static void validateDirectEventNames(
-      String viewManagerName, Map<String, Object> directEvents) {
+      String viewManagerName, @Nullable Map<String, Object> directEvents) {
     if (!ReactBuildConfig.DEBUG || directEvents == null) {
       return;
     }
@@ -139,7 +139,7 @@ private static void validateDirectEventNames(
         // For Fabric, events needs to be fired with a "top" prefix.
         // For the sake of Fabric Interop, here we normalize events adding "top" in their
         // name if the user hasn't provided it.
-        normalizeEventTypes(viewManagerBubblingEvents);
+        viewManagerBubblingEvents = normalizeEventTypes(viewManagerBubblingEvents);
       }
       recursiveMerge(cumulativeBubblingEventTypes, viewManagerBubblingEvents);
       recursiveMerge(viewManagerBubblingEvents, defaultBubblingEvents);
@@ -155,7 +155,7 @@ private static void validateDirectEventNames(
         // For Fabric, events needs to be fired with a "top" prefix.
         // For the sake of Fabric Interop, here we normalize events adding "top" in their
         // name if the user hasn't provided it.
-        normalizeEventTypes(viewManagerDirectEvents);
+        viewManagerDirectEvents = normalizeEventTypes(viewManagerDirectEvents);
       }
       recursiveMerge(cumulativeDirectEventTypes, viewManagerDirectEvents);
       recursiveMerge(viewManagerDirectEvents, defaultDirectEvents);
@@ -181,9 +181,9 @@ private static void validateDirectEventNames(
   }
 
   @VisibleForTesting
-  /* package */ static void normalizeEventTypes(@Nullable Map events) {
+  /* package */ static @Nullable Map normalizeEventTypes(@Nullable Map events) {
     if (events == null) {
-      return;
+      return null;
     }
     Set<String> keysToNormalize = new HashSet<>();
     for (Object key : events.keySet()) {
@@ -212,6 +212,7 @@ private static void validateDirectEventNames(
       String newKey = "top" + baseKey;
       events.put(newKey, value);
     }
+    return events;
   }
 
   /** Merges {@param source} map into {@param dest} map recursively */

diff --git a/...tAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelperTest.kt b/...tAndroid/src/test/java/com/facebook/react/uimanager/UIManagerModuleConstantsHelperTest.kt
@@ -13,28 +13,28 @@ import org.junit.Test
 class UIManagerModuleConstantsHelperTest {
   @Test
   fun normalizeEventTypes_withNull_doesNothing() {
-    UIManagerModuleConstantsHelper.normalizeEventTypes(null)
+    assertThat(UIManagerModuleConstantsHelper.normalizeEventTypes(null)).isNull()
   }
 
   @Test
   fun normalizeEventTypes_withEmptyMap_doesNothing() {
     val emptyMap = mutableMapOf<String, Any?>()
-    UIManagerModuleConstantsHelper.normalizeEventTypes(emptyMap)
-    assertThat(emptyMap.isEmpty()).isTrue()
+    assertThat(UIManagerModuleConstantsHelper.normalizeEventTypes(emptyMap)).isEmpty()
   }
 
   @Test
   fun normalizeEventTypes_withOnEvent_doesNormalize() {
     val onClickMap = mutableMapOf("onClick" to "¯\\_(ツ)_/¯")
-    UIManagerModuleConstantsHelper.normalizeEventTypes(onClickMap)
-    assertThat(onClickMap).containsKeys("topClick", "onClick")
+    assertThat(UIManagerModuleConstantsHelper.normalizeEventTypes(onClickMap))
+        .containsKeys("topClick", "onClick")
   }
 
   @Test
   fun normalizeEventTypes_withTopEvent_doesNormalize() {
     val onClickMap = mutableMapOf("topOnClick" to "¯\\_(ツ)_/¯")
-    UIManagerModuleConstantsHelper.normalizeEventTypes(onClickMap)
-    assertThat(onClickMap).containsKey("topOnClick").doesNotContainKey("onClick")
+    assertThat(UIManagerModuleConstantsHelper.normalizeEventTypes(onClickMap))
+        .containsKey("topOnClick")
+        .doesNotContainKey("onClick")
   }
 
   @Suppress("UNCHECKED_CAST")
@@ -49,25 +49,27 @@ class UIManagerModuleConstantsHelperTest {
                             "bubbled" to "onColorChanged",
                             "captured" to "onColorChangedCapture",
                         )))
-    UIManagerModuleConstantsHelper.normalizeEventTypes(nestedObjects)
-    assertThat(nestedObjects).containsKey("topColorChanged")
-    var innerMap = nestedObjects["topColorChanged"]
-    assertThat(innerMap).isNotNull()
-    requireNotNull(innerMap)
-    assertThat(innerMap).containsKey("phasedRegistrationNames")
-    var innerInnerMap = innerMap["phasedRegistrationNames"]
-    assertThat(innerInnerMap).isNotNull()
-    requireNotNull(innerInnerMap)
-    assertThat("onColorChanged").isEqualTo(innerInnerMap["bubbled"])
-    assertThat("onColorChangedCapture").isEqualTo(innerInnerMap["captured"])
-    assertThat(nestedObjects).containsKey("onColorChanged")
-    innerMap = nestedObjects["topColorChanged"]
-    assertThat(innerMap).isNotNull()
-    requireNotNull(innerMap)
+    val result =
+        checkNotNull(
+            UIManagerModuleConstantsHelper.normalizeEventTypes(nestedObjects)
+                as Map<String, Map<String, Map<String, String>>>) {
+              "returned map was null"
+            }
+    verifyNestedObjects(result, "topColorChanged")
+    verifyNestedObjects(result, "onColorChanged")
+  }
+
+  private fun verifyNestedObjects(
+      nestedObjects: Map<String, Map<String, Map<String, String>>>,
+      name: String,
+  ) {
+    assertThat(nestedObjects).containsKey(name)
+    val innerMap = checkNotNull(nestedObjects[name]) { """nestedObjects["$name"] is null""" }
     assertThat(innerMap).containsKey("phasedRegistrationNames")
-    innerInnerMap = innerMap["phasedRegistrationNames"]
-    assertThat(innerInnerMap).isNotNull()
-    requireNotNull(innerInnerMap)
+    val innerInnerMap =
+        checkNotNull(innerMap["phasedRegistrationNames"]) {
+          """nestedObjects["$name"]["phasedRegistrationNames"] is null"""
+        }
     assertThat("onColorChanged").isEqualTo(innerInnerMap["bubbled"])
     assertThat("onColorChangedCapture").isEqualTo(innerInnerMap["captured"])
   }