Conversation
PR SummaryMedium Risk Overview Updated Reviewed by Cursor Bugbot for commit 8fffd85. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The checked-in RNMParticle Xcode target does not compile the new Swift implementations referenced by its Objective-C++ sources.
Review effort: Balanced
Findings: 1
What changed in this PR
Moves Rokt-typed iOS logic into Swift to support future Swift Package Manager integration while preserving the Objective-C++ bridge.
Changes:
- Adds Swift event, configuration, and embedded-view adapters.
- Updates Objective-C++ callers and CocoaPods settings.
- Adds characterization tests to both CI modes.
| File | Description |
|---|---|
.github/workflows/pull-request.yml |
Runs new iOS tests. |
react-native-mparticle.podspec |
Configures Swift and module support. |
ios/RNMParticle/RNMPRoktSwift.h |
Declares the Swift bridge API. |
ios/RNMParticle/RNMPSDKImports.h |
Restricts RoktContracts imports to Objective-C. |
ios/RNMParticle/RNMPRokt.mm |
Uses Swift config and view adapters. |
ios/RNMParticle/RoktEventManager.mm |
Delegates event mapping to Swift. |
ios/RNMParticle/RoktNativeLayoutComponentView.h |
Exposes the embedded view as UIView. |
ios/RNMParticle/RoktNativeLayoutComponentView.mm |
Creates embedded views through Swift. |
ios/RNMParticle/Swift/RNMPRoktEventMapper.swift |
Maps Rokt events and side effects. |
ios/RNMParticle/Swift/RNMPRoktConfigFactory.swift |
Builds Rokt configuration objects. |
ios/RNMParticle/Swift/RNMPRoktViews.swift |
Creates and identifies embedded views. |
ios/RNMParticle.xcodeproj/project.pbxproj |
Adds the bridge header, but omits Swift sources. |
sample/ios/MParticleSampleTests/RNMPRoktEventMapperTests.m |
Characterizes event payloads and ordering. |
sample/ios/MParticleSampleTests/RNMPRoktConfigFactoryTests.m |
Tests configuration conversion. |
sample/ios/MParticleSampleTests/RNMPRoktSwiftTests.m |
Verifies bridge selectors and types. |
sample/ios/MParticleSample.xcodeproj/project.pbxproj |
Registers the new tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| C4A1D2E32F6A000100ABCDEF /* RoktPlaceholderRegistry.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RoktPlaceholderRegistry.h; sourceTree = "<group>"; }; | ||
| C4A1D2E42F6A000100ABCDEF /* RoktPlaceholderRegistry.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RoktPlaceholderRegistry.m; sourceTree = "<group>"; }; | ||
| C4A1D2E62F6A000100ABCDEF /* RNMPSDKImports.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNMPSDKImports.h; sourceTree = "<group>"; }; | ||
| C4A1D2E72F6A000100ABCDEF /* RNMPRoktSwift.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNMPRoktSwift.h; sourceTree = "<group>"; }; |
There was a problem hiding this comment.
Not changing this here. ios/RNMParticle.xcodeproj is not a working build path today. On main, before this stack, a standalone build fails, first on its 12.4 deployment target and then on its recursive React header search paths. Its mParticle header paths also point at the old pre-9.x pod layout, and it has none for RoktContracts. The supported integrations build these files through the podspec or Package.swift. Removing the project would be a separate change.
Under Swift Package Manager, Objective-C++ files cannot reach another target's generated Swift header, so they cannot use the RoktContracts types. Event mapping (RNMPRoktEventMapper), config building (RNMPRoktConfigFactory) and the embedded view (RNMPRoktViews) are now Swift. The .mm adapters reach them through the hand-written RNMPRoktSwift.h, which uses only Foundation and UIKit types, and no .mm file imports RoktContracts. The podspec adds swift_version, DEFINES_MODULE and private headers, plus the Swift include path for packages in Swift Package Manager mode. Characterization tests for every Rokt event type and for the config mapping were written against the Objective-C first, and pass unchanged against the Swift. A drift test checks every selector the header declares. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
67f1420 to
8fffd85
Compare

Why
React Native is moving apps to Swift Package Manager, and in that setup this package's Objective-C++ files cannot see the Rokt types they use today (the embedded view, events and config). The next pull request, an experimental package manifest for React Native's own Swift Package Manager mode, needs that code to live somewhere that can see them. Once this lands, all Rokt-typed logic is in Swift, and the Objective-C++ files only pass plain views and dictionaries through. Apps see no difference: the same events reach JavaScript, with the same contents, in the same order.
Programme
Part of the plan to support Swift Package Manager in this package before the CocoaPods central repository becomes read-only on 2026-12-02 (CocoaPods announcement). This is the fifth of eight pull requests, all merging into the
workstation/spm-migrationbranch, which merges intomainonce the series is complete, and shipping together in one release; the plan record is internal and cannot be linked here.What changes
Before:
RoktEventManager.mmmapped every Rokt event type to its JavaScript payload,RNMPRokt.mmbuilt the Rokt config and checked for embedded views, and the Fabric view created aRoktEmbeddedViewdirectly. All of them imported the RoktContracts headers.After: that logic is in three Swift classes in
ios/RNMParticle/Swift/:RNMPRoktEventMapper: event to payload, plus the callback and height side events;RNMPRoktConfigFactory: dictionary to config;RNMPRoktViews: creates and recognises the embedded view.The Objective-C++ files reach them through one hand-written header,
RNMPRoktSwift.h, which uses only Foundation and UIKit types; the generated Swift header can't be reached from Objective-C++ under Swift Package Manager. No.mmfile references RoktContracts any more, and the import header now gives RoktContracts to.mfiles only. The Fabric view'sroktEmbeddedViewproperty is typedUIView. The podspec gainsswift_version,DEFINES_MODULE, private headers (so C++ and React headers stay out of the module), and, in Swift Package Manager mode, the Swift include path for the packages.The tests were written first, against the original Objective-C, and pass unchanged against the Swift port:
CI runs the three new test classes in both legs. No documentation change is needed; the changelog is generated by the release-draft workflow.
Start reading at
RNMPRoktSwift.h, thenRoktEventManager.mm(the event adapter), then the Swift files.Linked work
Depends on: the Expo config plugin option (branch
thomson-t/spm-04-expo-spm) and the pull requests below it in this series; merge those first.Unblocks: the experimental
Package.swiftfor React Native's Swift Package Manager mode (branchthomson-t/spm-06-package-swift).Rollout
Path: this merges into
workstation/spm-migration, notmain, so nothing reachesmainor a release until the whole series has merged there and that branch is merged intomain. It then ships in the next release. It is live for every iOS app on that release.Feature flags: none.
Turning it off: a published version cannot be recalled. Reverting this pull request and releasing again restores the Objective-C mapping in the next version.
What we watch: this repository's issues, for Rokt events missing or changed in JavaScript, embedded placements that stop resizing, and iOS build errors mentioning Swift.
Risks
pre_installdynamic-framework hook failpod install("Swift podreact-native-mparticledepends uponmParticle-Apple-SDK-ObjC, which does not define modules"); contained because the nativemParticle-Apple-SDKpod already fails the same way in that setup, so the hook, oruse_frameworks!, is already required; we would see that error reported against that release by an app that had worked before.cacheAttributesvalue is not a string, the config factory drops the wholecacheAttributesmap, where the Objective-C passed such values through unchecked (and they would have crashed when read); not addressed further, because the JavaScript type allows only strings; we would see cache attributes ignored.Risk class: higher — this rewrites how every Rokt event and config reaches the SDK on iOS. Behaviour is pinned by tests written against the old code.
Who
Written by: an automated coding agent (Claude Code), at an engineer's request, following the internal Swift Package Manager migration plan and its proof of concept.
Code reviewed before opening: an independent review agent reviewed the change before it was committed.
Design reviewed before opening: the requesting engineer approved the plan's Swift-first layering, with hand-written Objective-C headers for the Swift classes.
Decision this implements: the requesting engineer's approval of the migration plan on 2026-09-28; the record is internal and cannot be linked.
Checked: on 2026-09-28, with Xcode 27.0 on an iOS 26.5 simulator, the sample app (React Native 0.84, New Architecture):
fmtdependency, which does not build with Xcode 27.Not checked: the Old Architecture at runtime; tvOS. CI, which builds with Xcode 16, passed on this pull request.
Size
Hand-written: about 515 lines added and 210 removed in 16 files; 284 of the added lines are tests, and the Swift and adapter code is about 230.
Generated: none.
Why one pull request: the tests pin the old behaviour and must land with the port they guard, and the adapters cannot switch to Swift one file at a time without leaving an Objective-C++ file that still needs the Rokt headers.
Notes for reviewers
Why hand-written headers. Under Swift Package Manager, Swift and Objective-C++ must be separate targets, and an Objective-C++ file can only reach another target's generated
-Swift.hwith C++ modules, which break React Native's headers. The mParticle SDK'sMPRokt.honly forward-declaresRoktEmbeddedView,RoktConfigandRoktEvent, so pointers to them still type-check in.mmfiles without the RoktContracts headers.The characterization test's spy.
RNMPRoktEventMapperTestsswaps the event manager's class for a subclass that recordssendEventWithName:body:instead of sending. The subclass adds no instance variables, so swapping the class of an existing instance is safe.A clamp worth knowing.
CacheConfigraises a zero duration to its 90-minute maximum. A config test now pins that, since the JavaScript wrapper sends 0 when the duration is missing.🤖 Generated with Claude Code