diff --git a/MIGRATING.md b/MIGRATING.md index bc3ab56..fe36472 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 aec8be5..653b603 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 1f65f1a..46b9d22 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 fc642fe..a0688dc 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 4ccc2bc..9dd4d3b 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 fa29fdf..0719dd5 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 6f65a20..3efca7a 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 156266d..80a8520 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 64af67d..849c98a 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 a95cff6..e391b42 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 a2aca0e..7073f82 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 cfce3df..5dbb485 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 ec97ef1..3c0c849 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 008dfda..edf30a2 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);