Skip to content

fix(attributes): preserve null custom attributes and iOS New Arch product attributes#362

Merged
thomson-t merged 3 commits into
mainfrom
thomson-t/fix-rn-custom-attribute-parity
Jul 24, 2026
Merged

fix(attributes): preserve null custom attributes and iOS New Arch product attributes#362
thomson-t merged 3 commits into
mainfrom
thomson-t/fix-rn-custom-attribute-parity

Conversation

@thomson-t

@thomson-t thomson-t commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Context

Two React Native SDK defects were reported:

  1. iOS New Architecture dropped product-level custom attributes entirely.
  2. null custom event attributes behaved inconsistently across platforms.

This PR unifies null handling and fixes the New Architecture product bug.

Behavior

  • Explicit null custom attribute → "" on both platforms (event, commerce, and product level). { coupon_code: null } shows as { coupon_code: "" } in Live Stream. Omit the key to represent an absent value.
  • iOS New Architecture products now retain their customAttributes.
  • Event-level non-null values keep their existing per-platform behavior: iOS preserves value types (42 stays 42), Android continues to stringify. Only null handling is unified.

Changes

JS (js/index.tsx)

  • Non-mutating normalize* helpers convert null'' at the bridge boundary for logEvent, logScreenEvent, logMPEvent, logCommerceEvent (event + product + impression attributes).
  • A null attribute container (e.g. product.customAttributes = null, event.info = null) is treated the same as undefined (field omitted) instead of hitting Object.keys(null) / null.map — prevents a runtime crash for plain-JS consumers. (PR review feedback.)
  • Public CustomAttributes type widened to allow null; the Codegen spec type stays non-null since values are normalized before crossing the bridge.

iOS (ios/RNMParticle/RNMParticle.mm)

  • RNMParticleStringAttributes (products): stringify values + null"" — required by MPProduct's string-only custom-attribute API.
  • RNMParticleEventAttributes (events/commerce-event level): preserve value types, null"" only.
  • New Architecture createMPProductFromDict now applies customAttributes (the dropped-attribute fix).
  • Legacy RCTConvert paths aligned with the same helpers.

Dependency floor (react-native-mparticle.podspec)

Android — no production change (convertStringMap already did null""); added MParticleModuleTest to lock it in.

Docs — README + CHANGELOG guidance on null custom attributes.

Tests

  • JS: js/__tests__/attribute-normalization.test.ts — null→'' normalization, input-non-mutation across all paths, and null-container omission. ✅ 5/5 pass.
  • iOS: RCTConvertCommerceMappingTests — New Arch + legacy product attrs (string/number/bool/null), legacy event/commerce null→"", and event-level numeric preserved.
  • Android: MParticleModuleTest — null event attr → "".
  • Typecheck + lint: npx tsc --noEmit and eslint ✅ clean.

Verification notes

  • yarn jest (5 tests), npx tsc --noEmit, and eslint pass locally.
  • The earlier iOS compile error (property 'deviceConsentState' not found) was a dependency-floor mismatch, now fixed: the podspec floor is raised to >= 9.2.2, and deviceConsentState ships in SDK 9.2.2 (pod install resolves to 9.3.x, which has it). Because sample/ios/Podfile.lock is gitignored, run pod install --repo-update in sample/ios to re-resolve the stale 9.2.1 lock before building the iOS test target.

🤖 Generated with Claude Code

…duct attributes

Normalize explicit null custom-attribute values to "" before serialization so
they are preserved consistently in Live Stream on iOS and Android, and fix iOS
New Architecture product conversion to retain product-level custom attributes.

- JS: non-mutating null -> "" normalization at the bridge boundary for
  event/screen/MP/commerce events and product/impression attributes; widen
  public CustomAttributes type to allow null.
- iOS: RNMParticleStringAttributes (products: stringify + null->"") and
  RNMParticleEventAttributes (events: preserve value types, null->"" only);
  apply product customAttributes in New Architecture createMPProductFromDict.
- Android: unit coverage for existing null -> "" bridge conversion.
- Docs: README + CHANGELOG guidance on null custom attributes.
- Tests: JS jest suite, iOS XCTest coverage, Android unit test.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Copilot AI review requested due to automatic review settings July 24, 2026 17:57
@thomson-t
thomson-t requested a review from a team as a code owner July 24, 2026 17:57
@cursor

cursor Bot commented Jul 24, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches analytics event/commerce bridging on all platforms and tightens the iOS SDK dependency floor; behavior change for null attributes is intentional but affects Live Stream output.

Overview
Custom attributes: Explicit null values on event, screen, commerce, and product attributes are normalized to "" before the native bridge on both platforms, with non-mutating helpers in js/index.tsx and matching iOS bridge logic. CustomAttributes now allows null; README documents Live Stream behavior.

