feat(plugin-session-replay-react-native): pass maskLevel through to native SessionReplay#1771
Open
aliaksandr-kazarez wants to merge 8 commits into
Open
feat(plugin-session-replay-react-native): pass maskLevel through to native SessionReplay#1771aliaksandr-kazarez wants to merge 8 commits into
aliaksandr-kazarez wants to merge 8 commits into
Conversation
…ative SessionReplay Wires a new `maskLevel` configuration field through the plugin's JS → ObjC → Swift and JS → Kotlin bridge so consumers of `@amplitude/plugin-session-replay-react-native` can request Light/Medium/Conservative masking from JS. Previously the plugin always defaulted to MEDIUM regardless of JS config because the bridge layers never forwarded the value to the native `SessionReplay` constructor. Brings the plugin variant to parity with `@amplitude/session-replay-react-native`, which already wires this up. Refs SDKRN-9 (parent), implements SDKRN-11. Co-authored-by: Cursor <cursoragent@cursor.com>
size-limit report 📦
|
| enableRemoteConfig?: boolean; | ||
| logLevel?: LogLevel; | ||
| autoStart?: boolean; | ||
| maskLevel?: MaskLevel; |
Contributor
Author
There was a problem hiding this comment.
hey, let's follow how other Session Replay SDKs are naming that, and follow it.
I believe it should be PrivacyConfig or similar.
Contributor
Author
There was a problem hiding this comment.
do you also need to apply those changes to session-replay-react-native package?
…nd mask levels table Co-authored-by: Cursor <cursoragent@cursor.com>
…askLevel Add PrivacyConfig interface wrapping maskLevel to match Flutter/Android SDK naming. Migrate plugin package from flat maskLevel to privacyConfig.maskLevel. Add PrivacyConfig to standalone package with deprecated maskLevel kept for backwards compat, preferring privacyConfig.maskLevel in the native bridge. Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Deprecated
maskLevelfield silently ignored due to default precedence- Removed maskLevel from default privacyConfig to allow proper fallback to the deprecated maskLevel field via the nullish coalescing operator.
Or push these changes by commenting:
@cursor push 6bae5a6e4c
Preview (6bae5a6e4c)
diff --git a/packages/session-replay-react-native/src/session-replay-config.ts b/packages/session-replay-react-native/src/session-replay-config.ts
--- a/packages/session-replay-react-native/src/session-replay-config.ts
+++ b/packages/session-replay-react-native/src/session-replay-config.ts
@@ -106,7 +106,7 @@
logLevel: LogLevel.Warn,
maskLevel: MaskLevel.Medium,
optOut: false,
- privacyConfig: { maskLevel: MaskLevel.Medium },
+ privacyConfig: {},
sampleRate: 0,
serverZone: 'US',
sessionId: -1,You can send follow-ups to the cloud agent here.
…privacyConfig
`getDefaultConfig()` previously returned `privacyConfig: { maskLevel: Medium }`,
so the resolution chain `config.privacyConfig?.maskLevel ?? config.maskLevel`
short-circuited on the default `privacyConfig.maskLevel` and never fell through
to the user-supplied deprecated `maskLevel`. As a result, passing
`{ maskLevel: Conservative }` (without `privacyConfig`) silently downgraded the
masking level to `Medium`.
Drop both `privacyConfig` and `maskLevel` from `getDefaultConfig()` and apply
the `Medium` default only at the resolution site in `nativeConfig()`. Added
test coverage for: (a) deprecated `maskLevel` is forwarded when no
`privacyConfig` is provided, (b) `privacyConfig.maskLevel` wins over the
deprecated `maskLevel` when both are set, (c) `Medium` is used when neither is
set.
Co-authored-by: Cursor <cursoragent@cursor.com>
… at config boundary The previous fix in 584ab86 worked but lived in the wrong layer: it stripped `privacyConfig` from `getDefaultConfig()` and re-derived the `Medium` default inside `nativeConfig()`. That mixed responsibilities — the default belongs in the defaults, and `nativeConfig()` should be a pure translation from a fully resolved internal config to the native shape. Move the deprecated→`privacyConfig` translation to a `normalizeConfig()` step that runs at the input boundary in `init()`. `getDefaultConfig()` goes back to owning the `Medium` default via `privacyConfig: { maskLevel: Medium }`, and a new module-internal `SessionReplayConfigInternal = Omit<SessionReplayConfig, 'maskLevel'>` type guarantees nothing past the boundary sees the deprecated field. `nativeConfig()` drops the `?? ?? ??` chain and just reads `privacyConfig.maskLevel`, which the merge in `init()` guarantees is set. Public `SessionReplayConfig` is unchanged; the deprecated `maskLevel` field and its `@deprecated` JSDoc stay put for backwards compat. The internal type is intentionally not re-exported from `index.tsx`. Behavior is identical: the three existing precedence tests still pass without modification, and two new tests assert that a user-supplied `privacyConfig` survives the default merge and that the internal `privacyConfig` object is not leaked through to the native bridge. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unsafe cast allows undefined maskLevel to reach native bridge
- Added runtime fallback using ?? MaskLevel.Medium to prevent undefined maskLevel from reaching the native bridge when privacyConfig is passed without maskLevel.
Or push these changes by commenting:
@cursor push 0207f9fba2
Preview (0207f9fba2)
diff --git a/packages/session-replay-react-native/src/session-replay.ts b/packages/session-replay-react-native/src/session-replay.ts
--- a/packages/session-replay-react-native/src/session-replay.ts
+++ b/packages/session-replay-react-native/src/session-replay.ts
@@ -3,6 +3,7 @@
import { NativeSessionReplay, type NativeSessionReplayConfig } from './native-module';
import {
getDefaultConfig,
+ MaskLevel,
PrivacyConfig,
SessionReplayConfig,
SessionReplayConfigInternal,
@@ -234,16 +235,15 @@
}
function nativeConfig(config: ResolvedSessionReplayConfig): NativeSessionReplayConfig {
- // `privacyConfig.maskLevel` is guaranteed to be set by the merge in
- // `init()`: `getDefaultConfig()` supplies `{ maskLevel: Medium }` and the
- // shallow spread only overwrites it with a normalized `privacyConfig` that
- // carries a `maskLevel`. Strip `privacyConfig` from the spread because the
- // native bridge only takes a flat `maskLevel` string.
+ // Apply runtime fallback for `maskLevel` in case `privacyConfig` was
+ // replaced wholesale by a partial user config (e.g. `privacyConfig: {}`).
+ // The shallow merge in `init()` can't guarantee the inner `maskLevel` field
+ // is set because `PrivacyConfig.maskLevel` is optional.
const { privacyConfig, ...rest } = config;
return {
...rest,
logLevel: rest.logLevel as NativeSessionReplayConfig['logLevel'],
- maskLevel: privacyConfig.maskLevel as NativeSessionReplayConfig['maskLevel'],
+ maskLevel: privacyConfig.maskLevel ?? MaskLevel.Medium,
};
}You can send follow-ups to the cloud agent here.
| maskLevel: config.maskLevel.toString() as NativeSessionReplayConfig['maskLevel'], | ||
| ...rest, | ||
| logLevel: rest.logLevel as NativeSessionReplayConfig['logLevel'], | ||
| maskLevel: privacyConfig.maskLevel as NativeSessionReplayConfig['maskLevel'], |
There was a problem hiding this comment.
Unsafe cast allows undefined maskLevel to reach native bridge
Medium Severity
privacyConfig.maskLevel is typed as MaskLevel | undefined (since PrivacyConfig.maskLevel is optional), but the as NativeSessionReplayConfig['maskLevel'] cast silently coerces undefined into the expected 'light' | 'medium' | 'conservative' string union. When a consumer passes privacyConfig: {}, the shallow merge in init() replaces the default { maskLevel: Medium } wholesale with {}, and undefined reaches the native bridge — likely crashing Android (String.lowercase() on null) and iOS. The sibling plugin-session-replay-react-native handles this correctly with ?? MaskLevel.Medium on the equivalent line.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6c93e5f. Configure here.
…Config and normalizeConfig - Collapse ResolvedSessionReplayConfig to Required<SessionReplayConfigInternal>; apiKey is already non-optional on SessionReplayConfig, so the Omit+Pick dance was equivalent and noisy. - Replace the ?? + nested-ternary chain in normalizeConfig with explicit top-to-bottom if/else branches that read like the spec text. - Add TODO(SDKRN-15) on the line that flattens privacyConfig.maskLevel to the bridge's flat maskLevel string, so the next person hits a pointer to the follow-up native-bridge migration ticket. No behavior change. 48/48 standalone tests + 8/8 plugin tests still pass; typecheck clean. Co-authored-by: Cursor <cursoragent@cursor.com>
… standalone package
Mirrors the recent shape polish on the standalone
`@amplitude/session-replay-react-native` package so the two share the same
config-handling style. No public API changes — same exports, same field
names, same `SessionReplayConfig` / `PrivacyConfig` / `MaskLevel`.
- `getDefaultConfig` now returns `Required<SessionReplayConfig>` with
`privacyConfig: { maskLevel: MaskLevel.Medium }` baked into the
defaults, matching the standalone's `Required<...>` return shape.
- `SessionReplayConfig` field order alphabetized and full JSDoc with
`@default` annotations copied from the standalone where semantics are
identical. Added a class-level JSDoc explaining why `apiKey`,
`deviceId`, `sessionId`, `serverZone`, and the deprecated `maskLevel`
are intentionally absent from the plugin config (auto-sourced from
the analytics client; no deprecated field to migrate).
- Introduced `type ResolvedSessionReplayConfig = Required<SessionReplayConfig>`
alias and typed `sessionReplayConfig` with it, mirroring the
standalone's `ResolvedSessionReplayConfig` pattern. Plugin does not
need a `SessionReplayConfigInternal` alias because it has no
deprecated field to omit.
- Removed redundant `?? default` fallbacks in `setup()` now that
`getDefaultConfig()` guarantees every field is set after the shallow
spread. Notably, this fixes a latent bug where `sampleRate` fell back
to `1` in `setup()` instead of the documented `0` default.
- Destructured `privacyConfig` out of `sessionReplayConfig` and read
`privacyConfig.maskLevel` directly with no `?? MaskLevel.Medium`
fallback. Added the matching
`// TODO(SDKRN-15): Migrate native bridge to accept the full
privacyConfig object instead of a flat maskLevel string.` comment.
- Aligned `index.tsx` export ordering and added `type` qualifier on
`SessionReplayConfig` to match the standalone.
The native bridge stays on its positional-args shape (vs the
standalone's config-object bridge) — that asymmetry is a deeper
refactor tracked under SDKRN-15.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Shallow merge lets
privacyConfig.maskLevelbecomeundefined- Implemented deep merge for privacyConfig in both plugin and standalone packages to preserve default maskLevel when users supply partial config.
Or push these changes by commenting:
@cursor push 486377b1a0
Preview (486377b1a0)
diff --git a/packages/plugin-session-replay-react-native/src/session-replay.ts b/packages/plugin-session-replay-react-native/src/session-replay.ts
--- a/packages/plugin-session-replay-react-native/src/session-replay.ts
+++ b/packages/plugin-session-replay-react-native/src/session-replay.ts
@@ -23,9 +23,14 @@
sessionReplayConfig: ResolvedSessionReplayConfig;
constructor(config: SessionReplayConfig = {}) {
+ const defaultConfig = getDefaultConfig();
this.sessionReplayConfig = {
- ...getDefaultConfig(),
+ ...defaultConfig,
...config,
+ privacyConfig: {
+ ...defaultConfig.privacyConfig,
+ ...config.privacyConfig,
+ },
};
console.log('Initializing SessionReplayPlugin with config: ', this.sessionReplayConfig);
}
@@ -36,9 +41,9 @@
// `apiKey`, `deviceId`, `sessionId`, and `serverZone` are sourced from the
// analytics client's `ReactNativeConfig` because the plugin runs inside an
// initialized Amplitude SDK and inherits identity from it.
- // `privacyConfig.maskLevel` is guaranteed to be set by the merge in the
- // constructor: `getDefaultConfig()` supplies `{ maskLevel: Medium }` and a
- // user-supplied `privacyConfig` always carries a `maskLevel`.
+ // `privacyConfig.maskLevel` is guaranteed to be set by the deep merge in the
+ // constructor: even if a user supplies a partial `privacyConfig`, the default
+ // `{ maskLevel: Medium }` is preserved.
const { privacyConfig } = this.sessionReplayConfig;
await PluginSessionReplayReactNative.setup(
config.apiKey,
diff --git a/packages/session-replay-react-native/src/session-replay.ts b/packages/session-replay-react-native/src/session-replay.ts
--- a/packages/session-replay-react-native/src/session-replay.ts
+++ b/packages/session-replay-react-native/src/session-replay.ts
@@ -55,13 +55,15 @@
return;
}
- // TODO: this is a shallow merge — a user-supplied `privacyConfig` replaces
- // the default object wholesale. That's fine while `PrivacyConfig` only
- // carries `maskLevel`, but if it ever grows more fields a deeper merge will
- // be needed so partial user configs don't drop defaults.
+ const defaultConfig = getDefaultConfig();
+ const normalizedConfig = normalizeConfig(config);
fullConfig = {
- ...getDefaultConfig(),
- ...normalizeConfig(config),
+ ...defaultConfig,
+ ...normalizedConfig,
+ privacyConfig: {
+ ...defaultConfig.privacyConfig,
+ ...normalizedConfig.privacyConfig,
+ },
};
logger.setLogLevel(fullConfig.logLevel);
@@ -233,11 +235,10 @@
}
function nativeConfig(config: ResolvedSessionReplayConfig): NativeSessionReplayConfig {
- // `privacyConfig.maskLevel` is guaranteed to be set by the merge in
- // `init()`: `getDefaultConfig()` supplies `{ maskLevel: Medium }` and the
- // shallow spread only overwrites it with a normalized `privacyConfig` that
- // carries a `maskLevel`. Strip `privacyConfig` from the spread because the
- // native bridge only takes a flat `maskLevel` string.
+ // `privacyConfig.maskLevel` is guaranteed to be set by the deep merge in
+ // `init()`: even if a user supplies a partial `privacyConfig`, the default
+ // `{ maskLevel: Medium }` is preserved. Strip `privacyConfig` from the spread
+ // because the native bridge only takes a flat `maskLevel` string.
const { privacyConfig, ...rest } = config;
return {
...rest,You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 22e0544. Configure here.
| isInitialized = false; | ||
|
|
||
| sessionReplayConfig: SessionReplayConfig; | ||
| sessionReplayConfig: ResolvedSessionReplayConfig; |
There was a problem hiding this comment.
Shallow merge lets privacyConfig.maskLevel become undefined
High Severity
The constructor's shallow ...config spread replaces the default privacyConfig: { maskLevel: Medium } wholesale when a user supplies { privacyConfig: {} }. Because maskLevel is optional in the PrivacyConfig interface, privacyConfig.maskLevel becomes undefined. This undefined is then passed as a positional argument to the native bridge where Kotlin expects a non-nullable String and Swift expects a non-optional String, causing a native crash. The same pattern exists in the standalone package's nativeConfig() function, where privacyConfig.maskLevel is cast via as to hide the undefined from TypeScript.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 22e0544. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires a new
maskLevelconfiguration field through the plugin's JS → ObjC → Swift and JS → Kotlin bridges so consumers of@amplitude/plugin-session-replay-react-nativecan requestLight/Medium/Conservativemasking from JS. Previously the plugin always defaulted toMEDIUMregardless of JS config because the bridge layers never forwarded the value to the nativeSessionReplayconstructor.Brings the plugin variant to parity with
@amplitude/session-replay-react-native, which already wires this up correctly.Tickets:
Bridge pattern template: #1076 (added
autoStartthe same way).Changes
All under
packages/plugin-session-replay-react-native/:src/session-replay-config.tsMaskLevelenum (Light/Medium/Conservative),maskLevel?: MaskLevelfield, defaultMediumsrc/index.tsxMaskLevelsrc/session-replay.tsmaskLevelas the 9th positional arg toPluginSessionReplayReactNative.setup(...)ios/PluginSessionReplayReactNative.mmmaskLevel:(NSString)maskLeveltoRCT_EXTERN_METHODios/PluginSessionReplayReactNative.swift@objc(setup:...)selector, addmaskLevel: Stringparameter, passmaskLevel: .fromString(maskLevel)toSessionReplay(...), addMaskLevel.fromString(_:)extension (copied verbatim from sibling standalone package)android/.../PluginSessionReplayReactNativeModule.ktMaskLevel+PrivacyConfigimports, addmaskLevel: Stringfinal parameter tosetup(...), map viawhenblock, passprivacyConfig = PrivacyConfig(maskLevel = mappedMaskLevel)toSessionReplay(...)test/index.test.tsPositional-arg contract (all four layers agree, source of truth is
src/session-replay.ts):apiKeydeviceIdsessionIdserverZonesampleRateenableRemoteConfiglogLevelautoStartmaskLevel(new)Deviation from the original task packet
The task packet listed 7 implementation files. This PR also touches one additional file:
packages/plugin-session-replay-react-native/jest.config.js— addedmoduleFileExtensions: ['tsx', 'ts', 'js', 'jsx', 'json']and added<rootDir>/example/totestPathIgnorePatterns.Reason: the existing config only included
['ts', 'js', 'json'](inherited from the base), which made it impossible fortest/index.test.tsto importSessionReplayPluginfromsrc/session-replay.ts(that file transitively imports./native-module.tsx). The fix mirrors the siblingpackages/session-replay-react-native/jest.config.js. Theexample/ignore preserves the prior baseline (the pre-existingexample/__tests__/App.test.tsxwas already broken at HEAD due to a missingreact-native-webviewinstall at the workspace root — not in scope to fix here).No shipped behavior change from this config edit. No
example/, README, or podspec touched.Test results
Confirmed
lib/typescript/session-replay-config.d.tsincludesexport declare enum MaskLevelandmaskLevel?: MaskLevel, andlib/typescript/index.d.tsre-exportsMaskLevel.iOS E2E coupling (NOT a blocker for merging)
iOS Conservative behavior cannot be fully E2E-verified until SDKRN-10 (the
session-replay-iosnative fix) ships and the XCFramework is vendored intoAmplitudeSessionReplay-iOS. Android Conservative behavior IS verifiable today (Android native heuristic already works againstReactTextView). This PR can merge independently — full E2E lives under parent SDKRN-9.Test plan
src/session-replay.ts,ios/...mm,ios/...swift, andandroid/.../...Module.ktMaskLevel.fromStringSwift extension and Kotlinwhenmapping match the sibling standalone packagegetDefaultConfig().maskLevel === MaskLevel.Mediumpnpm testandpnpm buildpassMade with Cursor
Note
Medium Risk
Changes the plugin’s public configuration surface and native bridge signatures to control masking behavior, which could affect what content is captured in session replays if misconfigured. Also touches iOS/Android native initialization paths, so regressions would show up at runtime rather than compile-time in JS.
Overview
Adds configurable masking to
@amplitude/plugin-session-replay-react-nativeby introducingPrivacyConfig/MaskLevel(Light/Medium/Conservative) with a default of Medium, and re-exporting these fromindex.tsx.Wires
privacyConfig.maskLevelthrough the JS pluginsetup()call as a new 9th positional argument and updates both iOS (ObjC selector + Swift mapping) and Android (Kotlin string→nativeMaskLevel+PrivacyConfig) bridges to apply it when constructing nativeSessionReplay.Updates docs and the example app to show the new
privacyConfig.maskLevelusage, adjusts Jest config to ignoreexample/and recognizetsx, and adds tests asserting defaults and that the mask level is forwarded correctly.Reviewed by Cursor Bugbot for commit 22e0544. Bugbot is set up for automated code reviews on this repo. Configure here.