fix(ios): resolve Rokt placeholders via RCTViewRegistry - #377
Conversation
selectPlacements routed its only native call through [self.bridge.uiManager addUIBlock:]. That path is no longer viable on the New Architecture: - React Native 0.84 defaults RCT_REMOVE_LEGACY_ARCH=1, under which RCTUIManager's addUIBlock: compiles to an empty method body, so the block never runs. - In bridgeless mode self.bridge is an RCTBridgeProxy whose RCTUIManagerProxy logs "This method isn't implemented faithfully. Please migrate to RCTViewRegistry". - The existing nil-bridge check only logged; it had no early return, so the call was messaged to nil and discarded with no RoktEvent, no promise rejection and no diagnostic. Callers only saw a placement that never settled. - methodQueue returned self.bridge.uiManager.methodQueue, which is nil on the proxy, so React Native silently substituted a shared background queue. Resolve placeholders through RCTViewRegistry instead, which React Native populates in both bridge and bridgeless modes, and dispatch with RCTExecuteOnMainQueue. methodQueue now returns the main queue, matching Android's UiThreadUtil.runOnUiThread, and the bridge property is removed. Non-numeric react tags are rejected up front because the spec allows `number | null` and viewForReactTag: would throw on NSNull, where the old dictionary subscript returned nil. Also set dependencyProvider in the sample app. It has been required since React Native 0.76; without it RCTReactNativeFactory reports no third-party Fabric components, so RoktNativeLayout was never registered, <RoktLayoutView> mounted as RCTUnimplementedViewComponentView and embedded placements resolved no placeholder view. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR SummaryMedium Risk Overview
The sample app sets Reviewed by Cursor Bugbot for commit 4126215. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
Updates the iOS Rokt integration to resolve embedded placement placeholder views via RCTViewRegistry (instead of RCTUIManager addUIBlock:), making selectPlacements work correctly under React Native New Architecture / bridgeless mode, and adds sample + test coverage to validate the new resolution path.
Changes:
- iOS: switch placeholder resolution to
RCTViewRegistryand dispatch to the main queue; remove reliance onbridge.uiManager. - iOS tests: add bridgeless
RCTViewRegistry-backed unit tests covering successful registry wiring and all placeholder-skip branches. - Sample app: set
dependencyProviderso third-party Fabric components (includingRoktNativeLayout) register correctly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sample/ios/MParticleSampleTests/RNMPRoktPlaceholderTests.m | Adds unit tests for placeholder resolution against a real RCTViewRegistry in bridgeless mode. |
| sample/ios/MParticleSample/AppDelegate.mm | Registers RCTAppDependencyProvider to ensure Fabric components are discoverable in the sample app. |
| sample/ios/MParticleSample.xcodeproj/project.pbxproj | Wires the new test file into the sample test target. |
| ios/RNMParticle/RNMPRokt.mm | Replaces addUIBlock: with RCTViewRegistry-based placeholder resolution on the main thread. |
| ios/RNMParticle/RNMPRokt.h | Removes the bridge property from the New Architecture interface. |
Suppressed comments (1)
ios/RNMParticle/RNMPRokt.mm:345
- The error log prints the placeholder key as the tag (
#%@, wherekeyis the placeholder location string). Logging the actualreactTag(and optionally the placeholder key) will make it clear which tag failed to resolve.
RCTLogError(@"Cannot find RoktEmbeddedView with tag #%@", key);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
selectPlacementsrouted its only native call through[self.bridge.uiManager addUIBlock:]. That path is no longer viable on the New Architecture:RCT_REMOVE_LEGACY_ARCH=1(scripts/react_native_pods.rb:93), under whichRCTUIManager'saddUIBlock:compiles to an empty method body — the block never runs.self.bridgeis anRCTBridgeProxy, whoseRCTUIManagerProxylogs "This method isn't implemented faithfully. Please migrate to RCTViewRegistry" (silenced at the default log level).RoktEvent, no promise rejection, no diagnostic. Callers only saw a placement that never settled.methodQueuereturnedself.bridge.uiManager.methodQueue, which is nil on the proxy, so React Native silently substituted a shared background queue — the declared "run on the UIManager queue" intent had not held for some time.Changes:
RCTViewRegistry(@synthesize viewRegistry_DEPRECATED), which React Native populates in both bridge and bridgeless modes viaRCTBridgeModuleDecorator. This is what React Native's own core modules and react-native-maps / -screens / -svg use.RCTExecuteOnMainQueue;methodQueuenow returns the main queue, matching Android'sUiThreadUtil.runOnUiThread(MPRoktModule.kt). Thebridgeproperty is removed.number | nullandviewForReactTag:throws onNSNull, where the old dictionary subscript returned nil.dependencyProvider, required since React Native 0.76. Without itRCTReactNativeFactoryreports no third-party Fabric components, soRoktNativeLayoutwas never registered,<RoktLayoutView>mounted asRCTUnimplementedViewComponentViewand embedded placements resolved no placeholder view.Behaviour is otherwise unchanged — unresolvable placeholders still log via
RCTLogErrorand are skipped. Emitting aPlacementFailureon those paths instead of staying silent is deliberately left for a follow-up.Testing Plan
New
sample/ios/MParticleSampleTests/RNMPRoktPlaceholderTests.m(5 tests) drivesresolvePlaceholders:against a realRCTViewRegistrywith a bridgeless component-view provider — the same hookRCTInstancewires to the surface presenter. Covers the registry wiring plus every skip branch: unmounted tag, wrong view class, non-numeric tag, and empty placeholders.Verified on the sample app, iPhone 17 Pro / iOS 26.5, New Architecture:
calling mParticle Core selectPlacementsaddUIBlocklog lines1 placeholder(s)→ resolved 01 placeholder(s)→ resolved 1RoktFabricWrapperView initializedThe before/after was run by swapping only
RNMPRokt.{h,mm}between builds with identical JS.yarn testandyarn lintpass; iOS suitesRNMPRoktPlaceholderTests+RCTConvertCommerceMappingTestsare 12/12.Additional testing worth doing:
#elsebranches (RCTViewRegistrycovers both, but this was not built here).RCT_REMOVE_LEGACY_ARCH=0.Master Issue
N/A — no linked work item.