iOS: New Architecture product mapping applies customAttributes again via RNMParticleStringAttributes / RNMParticleEventAttributes. Legacy and TurboModule paths share the same normalization. Podspec floor for mParticle-Apple-SDK-ObjC is raised to >= 9.2.2 so device-consent APIs compile.

Tests & tooling: Jest (attribute-normalization.test.ts), expanded iOS RCTConvertCommerceMappingTests, and Android MParticleModuleTest lock in null handling. yarn test runs Jest before lint.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR standardizes how null custom attributes are handled at the React Native bridge boundary (normalizing explicit null to empty string), and fixes an iOS New Architecture issue where product-level customAttributes were being dropped. It also adds tests across JS/iOS/Android plus documentation updates to clarify the intended behavior.

Changes:

  • JS: add non-mutating normalization helpers to convert explicit null attribute values to '' before crossing the bridge; widen public CustomAttributes to allow null.
  • iOS: add attribute-normalization helpers for event-level vs product-level attributes, and apply product customAttributes in the New Architecture product mapping.
  • Tests/docs/tooling: add JS Jest tests + config, add iOS commerce mapping tests, add Android unit test, and document the null behavior in README/CHANGELOG.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
yarn.lock Adds/updates Jest + ts-jest and related dependency lock entries.
package.json Runs Jest in npm test and adds Jest/ts-jest typings & deps.
jest.config.js Introduces ts-jest configuration to run JS/TS unit tests under js/**.
js/index.tsx Adds normalize* helpers and normalizes null attributes at the JS→native boundary.
js/tests/attribute-normalization.test.ts Adds Jest coverage for null-to-empty-string normalization and non-mutation guarantees.
ios/RNMParticle/RNMParticle.mm Normalizes event-level attributes (preserve types; null→"") and product attributes (string-only; null→""), and applies product customAttributes in New Arch mapping.
sample/ios/MParticleSampleTests/RCTConvertCommerceMappingTests.m Adds regression tests covering legacy + New Arch product attrs and legacy event/commerce null normalization.
android/src/test/java/com/mparticle/react/MParticleModuleTest.java Adds unit test to lock in Android null→"" custom attribute conversion.
README.md Documents intended handling of explicit null custom attribute values.
CHANGELOG.md Notes the cross-platform null-handling change and iOS New Arch product fix.
Comments suppressed due to low confidence (4)

js/index.tsx:182

  • normalizeMPEvent only checks info === undefined; if event.info is null at runtime (common from plain JS or any-casts), normalizeCustomAttributes(null) will throw. Consider treating null the same as undefined (omit info) to avoid a crash.
  const { info, ...eventProperties } = event;
  if (info === undefined) {
    return eventProperties;

js/index.tsx:202

  • normalizeCommerceEvent treats customAttributes as present when it is null (only checks === undefined), which will crash in normalizeCustomAttributes(null). Handle null the same as undefined by omitting the customAttributes field.
    ...(customAttributes === undefined
      ? {}
      : {
          customAttributes: normalizeCustomAttributes(customAttributes),
        }),

js/index.tsx:205

  • normalizeCommerceEvent only checks products === undefined before calling products.map(...). If products is null at runtime, this will throw. Handle null the same as undefined (omit products).
    ...(products === undefined
      ? {}
      : { products: products.map(normalizeProduct) }),

js/index.tsx:208

  • normalizeCommerceEvent only checks impressions === undefined before calling impressions.map(...). If impressions is null at runtime, this will throw. Handle null the same as undefined (omit impressions).
    ...(impressions === undefined
      ? {}
      : { impressions: impressions.map(normalizeImpression) }),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread js/index.tsx
…e containers

- podspec: mParticle-Apple-SDK-ObjC floor '~> 9.2' -> '>= 9.2.2', '< 10.0'.
  The device-consent bridge uses MParticle.deviceConsentState, introduced in
  SDK 9.2.2; the old floor permitted 9.2.0/9.2.1 which fail to compile.
- js: treat a null customAttributes/info/products/impressions container the
  same as undefined (omit it) instead of calling Object.keys(null)/null.map,
  which threw for plain-JS consumers. Addresses PR review feedback.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9b4d42c. Configure here.

Comment thread js/index.tsx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

js/index.tsx:239

  • logScreenEvent currently checks only attributes === null. If a consumer calls logScreenEvent(screenName, undefined, shouldUploadEvent), this will attempt normalizeCustomAttributes(undefined) and throw. Treat undefined the same as null here.
  MParticleModule.logScreenEvent(
    screenName,
    attributes === null ? null : normalizeCustomAttributes(attributes),
    shouldUploadEvent
  );

Comment thread js/index.tsx

@BrandonStalnaker BrandonStalnaker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice to add but otherwise good

@thomson-t
thomson-t merged commit 2964357 into main Jul 24, 2026
12 checks passed
@thomson-t
thomson-t deleted the thomson-t/fix-rn-custom-attribute-parity branch July 24, 2026 19:44
@rokt-releases rokt-releases Bot mentioned this pull request Jul 24, 2026
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.

5 participants