feat(rokt)!: accept only placeholder names in selectPlacements

MIGRATING.md

@@ -20,11 +20,14 @@ To go back, undo these steps: remove `$RNMParticleUseSPM` and the helper call, r
 
 ## Migrating embedded placements to placeholder names
 
-`MParticle.Rokt.selectPlacements` can find each embedded `RoktLayoutView` by its
+`MParticle.Rokt.selectPlacements` finds each embedded `RoktLayoutView` by its
 `placeholderName`, so apps no longer need a ref, `findNodeHandle`, or to wait for
-the view to mount before calling it. This is not a breaking change: the map of
-placeholder names to React tags still works. We recommend moving to names the
-next time you touch the integration.
+the view to mount before calling it.
+
+**Breaking:** the map of placeholder names to React tags has been removed, and
+`placeholders` must be an array of names. A map passed from plain JavaScript logs
+an error, and the placement is requested without embedded views. Earlier 3.x
+releases accept both forms, so you can switch to names before you upgrade.
 
 Before, tag-based:
 
@@ -64,23 +67,18 @@ return <MParticle.RoktLayoutView placeholderName="Location1" />;
 To migrate, remove the ref, the `findNodeHandle` import and any `onLayout`
 handler or timer used to delay the call, then pass an array of placeholder
 names. Each name must match the `placeholderName` of a `RoktLayoutView`, the same
-key the map uses today.
+key the map used.
 
 ### Behavior changes to check
 
 - **A named placeholder that has not mounted yet delays the request by up to 2
   seconds.** The SDK waits for the view, then calls Rokt with the views it has.
   A misspelled or never-rendered name therefore arrives 2 seconds late, and is
   logged as `Cannot resolve placeholder`.
-- **`null` in the map form is resolved by name.** `findNodeHandle` returns `null`
-  before the view mounts; that placeholder was skipped before and is now looked
-  up by its key.
 - **A waiting call can end in `PlacementFailure`.** If `close()` runs, or a newer
   call with the same identifier replaces it, before its placeholders mount, the
   waiting call emits `PlacementFailure` instead of being dropped silently.
 
-The map form is planned for removal in a future major version.
-
 ## Migrating from versions < 3.0.0
 
 `3.0.0` moved iOS to the mParticle Apple SDK **9.x**. Later 3.x releases raised

README.md

