Skip to content

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

Open
thomson-t wants to merge 1 commit into
thomson-t/spm-06-package-swiftfrom
thomson-t/spm-07-remove-placeholder-map
Open

thomson-t wants to merge 1 commit into
thomson-t/spm-06-package-swiftfrom
thomson-t/spm-07-remove-placeholder-map

Conversation

@thomson-t

Copy link
Copy Markdown
Contributor

Why

Embedded placements can still be requested with a map of placeholder name to findNodeHandle view tag. That form keeps the SDK on view-tag lookups React Native is removing (viewRegistry_DEPRECATED on iOS, NativeViewHierarchyManager and UIManager.resolveView on Android), and integrations that use it can still read a tag before the view exists and fail to embed without an error. Name-based placeholders already cover every case the map did, including views that mount after the call. Once this lands, selectPlacements has one way to embed a placement: an array of RoktLayoutView placeholder names. This is a breaking change for apps that still pass the map; the release that ships the Swift Package Manager work is a major version for this reason.

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. This is the seventh pull request in the series merging into the workstation/spm-migration branch, which merges into main once the series is complete and ships in one release; the plan record is internal and cannot be linked here.

What changes

Before: placeholders is a name array or a map of name to view tag. The wrapper turns both into a map, using 0 to mean "look up by name", and each native module resolves positive values as view tags.

After: placeholders is an array of placeholder names, from the TypeScript type through the native module interface to both native modules.

  • JavaScript: RoktPlaceholders is string[]. A map passed from plain JavaScript logs an error, and the placement is requested without embedded views. Without this check the platforms would disagree: Android would throw, and iOS would drop or misread the argument.
  • iOS: resolvePlaceholders: and the wait for unmounted views read names only and skip non-string entries. The view-tag lookup and @synthesize viewRegistry_DEPRECATED are removed.
  • Android: both architectures call one name resolver in MPRoktModuleImpl, which skips non-string entries. The view-tag lookup (UIManager.resolveView, NativeViewHierarchyManager) is removed. The legacy architecture still uses addUIBlock to run after pending UI work.
  • Tests: the jest, Android unit and XCTest cases for view tags are replaced by array cases, including non-string entries.
  • Docs: README and MIGRATING say the map form is removed, and that earlier 3.x releases accept both forms, so apps can switch before they upgrade.

Start reading at js/rokt/rokt.ts, then MPRoktModuleImpl.kt and RNMPRokt.mm. Unchanged on purpose: the name registry, the 2-second wait for unmounted views, and PlacementFailure for dropped waits.

Linked work

Depends on: the experimental Package.swift (branch thomson-t/spm-06-package-swift) and the pull requests below it in this series; merge those first.
Related: the pull request that added name-based placeholders, #410.

Rollout

Path: this merges into workstation/spm-migration, not main, so nothing reaches main or a release until the whole series has merged there and that branch is merged into main. It then ships in the next release, which must be a major version.
Feature flags: none.
Turning it off: revert this pull request; the map form comes back in the next release.
What we watch: this repository's issues, for embedded placements that stop rendering after an upgrade, and the error selectPlacements: placeholders must be an array in partner reports.

Risks

  • Apps that still pass the map lose their embedded placements after upgrading. Not prevented, because this is the intended break. It is mitigated by the TypeScript error, the logged error, MIGRATING, and 3.x releases that accept both forms. We would see reports of embedded placements not rendering, with the error above in the logs.
  • A native caller that bypasses the wrapper passes a map or a non-string entry. Prevented, because both native modules read only string entries and skip anything else. We would see Cannot resolve placeholder logs.
  • The Old Architecture iOS and Android modules change signature. Covered by the Android unit tests and lint, which compile the legacy module. An Expo 54 / React Native 0.81.5 app with the New Architecture off builds on iOS. We would see build errors on React Native 0.81 with the New Architecture off.

Risk class: medium, because this is a breaking API change.

Who

Written by: an automated coding agent (Claude Code), at an engineer's request.
Code reviewed before opening: an independent review agent reviewed the change before it was committed.
Design reviewed before opening: the requesting engineer approved removing the map form in the release that ships Swift Package Manager support, and chose the log-and-drop behaviour for a leftover map.
Decision this implements: the engineering decision on 2026-09-29 to ship the breaking placeholder change together with Swift Package Manager support; the record is internal and cannot be linked.
Checked: on 2026-09-30, with Xcode 27.0, an iOS 26.5 simulator and an Android emulator (API 36):

  • yarn test (jest and eslint), yarn build and yarn build:plugin; ./gradlew test ktlintCheck lint in android/.
  • Sample app (React Native 0.84, New Architecture, default CocoaPods mode):
    • The full Debug XCTest suite, with Metro running: 54 passed and 1 skipped, the legacy-architecture-only test.
    • The Podfile.lock is unchanged, and the Release build has one copy of each SDK class.
    • Embedded placement by name, called from the same effect that renders the view: renders on iOS and Android.
    • The old map form, passed from plain JavaScript: the error is logged, the app keeps running, and the placement request goes out without embedded views, ending in PlacementFailure on iOS and Android.
  • Swift Package Manager mode (MP_USE_SPM=1): the sample builds in Release with one copy of each SDK class, all in the app binary.
  • Old Architecture: an Expo 54 / React Native 0.81.5 app with the New Architecture off builds in Release on iOS.
    Not checked: overlay placements (they pass no placeholders, so that path is unchanged); React Native's Swift Package Manager mode; physical devices.

Size

Hand-written: 149 lines added and 263 removed in 14 files (55 added and 70 removed of them in tests).
Generated: none.

🤖 Generated with Claude Code

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<string>. 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) <noreply@anthropic.com>
@thomson-t
thomson-t marked this pull request as ready for review September 30, 2026 15:28
@thomson-t
thomson-t requested a review from a team as a code owner September 30, 2026 15:28
@cursor

cursor Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
This is a breaking API change that drops support for React view tag maps in embedded Rokt placements. Applications still passing tag maps will fail to render embedded placements until updated.

Overview
Breaking Change: Updates MParticle.Rokt.selectPlacements to accept embedded placeholders solely as an array of placeholderName strings (string[]), removing support for the legacy map of placeholder names to findNodeHandle React view tags.

In JavaScript, RoktPlaceholders is now typed as string[]. Passing a legacy map from plain JavaScript logs an error and omits embedded views from the placement request.

On iOS and Android across both architectures, native module signatures and implementations now accept arrays instead of dictionaries. All legacy view tag lookup logic (such as viewRegistry_DEPRECATED and UIManager.resolveView) has been removed in favor of name-based lookups via RoktPlaceholderRegistry.

Reviewed by Cursor Bugbot for commit 5ba7575. Bugbot is set up for automated code reviews on this repo. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant