From 046c618179e831ca0fbe2ed3d7dbb291b5cf2d67 Mon Sep 17 00:00:00 2001 From: Thomson Thomas Date: Wed, 30 Sep 2026 10:49:43 -0400 Subject: [PATCH] feat(rokt)!: accept only placeholder names in selectPlacements selectPlacements now takes an array of RoktLayoutView placeholderNames only. The map of placeholder name to findNodeHandle react tag is removed, together with the native view-tag lookups behind it (viewRegistry_DEPRECATED on iOS, UIManager.resolveView and NativeViewHierarchyManager on Android). The native module interface now declares placeholders as Array. A map passed from plain JavaScript logs an error, and the placement is requested without embedded views, so every platform behaves the same instead of Android throwing and iOS dropping or misreading the argument. Both native modules skip non-string entries. The Android name lookup is shared by both architectures in MPRoktModuleImpl. BREAKING CHANGE: selectPlacements no longer accepts { [placeholderName]: findNodeHandle(ref) }. Pass ['placeholderName'] instead; earlier 3.x releases accept both forms, so apps can switch before upgrading. Co-Authored-By: Claude Opus 5.5 (1M context) --- MIGRATING.md | 18 ++-- README.md | 4 +- .../mparticle/react/rokt/MPRoktModuleImpl.kt | 54 ++++++++---- .../react/rokt/RoktPlaceholderRegistry.kt | 2 +- .../com/mparticle/react/rokt/MPRoktModule.kt | 68 +-------------- .../com/mparticle/react/NativeMPRoktSpec.kt | 3 +- .../com/mparticle/react/rokt/MPRoktModule.kt | 33 +------ .../react/rokt/MPRoktModuleImplTest.kt | 5 +- ios/RNMParticle/RNMPRokt.mm | 63 ++++---------- ios/RNMParticle/RoktPlaceholderRegistry.h | 2 +- js/__tests__/rokt-placeholders.test.ts | 35 +++++--- js/codegenSpecs/rokt/NativeMPRokt.ts | 2 +- js/rokt/rokt.ts | 38 ++++----- .../RNMPRoktPlaceholderTests.m | 85 +++++++------------ 14 files changed, 149 insertions(+), 263 deletions(-) diff --git a/MIGRATING.md b/MIGRATING.md index bc3ab56a..fe36472c 100644 --- a/MIGRATING.md +++ b/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,7 +67,7 @@ return ; 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 @@ -72,15 +75,10 @@ key the map uses today. 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 diff --git a/README.md b/README.md index aec8be55..653b6035 100644 --- a/README.md +++ b/README.md @@ -785,8 +785,8 @@ useEffect(() => { return ; ``` -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 | | ----------------------------------------------------- | ----------------------------------------------------- | diff --git a/android/src/main/java/com/mparticle/react/rokt/MPRoktModuleImpl.kt b/android/src/main/java/com/mparticle/react/rokt/MPRoktModuleImpl.kt index 1f65f1a1..46b9d22a 100644 --- a/android/src/main/java/com/mparticle/react/rokt/MPRoktModuleImpl.kt +++ b/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,6 +18,7 @@ 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 @@ -24,6 +26,7 @@ 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 { - if (placeholders == null) return emptyList() - val pending = mutableListOf() - 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> { + val names = placeholderNames(placeholders) + if (names.size != (placeholders?.size() ?: 0)) { + Logger.warning("Ignoring placeholders that are not placeholderName strings") + } + val views = HashMap>() + 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 = + 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 { + 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( diff --git a/android/src/main/java/com/mparticle/react/rokt/RoktPlaceholderRegistry.kt b/android/src/main/java/com/mparticle/react/rokt/RoktPlaceholderRegistry.kt index fc642fe8..a0688dc4 100644 --- a/android/src/main/java/com/mparticle/react/rokt/RoktPlaceholderRegistry.kt +++ b/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. diff --git a/android/src/newarch/java/com/mparticle/react/rokt/MPRoktModule.kt b/android/src/newarch/java/com/mparticle/react/rokt/MPRoktModule.kt index 4ccc2bca..9dd4d3bb 100644 --- a/android/src/newarch/java/com/mparticle/react/rokt/MPRoktModule.kt +++ b/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> { - val placeholdersMap = HashMap>() - 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 - } } diff --git a/android/src/oldarch/java/com/mparticle/react/NativeMPRoktSpec.kt b/android/src/oldarch/java/com/mparticle/react/NativeMPRoktSpec.kt index fa29fdf0..0719dd5a 100644 --- a/android/src/oldarch/java/com/mparticle/react/NativeMPRoktSpec.kt +++ b/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?, ) diff --git a/android/src/oldarch/java/com/mparticle/react/rokt/MPRoktModule.kt b/android/src/oldarch/java/com/mparticle/react/rokt/MPRoktModule.kt index 6f65a20c..3efca7a6 100644 --- a/android/src/oldarch/java/com/mparticle/react/rokt/MPRoktModule.kt +++ b/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> { - val placeholderMap: MutableMap> = 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 - } } diff --git a/android/src/test/java/com/mparticle/react/rokt/MPRoktModuleImplTest.kt b/android/src/test/java/com/mparticle/react/rokt/MPRoktModuleImplTest.kt index 156266df..80a85204 100644 --- a/android/src/test/java/com/mparticle/react/rokt/MPRoktModuleImplTest.kt +++ b/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(), impl.unmountedPlaceholderNames(null)) diff --git a/ios/RNMParticle/RNMPRokt.mm b/ios/RNMParticle/RNMPRokt.mm index 64af67d1..849c98ae 100644 --- a/ios/RNMParticle/RNMPRokt.mm +++ b/ios/RNMParticle/RNMPRokt.mm @@ -12,7 +12,6 @@ #import "RoktPlaceholderRegistry.h" #ifdef RCT_NEW_ARCH_ENABLED -#import "RoktNativeLayoutComponentView.h" #import #endif // RCT_NEW_ARCH_ENABLED @@ -46,9 +45,6 @@ @interface RNMPRokt () @implementation RNMPRokt -// Maps React tags to UIViews in both bridge and bridgeless modes, unlike bridge.uiManager. -@synthesize viewRegistry_DEPRECATED = _viewRegistry_DEPRECATED; - RCT_EXTERN void RCTRegisterModule(Class); + (NSString *)moduleName { @@ -112,7 +108,7 @@ - (void)ensureEventManager { // New Architecture Implementation — selectPlacements - (void)selectPlacements:(NSString *)identifer attributes:(NSDictionary *)attributes - placeholders:(NSDictionary *)placeholders + placeholders:(NSArray *)placeholders roktConfig:(JS::NativeMPRokt::RoktConfigType &)roktConfig fontFilesMap:(NSDictionary *)fontFilesMap { @@ -123,7 +119,7 @@ - (void)selectPlacements:(NSString *)identifer RoktConfig *config = [RNMPRoktConfigFactory configFromDictionary:roktConfigDict]; #else // Old Architecture Implementation — selectPlacements -RCT_EXPORT_METHOD(selectPlacements:(NSString *) identifer attributes:(NSDictionary *)attributes placeholders:(NSDictionary * _Nullable)placeholders roktConfig:(NSDictionary * _Nullable)roktConfig fontFilesMap:(NSDictionary * _Nullable)fontFilesMap) +RCT_EXPORT_METHOD(selectPlacements:(NSString *) identifer attributes:(NSDictionary *)attributes placeholders:(NSArray * _Nullable)placeholders roktConfig:(NSDictionary * _Nullable)roktConfig fontFilesMap:(NSDictionary * _Nullable)fontFilesMap) { _rokt_log(@"[mParticle-Rokt] Old Architecture Implementation"); NSMutableDictionary *finalAttributes = [self convertToMutableDictionaryOfStrings:attributes]; @@ -311,65 +307,42 @@ - (void)getSessionIdWithResolve:(RCTPromiseResolveBlock)resolve return finalAttributes; } -// Main thread only — RCTViewRegistry and RoktPlaceholderRegistry read the mounted view hierarchy. -// 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. -- (NSMutableDictionary *)resolvePlaceholders:(NSDictionary *)placeholders +// Main thread only — RoktPlaceholderRegistry reads the mounted view hierarchy. +- (NSMutableDictionary *)resolvePlaceholders:(NSArray *)placeholders { _rokt_log(@"[mParticle-Rokt] resolvePlaceholders: %lu placeholder(s)", (unsigned long)placeholders.count); NSMutableDictionary *nativePlaceholders = [[NSMutableDictionary alloc]initWithCapacity:placeholders.count]; - for(id key in placeholders){ - id reactTag = [placeholders objectForKey:key]; - UIView *embeddedView = nil; - if ([reactTag isKindOfClass:[NSNumber class]] && [reactTag integerValue] > 0) { - embeddedView = [self embeddedViewForReactTag:reactTag]; - } - if (embeddedView == nil && [key isKindOfClass:[NSString class]]) { - UIView *view = [RoktPlaceholderRegistry viewForName:key]; - // nil is not an embedded view, covering both "not mounted" and "wrong class". - if ([RNMPRoktViews isEmbeddedView:view]) { - embeddedView = view; - } + for (id name in placeholders) { + if (![name isKindOfClass:[NSString class]]) { + RCTLogError(@"Cannot resolve placeholder %@: expected a placeholderName string", name); + continue; } - if (embeddedView == nil) { - RCTLogError(@"Cannot resolve placeholder %@ (value %@)", key, reactTag); + UIView *view = [RoktPlaceholderRegistry viewForName:name]; + // nil is not an embedded view, covering both "not mounted" and "wrong class". + if (![RNMPRoktViews isEmbeddedView:view]) { + RCTLogError(@"Cannot resolve placeholder %@", name); continue; } - nativePlaceholders[key] = embeddedView; + nativePlaceholders[name] = view; } _rokt_log(@"[mParticle-Rokt] resolvePlaceholders: resolved %lu native placeholder(s)", (unsigned long)nativePlaceholders.count); return nativePlaceholders; } -// Names passed for name-based resolution (a non-positive value) that have no mounted view yet. -// Legacy react tags are never waited for, so they behave exactly as before. -+ (NSArray *)unmountedPlaceholderNames:(NSDictionary *)placeholders +// Placeholder names that have no mounted view yet. ++ (NSArray *)unmountedPlaceholderNames:(NSArray *)placeholders { NSMutableArray *pending = [NSMutableArray array]; - for (id key in placeholders) { - id value = placeholders[key]; - BOOL isReactTag = [value isKindOfClass:[NSNumber class]] && [value integerValue] > 0; - if (!isReactTag && [key isKindOfClass:[NSString class]] && [RoktPlaceholderRegistry viewForName:key] == nil) { - [pending addObject:key]; + for (id name in placeholders) { + if ([name isKindOfClass:[NSString class]] && [RoktPlaceholderRegistry viewForName:name] == nil) { + [pending addObject:name]; } } return pending; } -- (nullable UIView *)embeddedViewForReactTag:(NSNumber *)reactTag -{ - UIView *view = [_viewRegistry_DEPRECATED viewForReactTag:reactTag]; -#ifdef RCT_NEW_ARCH_ENABLED - return [view isKindOfClass:[RoktNativeLayoutComponentView class]] - ? ((RoktNativeLayoutComponentView *)view).roktEmbeddedView - : nil; -#else - return [RNMPRoktViews isEmbeddedView:view] ? view : nil; -#endif // RCT_NEW_ARCH_ENABLED -} - #ifdef RCT_NEW_ARCH_ENABLED - (std::shared_ptr)getTurboModule:(const facebook::react::ObjCTurboModule::InitParams &)params { return std::make_shared(params); diff --git a/ios/RNMParticle/RoktPlaceholderRegistry.h b/ios/RNMParticle/RoktPlaceholderRegistry.h index a95cff66..e391b426 100644 --- a/ios/RNMParticle/RoktPlaceholderRegistry.h +++ b/ios/RNMParticle/RoktPlaceholderRegistry.h @@ -4,7 +4,7 @@ NS_ASSUME_NONNULL_BEGIN /** * Maps a `RoktLayoutView`'s `placeholderName` to its mounted `RoktEmbeddedView`, so - * `selectPlacements` can resolve placeholders by name instead of a `findNodeHandle` react tag. + * `selectPlacements` can resolve placeholders by name. * * Main thread only. Plain Objective-C so the legacy `.m` view manager can import it. */ diff --git a/js/__tests__/rokt-placeholders.test.ts b/js/__tests__/rokt-placeholders.test.ts index a2aca0ed..7073f826 100644 --- a/js/__tests__/rokt-placeholders.test.ts +++ b/js/__tests__/rokt-placeholders.test.ts @@ -1,7 +1,7 @@ /** - * `selectPlacements` accepts placeholder names (`['Location1']`) or the legacy map of - * name to `findNodeHandle` react tag. The native spec only knows the map shape, so the - * name form is sent with invalid React tag zero, which native resolves by `placeholderName`. + * `selectPlacements` accepts only placeholder names (`['Location1']`). The legacy map of name + * to `findNodeHandle` react tag is rejected in JS with an error log, so every platform behaves + * the same: the placement is requested without embedded views. */ jest.mock( 'react-native', @@ -16,22 +16,29 @@ jest.mock( import { toNativePlaceholders } from '../rokt/rokt'; describe('toNativePlaceholders', () => { - it('maps placeholder names to zero for name-based resolution', () => { - expect(toNativePlaceholders(['Location1', 'Location2'])).toEqual({ - Location1: 0, - Location2: 0, - }); + afterEach(() => { + jest.restoreAllMocks(); }); - it('preserves legacy react tags and normalizes null entries', () => { - const legacy = { Location1: 42, Location2: null }; - expect(toNativePlaceholders(legacy)).toEqual({ - Location1: 42, - Location2: 0, - }); + it('passes placeholder names through unchanged', () => { + const names = ['Location1', 'Location2']; + expect(toNativePlaceholders(names)).toBe(names); }); it('passes undefined through for overlay placements', () => { expect(toNativePlaceholders(undefined)).toBeUndefined(); }); + + it('rejects the legacy map of name to react tag with an error log', () => { + const error = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + const legacy = { Location1: 42 } as unknown as string[]; + + expect(toNativePlaceholders(legacy)).toBeUndefined(); + expect(error).toHaveBeenCalledTimes(1); + expect(error.mock.calls[0][0]).toContain( + 'array of RoktLayoutView placeholderNames' + ); + }); }); diff --git a/js/codegenSpecs/rokt/NativeMPRokt.ts b/js/codegenSpecs/rokt/NativeMPRokt.ts index cfce3dfd..5dbb4850 100644 --- a/js/codegenSpecs/rokt/NativeMPRokt.ts +++ b/js/codegenSpecs/rokt/NativeMPRokt.ts @@ -19,7 +19,7 @@ export interface Spec extends TurboModule { selectPlacements( identifier: string, attributes?: { [key: string]: RoktAttributeValue }, - placeholders?: { [key: string]: number }, + placeholders?: Array, roktConfig?: RoktConfigType, fontFilesMap?: { [key: string]: string } ): void; diff --git a/js/rokt/rokt.ts b/js/rokt/rokt.ts index ec97ef19..3c0c849d 100644 --- a/js/rokt/rokt.ts +++ b/js/rokt/rokt.ts @@ -32,34 +32,28 @@ function getMPRokt(): NativeMPRoktInterface { export type RoktAttributeValue = string | number | boolean; /** - * Embedded placeholders for `selectPlacements`. - * - * Preferred: the `placeholderName`s of `RoktLayoutView`s, e.g. `['Location1']`. - * Legacy: a map of placeholder name to `findNodeHandle(ref)` react tag. Still supported. + * Embedded placeholders for `selectPlacements`: the `placeholderName`s of `RoktLayoutView`s, + * e.g. `['Location1']`. */ -export type RoktPlaceholders = string[] | Record; +export type RoktPlaceholders = string[]; /** - * Converts the public placeholder forms to the native spec's map shape. React tags are - * positive, so zero is an explicit request to resolve the view by its `placeholderName`. - * A numeric sentinel is required because React Native codegen drops null-valued map entries. + * The map of placeholder name to `findNodeHandle` react tag is no longer supported. A plain-JS + * caller that still passes one gets an error log, and the placement is requested without + * embedded views, instead of a platform-specific crash or conversion failure in native code. */ export function toNativePlaceholders( placeholders?: RoktPlaceholders -): Record | undefined { - if (placeholders == null) { - return undefined; +): string[] | undefined { + if (placeholders == null || Array.isArray(placeholders)) { + return placeholders; } - - const entries: ReadonlyArray = - Array.isArray(placeholders) - ? placeholders.map(name => [name, null] as const) - : Object.entries(placeholders); - - return entries.reduce>((map, [name, reactTag]) => { - map[name] = reactTag ?? 0; - return map; - }, {}); + console.error( + '[mParticle] selectPlacements: placeholders must be an array of RoktLayoutView placeholderNames, ' + + "e.g. ['Location1']. The map of name to findNodeHandle tag is no longer supported, so the " + + 'placement is requested without embedded views. See MIGRATING.md.' + ); + return undefined; } /** @@ -83,7 +77,7 @@ export abstract class Rokt { * * @param {string} identifier - The page identifier for the placement. * @param {Record} attributes - Attributes to be associated with the placement. - * @param {RoktPlaceholders} [placeholders] - Optional embedded placeholders: `placeholderName`s of `RoktLayoutView`s (preferred), or a legacy map of name to react tag. A named view does not need to be mounted yet: the SDK waits up to 2 seconds for it, so this can be called from the same `useEffect` that renders it. + * @param {RoktPlaceholders} [placeholders] - Optional embedded placeholders: the `placeholderName`s of `RoktLayoutView`s, e.g. `['Location1']`. A named view does not need to be mounted yet: the SDK waits up to 2 seconds for it, so this can be called from the same `useEffect` that renders it. * @param {IRoktConfig} [roktConfig] - Optional configuration settings for Rokt. * @param {Record} [fontFilesMap] - Optional mapping of font files. * @returns {Promise} A promise that resolves when the placement request is sent. diff --git a/sample/ios/MParticleSampleTests/RNMPRoktPlaceholderTests.m b/sample/ios/MParticleSampleTests/RNMPRoktPlaceholderTests.m index 008dfda3..edf30a20 100644 --- a/sample/ios/MParticleSampleTests/RNMPRoktPlaceholderTests.m +++ b/sample/ios/MParticleSampleTests/RNMPRoktPlaceholderTests.m @@ -1,5 +1,4 @@ #import -#import #import #import #import "../../../ios/RNMParticle/RNMPSDKImports.h" @@ -8,20 +7,13 @@ // Implemented in RNMPRokt.mm. @interface RNMPRokt (PlaceholderTests) -- (NSMutableDictionary *)resolvePlaceholders:(NSDictionary *)placeholders; +- (NSMutableDictionary *)resolvePlaceholders:(NSArray *)placeholders; ++ (NSArray *)unmountedPlaceholderNames:(NSArray *)placeholders; @end /** - * Guards how `-[RNMPRokt resolvePlaceholders:]` turns placeholder react tags into the - * embedded views handed to `MPRokt selectPlacements`. - * - * Resolution goes through `RCTViewRegistry` rather than the legacy - * `self.bridge.uiManager addUIBlock:` view registry, because the latter is a no-op method - * body when RCT_REMOVE_LEGACY_ARCH is defined (React Native 0.84's default) and a no-op - * when `self.bridge` is nil — either way selectPlacements was discarded with no event - * emitted. The registry is exercised for real here: the tests install a bridgeless - * component-view provider, the same hook RCTInstance wires to the surface presenter in - * production. + * Guards how `-[RNMPRokt resolvePlaceholders:]` turns placeholder names into the embedded + * views handed to `MPRokt selectPlacements`, looking each name up in RoktPlaceholderRegistry. * * Scope limit, deliberate: this test target statically links the react-native-mparticle * pod a second time on top of the app it hosts, so `RoktNativeLayoutComponentView` and @@ -30,18 +22,13 @@ - (NSMutableDictionary *)resolvePlaceholders:(NSDictionary *)placeholders; * warning). Asserting a mounted placeholder resolves all the way to its `RoktEmbeddedView` * would therefore be testing the linkage, not the code. That step is verified by running * an embedded placement in the sample app instead. What is covered below is - * binary-independent: that the module is wired to a working registry, and every branch - * that refuses to resolve a tag. + * binary-independent: every branch that refuses to resolve a placeholder. */ @interface RNMPRoktPlaceholderTests : XCTestCase @end @implementation RNMPRoktPlaceholderTests { RNMPRokt *_rokt; - // viewRegistry_DEPRECATED is a weak property (React Native retains the registry via - // RCTBridgeModuleDecorator for the instance's lifetime), so the test has to own it. - RCTViewRegistry *_viewRegistry; - NSMutableDictionary *_views; NSInteger _loggedErrorCount; NSMutableArray *_loggedErrors; RCTLogFunction _originalLogFunction; @@ -51,21 +38,13 @@ - (void)setUp { [super setUp]; _rokt = [RNMPRokt new]; - _views = [NSMutableDictionary new]; - - _viewRegistry = [RCTViewRegistry new]; - __weak __typeof__(self) weakSelf = self; - [_viewRegistry setBridgelessComponentViewProvider:^UIView *(NSNumber *reactTag) { - __strong __typeof__(weakSelf) strongSelf = weakSelf; - return strongSelf ? strongSelf->_views[reactTag] : nil; - }]; - _rokt.viewRegistry_DEPRECATED = _viewRegistry; // Unresolvable placeholders are reported via RCTLogError. Capture instead of letting // it surface as test noise, so the diagnostic itself can be asserted. _loggedErrorCount = 0; _loggedErrors = [NSMutableArray new]; _originalLogFunction = RCTGetLogFunction(); + __weak __typeof__(self) weakSelf = self; RCTSetLogFunction(^(RCTLogLevel level, __unused RCTLogSource source, __unused NSString *fileName, @@ -84,49 +63,36 @@ - (void)tearDown [RoktPlaceholderRegistry cancelAllWaits]; RCTSetLogFunction(_originalLogFunction); _rokt = nil; - _viewRegistry = nil; - _views = nil; [super tearDown]; } -// Regression guard for the change itself: without `@synthesize viewRegistry_DEPRECATED` -// in RNMPRokt.mm the module has no way to reach a view, and every embedded placement -// silently resolves to nothing. -- (void)testModuleIsWiredToAViewRegistryThatResolvesMountedViews +- (void)testSkipsNameWithNoMountedView { - UIView *mountedView = [[UIView alloc] init]; - _views[@101] = mountedView; - - XCTAssertNotNil(_rokt.viewRegistry_DEPRECATED); - XCTAssertEqualObjects([_rokt.viewRegistry_DEPRECATED viewForReactTag:@101], mountedView); - XCTAssertNil([_rokt.viewRegistry_DEPRECATED viewForReactTag:@999]); -} - -- (void)testSkipsTagThatIsNotMounted -{ - NSDictionary *resolved = [_rokt resolvePlaceholders:@{@"Location1" : @999}]; + NSDictionary *resolved = [_rokt resolvePlaceholders:@[ @"Location1" ]]; XCTAssertEqual(resolved.count, 0u); XCTAssertEqual(_loggedErrorCount, 1, @"errors: %@", _loggedErrors); + XCTAssertTrue([_loggedErrors.firstObject hasPrefix:@"Cannot resolve placeholder"], + @"errors: %@", _loggedErrors); } -- (void)testSkipsTagResolvingToUnexpectedViewClass +- (void)testSkipsNameRegisteredToUnexpectedViewClass { - _views[@101] = [[UIView alloc] init]; + UIView *view = [UIView new]; + [RoktPlaceholderRegistry registerView:view name:@"Location1"]; - NSDictionary *resolved = [_rokt resolvePlaceholders:@{@"Location1" : @101}]; + NSDictionary *resolved = [_rokt resolvePlaceholders:@[ @"Location1" ]]; XCTAssertEqual(resolved.count, 0u); XCTAssertEqual(_loggedErrorCount, 1, @"errors: %@", _loggedErrors); + [RoktPlaceholderRegistry unregisterView:view]; } -- (void)testSkipsNonNumericValueWithNoRegisteredNameWithoutThrowing +- (void)testSkipsNonStringEntriesWithoutThrowing { - // Defensive coverage for malformed direct native calls: viewForReactTag: would throw on - // NSNull, so non-numeric values must never reach it. They are resolved by placeholder name - // instead, and nothing is registered under these names. - NSDictionary *resolved = - [_rokt resolvePlaceholders:@{@"Location1" : [NSNull null], @"Location2" : @"101"}]; + // Defensive coverage for malformed direct native calls, such as a legacy react tag: + // only placeholderName strings are looked up. + NSDictionary *resolved = [_rokt resolvePlaceholders:@[ [NSNull null], @101 ]]; XCTAssertEqual(resolved.count, 0u); XCTAssertEqual(_loggedErrorCount, 2, @"errors: %@", _loggedErrors); @@ -134,6 +100,17 @@ - (void)testSkipsNonNumericValueWithNoRegisteredNameWithoutThrowing @"errors: %@", _loggedErrors); } +- (void)testUnmountedPlaceholderNamesListsOnlyUnregisteredStrings +{ + UIView *view = [UIView new]; + [RoktPlaceholderRegistry registerView:view name:@"Location1"]; + + NSArray *pending = [RNMPRokt unmountedPlaceholderNames:@[ @"Location1", @"Location2", @101 ]]; + + XCTAssertEqualObjects(pending, (@[ @"Location2" ])); + [RoktPlaceholderRegistry unregisterView:view]; +} + // Name-based resolution goes through RoktPlaceholderRegistry. Its semantics are // binary-independent, so they are asserted directly with plain views; the final // isKindOfClass: step hits the same linkage limit described above. @@ -216,7 +193,7 @@ - (void)testEmptyPlaceholdersResolveToEmptyDictionary { // Overlay / bottom-sheet placements pass no placeholders at all, so this path must not // depend on the view hierarchy in any way. - NSDictionary *resolved = [_rokt resolvePlaceholders:@{}]; + NSDictionary *resolved = [_rokt resolvePlaceholders:@[]]; XCTAssertEqual(resolved.count, 0u); XCTAssertEqual(_loggedErrorCount, 0, @"errors: %@", _loggedErrors);