@@ -785,8 +785,8 @@ useEffect(() => {
 return <MParticle.RoktLayoutView placeholderName="Location1" />;
 ```
 
-The earlier form, a map of `placeholderName` to `findNodeHandle(ref)`, is still
-supported: `{ Location1: findNodeHandle(this.placeholder1.current) }`.
+The earlier map of `placeholderName` to `findNodeHandle(ref)` is no longer
+supported: see [MIGRATING](./MIGRATING.md#migrating-embedded-placements-to-placeholder-names).
 
 | Method                                                | Notes                                                 |
 | ----------------------------------------------------- | ----------------------------------------------------- |

android/src/main/java/com/mparticle/react/rokt/MPRoktModuleImpl.kt

@@ -9,6 +9,7 @@ import com.facebook.react.bridge.Arguments
 import com.facebook.react.bridge.Promise
 import com.facebook.react.bridge.ReactApplicationContext
 import com.facebook.react.bridge.ReactContext
+import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.bridge.ReadableType
 import com.facebook.react.bridge.UiThreadUtil
@@ -17,13 +18,15 @@ import com.facebook.react.modules.core.DeviceEventManagerModule
 import com.mparticle.MParticle
 import com.mparticle.WrapperSdk
 import com.mparticle.internal.Logger
+import com.mparticle.kits.RoktEmbeddedView
 import com.mparticle.kits.rokt
 import com.rokt.roktsdk.CacheConfig
 import com.rokt.roktsdk.RoktConfig
 import com.rokt.roktsdk.RoktEvent
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.launch
+import java.lang.ref.WeakReference
 import java.math.BigDecimal
 
 class MPRoktModuleImpl(
@@ -74,15 +77,14 @@ class MPRoktModuleImpl(
     }
 
     /**
-     * Runs [select] on the UI thread once every placeholder named for name-based resolution has
-     * mounted, or after [PLACEHOLDER_MOUNT_TIMEOUT_MS]. A placeholder may not be mounted yet when
-     * selectPlacements arrives, e.g. when it is called from the same useEffect that rendered it:
-     * Fabric creates views on the next frame. Legacy react tags are never waited for.
-     * Must be called on the UI thread.
+     * Runs [select] on the UI thread once every named placeholder has mounted, or after
+     * [PLACEHOLDER_MOUNT_TIMEOUT_MS]. A placeholder may not be mounted yet when selectPlacements
+     * arrives, e.g. when it is called from the same useEffect that rendered it: Fabric creates
+     * views on the next frame. Must be called on the UI thread.
      */
     fun whenPlaceholdersMounted(
         identifier: String,
-        placeholders: ReadableMap?,
+        placeholders: ReadableArray?,
         select: () -> Unit,
     ) {
         val pending = unmountedPlaceholderNames(placeholders)
@@ -111,19 +113,37 @@ class MPRoktModuleImpl(
         sendEvent(reactContext, "RoktEvents", params)
     }
 
-    // Names passed for name-based resolution (a non-positive value) that have no mounted view yet.
-    internal fun unmountedPlaceholderNames(placeholders: ReadableMap?): List<String> {
-        if (placeholders == null) return emptyList()
-        val pending = mutableListOf<String>()
-        val iterator = placeholders.keySetIterator()
-        while (iterator.hasNextKey()) {
-            val key = iterator.nextKey()
-            val isReactTag = placeholders.getType(key) == ReadableType.Number && placeholders.getDouble(key) > 0
-            if (!isReactTag && RoktPlaceholderRegistry.lookup(key) == null) {
-                pending += key
+    /**
+     * Resolves placeholder names to their mounted RoktEmbeddedView instances.
+     * Must be called on the UI thread — it resolves live views.
+     */
+    fun resolvePlaceholders(placeholders: ReadableArray?): Map<String, WeakReference<RoktEmbeddedView>> {
+        val names = placeholderNames(placeholders)
+        if (names.size != (placeholders?.size() ?: 0)) {
+            Logger.warning("Ignoring placeholders that are not placeholderName strings")
+        }
+        val views = HashMap<String, WeakReference<RoktEmbeddedView>>()
+        for (name in names) {
+            val view = RoktPlaceholderRegistry.lookup(name) as? RoktEmbeddedView
+            if (view != null) {
+                views[name] = WeakReference(view)
+            } else {
+                Logger.warning("Cannot resolve placeholder for key: $name")
             }
         }
-        return pending
+        return views
+    }
+
+    // Placeholder names that have no mounted view yet.
+    internal fun unmountedPlaceholderNames(placeholders: ReadableArray?): List<String> =
+        placeholderNames(placeholders).filter { RoktPlaceholderRegistry.lookup(it) == null }
+
+    // Only strings are placeholder names; getString would throw on any other entry.
+    private fun placeholderNames(placeholders: ReadableArray?): List<String> {
+        if (placeholders == null) return emptyList()
+        return (0 until placeholders.size()).mapNotNull { index ->
+            if (placeholders.getType(index) == ReadableType.String) placeholders.getString(index) else null
+        }
     }
 
     fun setSessionId(

android/src/main/java/com/mparticle/react/rokt/RoktPlaceholderRegistry.kt

@@ -7,7 +7,7 @@ import java.lang.ref.WeakReference
 
 /**
  * Maps a `RoktLayoutView`'s `placeholderName` to its mounted native view, so
- * `selectPlacements` can resolve placeholders by name instead of a `findNodeHandle` react tag.
+ * `selectPlacements` can resolve placeholders by name.
  *
  * UI thread only: views register from the view manager and are resolved inside
  * `runOnUiThread` / `addUIBlock`, so no locking.

android/src/newarch/java/com/mparticle/react/rokt/MPRoktModule.kt

@@ -3,16 +3,13 @@ package com.mparticle.react.rokt
 import com.facebook.react.bridge.Promise
 import com.facebook.react.bridge.ReactApplicationContext
 import com.facebook.react.bridge.ReactMethod
+import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableMap
-import com.facebook.react.bridge.ReadableType
 import com.facebook.react.bridge.UiThreadUtil
-import com.facebook.react.uimanager.UIManagerHelper
 import com.mparticle.MParticle
 import com.mparticle.internal.Logger
-import com.mparticle.kits.RoktEmbeddedView
 import com.mparticle.kits.rokt
 import com.mparticle.react.NativeMPRoktSpec
-import java.lang.ref.WeakReference
 
 class MPRoktModule(
     private val reactContext: ReactApplicationContext,
@@ -25,7 +22,7 @@ class MPRoktModule(
     override fun selectPlacements(
         identifier: String,
         attributes: ReadableMap?,
-        placeholders: ReadableMap?,
+        placeholders: ReadableArray?,
         roktConfig: ReadableMap?,
         fontFilesMap: ReadableMap?,
     ) {
@@ -43,14 +40,13 @@ class MPRoktModule(
 
         // Rokt SDK 6's selectPlacements clears prior placeholder content via removeAllViews(),
         // which detaches Compose views and must run on the main thread. Resolve the placeholder
-        // views and invoke the SDK together on the UI thread. (oldarch uses UIManager.addUIBlock;
-        // iOS uses the uiManager methodQueue — same intent.)
+        // views and invoke the SDK together on the UI thread, as oldarch's UIManager.addUIBlock does.
         UiThreadUtil.runOnUiThread {
             impl.whenPlaceholdersMounted(identifier, placeholders) {
                 MParticle.getInstance()?.rokt?.selectPlacements(
                     identifier = identifier,
                     attributes = attributeMap,
-                    embeddedViews = resolvePlaceholders(placeholders),
+                    embeddedViews = impl.resolvePlaceholders(placeholders),
                     fontTypefaces = null, // TODO
                     config = config,
                 )
@@ -93,60 +89,4 @@ class MPRoktModule(
     override fun getSessionId(promise: Promise) {
         impl.getSessionId(promise)
     }
-
-    /**
-     * Resolve placeholders to their RoktEmbeddedView instances. A positive numeric value is a
-     * legacy `findNodeHandle` react tag. Zero is the name-lookup sentinel used by the JS wrapper;
-     * unresolved tags also fall back to placeholderName.
-     * Must be called on the UI thread — it resolves live views.
-     */
-    private fun resolvePlaceholders(placeholders: ReadableMap?): Map<String, WeakReference<RoktEmbeddedView>> {
-        val placeholdersMap = HashMap<String, WeakReference<RoktEmbeddedView>>()
-        if (placeholders == null) {
-            return placeholdersMap
-        }
-
-        val iterator = placeholders.keySetIterator()
-        while (iterator.hasNextKey()) {
-            val key = iterator.nextKey()
-            try {
-                val taggedView =
-                    if (
-                        placeholders.getType(key) == ReadableType.Number &&
-                        placeholders.getDouble(key) > 0
-                    ) {
-                        resolveReactTag(placeholders.getDouble(key).toInt())
-                    } else {
-                        null
-                    }
-                val view = taggedView ?: RoktPlaceholderRegistry.lookup(key) as? RoktEmbeddedView
-
-                if (view != null) {
-                    placeholdersMap[key] = WeakReference(view)
-                    Logger.debug("Successfully found Widget for key: $key")
-                } else {
-                    Logger.warning("Cannot resolve placeholder for key: $key")
-                }
-            } catch (e: Exception) {
-                Logger.warning("Error processing placeholder for key $key: ${e.message}")
-            }
-        }
-
-        return placeholdersMap
-    }
-
-    private fun resolveReactTag(reactTag: Int): RoktEmbeddedView? {
-        val uiManager = UIManagerHelper.getUIManagerForReactTag(reactContext, reactTag)
-        if (uiManager == null) {
-            Logger.warning("UIManager not found for tag: $reactTag")
-            return null
-        }
-        // resolveView throws for a tag that is no longer mounted; the caller falls back to the name.
-        val view = runCatching { uiManager.resolveView(reactTag) }.getOrNull()
-        if (view !is RoktEmbeddedView) {
-            Logger.warning("View with tag $reactTag is not a Widget: ${view?.javaClass?.simpleName}")
-            return null
-        }
-        return view
-    }
 }

android/src/oldarch/java/com/mparticle/react/NativeMPRoktSpec.kt

@@ -3,6 +3,7 @@ package com.mparticle.react
 import com.facebook.react.bridge.Promise
 import com.facebook.react.bridge.ReactApplicationContext
 import com.facebook.react.bridge.ReactContextBaseJavaModule
+import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableMap
 
 abstract class NativeMPRoktSpec(
@@ -17,7 +18,7 @@ abstract class NativeMPRoktSpec(
     abstract fun selectPlacements(
         identifier: String,
         attributes: ReadableMap?,
-        placeholders: ReadableMap?,
+        placeholders: ReadableArray?,
         roktConfig: ReadableMap?,
         fontFilesMap: ReadableMap?,
     )

android/src/oldarch/java/com/mparticle/react/rokt/MPRoktModule.kt

@@ -3,15 +3,13 @@ package com.mparticle.react.rokt
 import com.facebook.react.bridge.Promise
 import com.facebook.react.bridge.ReactApplicationContext
 import com.facebook.react.bridge.ReactMethod
+import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableMap
-import com.facebook.react.uimanager.NativeViewHierarchyManager
 import com.facebook.react.uimanager.UIManagerModule
 import com.mparticle.MParticle
 import com.mparticle.internal.Logger
-import com.mparticle.kits.RoktEmbeddedView
 import com.mparticle.kits.rokt
 import com.mparticle.react.NativeMPRoktSpec
-import java.lang.ref.WeakReference
 
 class MPRoktModule(
     private val reactContext: ReactApplicationContext,
@@ -24,7 +22,7 @@ class MPRoktModule(
     override fun selectPlacements(
         identifier: String,
         attributes: ReadableMap?,
-        placeholders: ReadableMap?,
+        placeholders: ReadableArray?,
         roktConfig: ReadableMap?,
         fontFilesMap: ReadableMap?,
     ) {
@@ -39,12 +37,12 @@ class MPRoktModule(
         }
 
         val config = roktConfig?.let { impl.buildRoktConfig(it) }
-        uiManager?.addUIBlock { nativeViewHierarchyManager ->
+        uiManager?.addUIBlock {
             impl.whenPlaceholdersMounted(identifier, placeholders) {
                 MParticle.getInstance()?.rokt?.selectPlacements(
                     identifier = identifier,
                     attributes = impl.readableMapToMapOfStrings(attributes),
-                    embeddedViews = safeUnwrapPlaceholders(placeholders, nativeViewHierarchyManager),
+                    embeddedViews = impl.resolvePlaceholders(placeholders),
                     fontTypefaces = null, // TODO
                     config = config,
                 )
@@ -87,27 +85,4 @@ class MPRoktModule(
     override fun getSessionId(promise: Promise) {
         impl.getSessionId(promise)
     }
-
-    // Positive numeric values are legacy react tags. Zero is the name-lookup sentinel used by
-    // the JS wrapper; unresolved tags also fall back to placeholderName.
-    private fun safeUnwrapPlaceholders(
-        placeholders: ReadableMap?,
-        nativeViewHierarchyManager: NativeViewHierarchyManager,
-    ): Map<String, WeakReference<RoktEmbeddedView>> {
-        val placeholderMap: MutableMap<String, WeakReference<RoktEmbeddedView>> = HashMap()
-
-        // A for loop, not forEach: HashMap.forEach(BiConsumer) needs API 24 and minSdk is 21.
-        for ((key, value) in placeholders?.toHashMap().orEmpty()) {
-            val view =
-                (value as? Double)?.takeIf { it > 0 }?.let {
-                    runCatching { nativeViewHierarchyManager.resolveView(it.toInt()) as? RoktEmbeddedView }.getOrNull()
-                } ?: RoktPlaceholderRegistry.lookup(key) as? RoktEmbeddedView
-            if (view != null) {
-                placeholderMap[key] = WeakReference(view)
-            } else {
-                Logger.warning("Cannot resolve placeholder for key: $key")
-            }
-        }
-        return placeholderMap
-    }
 }

android/src/test/java/com/mparticle/react/rokt/MPRoktModuleImplTest.kt

@@ -1,5 +1,6 @@
 package com.mparticle.react.rokt
 
+import com.facebook.react.bridge.JavaOnlyArray
 import com.facebook.react.bridge.ReactApplicationContext
 import com.mparticle.MParticle
 import com.mparticle.WrapperSdk
@@ -92,11 +93,11 @@ class MPRoktModuleImplTest {
     }
 
     @Test
-    fun `unmountedPlaceholderNames waits only for names, never for react tags`() {
+    fun `unmountedPlaceholderNames waits only for unmounted placeholderName strings`() {
         val mounted = Mockito.mock(android.view.View::class.java)
         RoktPlaceholderRegistry.register(mounted, "Mounted")
         try {
-            val placeholders = MockMap(mapOf("Location1" to 0.0, "Mounted" to 0.0, "Tagged" to 42.0))
+            val placeholders = JavaOnlyArray.of("Location1", "Mounted", 42.0)
 
             assertEquals(listOf("Location1"), impl.unmountedPlaceholderNames(placeholders))
             assertEquals(emptyList<String>(), impl.unmountedPlaceholderNames(null))