feat(session-replay-react-native): Fabric-native dual-architecture AmpMaskView#1886
Conversation
…ual-architecture component On the New Architecture the legacy Paper-only AmpMaskView was driven through RCTLegacyViewManagerInteropComponentView, whose out-of-band re-parenting mis-renders nested mask/unmask subtrees (react/react-native#44817-class, customer-verified). AmpMaskView is now a real Fabric component under the same native name (AMPMaskComponentView) and same public JS API: - codegen spec src/specs/AmpMaskViewNativeComponent.ts; on the old architecture codegenNativeComponent falls back to requireNativeComponent, resolving to the unchanged legacy Paper managers (zero old-arch change) - iOS: ios/fabric/AMPMaskComponentView (RCTViewComponentView, plain codegen descriptor), fully guarded by RCT_NEW_ARCH_ENABLED; masking applied through a plain-ObjC AmpMaskApplier seam (amp_isBlocked); prepareForRecycle resets masking state so recycled instances cannot leak markers - Android: newarch-only AmpMaskViewManager (ReactViewManager + codegen delegate) registered in place of the legacy manager; oldarch untouched - jest: codegenNativeComponent moduleNameMapper mock + component/spec tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
size-limit report 📦
|
…eens in example app MaskingScreen: layout parity vs plain View (onLayout-asserted), nested mask/unmask stacking (the legacy-interop Effect B trigger), and touch pass-through — each with an on-screen PASS/FAIL verdict. RecycleScreen: navigation-replace stress for Fabric view recycling; mask fills must stay inside their dashed outline slots across visits with alternating origins (the customer-reported stale-frame trigger). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t only The two parity wrappers are stacked siblings, so y necessarily differs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…intro, drop beta.2 removal note Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dc07885. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR upgrades @amplitude/session-replay-react-native’s <AmpMaskView> to be a real codegen/Fabric component on React Native’s New Architecture while preserving the same JS API and native component name (AMPMaskComponentView) for legacy builds.
Changes:
- Adds a codegen native component spec for
AMPMaskComponentViewand wiresAmpMaskViewto it (union-typed public prop, codegen-friendly native prop). - Implements New Architecture native components/managers for iOS (Fabric
RCTViewComponentView) and Android (newarchReactViewManager+ codegen delegate). - Adds Jest mocks/tests and example-app screens to validate layout parity, nested mask/unmask behavior, and recycling behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/session-replay-react-native/test/specs/AmpMaskViewNativeComponent.test.ts | Verifies the codegen spec registers the expected native component name under Jest. |
| packages/session-replay-react-native/test/amp-mask-view.test.tsx | Tests JS wrapper renders the codegen component and forwards props/children. |
| packages/session-replay-react-native/test/mocks/codegenNativeComponent.ts | Provides a Jest-safe mock for the deep codegenNativeComponent import path. |
| packages/session-replay-react-native/src/specs/AmpMaskViewNativeComponent.ts | Introduces the codegen component spec and native prop typing. |
| packages/session-replay-react-native/src/index.tsx | Exports AmpMaskViewProps alongside AmpMaskView. |
| packages/session-replay-react-native/src/amp-mask-view.tsx | Switches implementation to the codegen component while keeping the public union-typed required mask prop. |
| packages/session-replay-react-native/README.md | Documents that <AmpMaskView> behaves like <View> and works on both architectures. |
| packages/session-replay-react-native/package.json | Updates codegenConfig to generate components (type: "all") and adds iOS componentProvider entry. |
| packages/session-replay-react-native/jest.config.js | Adds moduleNameMapper for the deep codegen import path used by the spec. |
| packages/session-replay-react-native/ios/fabric/AMPMaskComponentView.mm | Adds Fabric RCTViewComponentView implementation for New Architecture masking + recycle reset. |
| packages/session-replay-react-native/ios/fabric/AMPMaskComponentView.h | Declares the Fabric component view class, guarded by RCT_NEW_ARCH_ENABLED. |
| packages/session-replay-react-native/ios/AmpMaskApplier.m | Adds an ObjC seam to apply/reset amp_isBlocked without importing Swift in ObjC++. |
| packages/session-replay-react-native/ios/AmpMaskApplier.h | Declares the masking/reset seam used by the Fabric component view. |
| packages/session-replay-react-native/example/App.tsx | Adds Masking/Recycle demo screens to validate layout nesting and Fabric recycling behavior. |
| packages/session-replay-react-native/android/src/newarch/java/com/amplitude/sessionreplayreactnative/SessionReplayReactNativePackage.kt | Registers the Fabric-native mask view manager on New Architecture builds. |
| packages/session-replay-react-native/android/src/newarch/java/com/amplitude/sessionreplayreactnative/fabric/AmpMaskViewManager.kt | Implements the New Architecture AMPMaskComponentView manager with codegen delegate routing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # packages/session-replay-react-native/android/src/newarch/java/com/amplitude/sessionreplayreactnative/SessionReplayReactNativePackage.kt # packages/session-replay-react-native/example/App.tsx # packages/session-replay-react-native/package.json # packages/session-replay-react-native/src/index.tsx
chungdaniel
left a comment
chungdaniel
left a comment
There was a problem hiding this comment.
LGTM other than one comment
…iewCls RN 0.76's generated RCTThirdPartyFabricComponentsProvider declares the Cls lookup with C linkage; the mangled C++ name left the app link failing with 'Undefined symbols: _AMPMaskComponentViewCls' on RN 0.76 + New Architecture. No effect on RN >= 0.77 (class resolved by name).
Verification results (2026-07-15) — full matrix greenBuild matrix (12/12 PASS): RN 0.73 (old arch) / 0.76 (old arch) / 0.79 / 0.83.6-Expo55 / 0.86 / 0.88-nightly × {Android, iOS}. Codegen clean across the range. RN 0.76 + New Arch iOS additionally links after Functional (example app, both platforms × both archs): layout parity vs plain Replay-content (captured rrweb payloads via dev-SDK DebugServer, both platforms New Arch + RN 0.79 + Expo 55/RN 0.83.6 lane):
Cloud replays for eyeball: Only skipped cell: RN 0.86 Android runtime privacy (dev-SDK harness Gradle-lockfile friction, not package code; covered by-proxy on 0.79 + 0.83.6). 🤖 Generated with Claude Code |
iOS runtime matrix completed (2026-07-16)Two iOS runtime payload cells that were skipped on the first pass (port contention) are now closed with dev-SDK DebugServer payload-level proof, both PASS:
(Note: the 0.86 iOS cell turned out to have never actually run earlier — its lane Podfile was stock-SDK, no DebugServer — so this is a genuine first verification, not just a re-run.) Net: iOS New-Arch masking is now directly payload-verified on 0.79, 0.83.6, and 0.86, in addition to all compile/link cells 0.73→nightly. Nested-unmask redacted per documented iOS SDK design; no raw marker text in any payload. 🤖 Generated with Claude Code |
- resetView (iOS recycle) now fails closed to masked instead of unmasked: amp_isBlocked is a bool with no inherit/none state, so a recycled view can only reset to masked or unmasked; masked errs toward hiding content if a reused instance is captured before updateProps re-applies. Matches applyMask's unknown-value policy and the masked default. Corrects the header/comment that claimed it 'clears' state. - lint/fix globs now cover *.tsx (were *.ts only), so src/index.tsx, src/amp-mask-view.tsx and test/amp-mask-view.test.tsx are linted. - README: fix 'maks' -> 'mask' typo.
Mercy811
left a comment
Mercy811
left a comment
There was a problem hiding this comment.
It's weird that it still need dx team approval after @chungdaniel's approval. I confirm he is in the team. Approve here to unblock but did't review this PR.
Summary
Stacked on #1885. Makes
<AmpMaskView>a real Fabric component on the New Architecture while keeping the old-architecture path byte-identical — same public JS API, same native component name (AMPMaskComponentView).Why
On the New Architecture the legacy Paper-only
AmpMaskViewis driven throughRCTLegacyViewManagerInteropComponentView, whose out-of-band re-parenting mis-renders nestedmask ⊃ unmasksubtrees (June root-cause, react/react-native#44817-class; re-confirmed by a customer report 2026-07-15). A plain-layout-node Fabric component with this exact shape was verified immune in the customer's own environment (RN 0.83.6, Expo 55, New Arch) — including Fabric view recycling, which it survives via an explicitprepareForRecyclemasking reset.How
src/specs/AmpMaskViewNativeComponent.ts(mask: WithDefault<string,'amp-mask'>);src/amp-mask-view.tsxre-exports it typed with the required union prop. On old archcodegenNativeComponentfalls back torequireNativeComponent('AMPMaskComponentView')→ the unchanged legacy managers.ios/fabric/AMPMaskComponentView.{h,mm}—RCTViewComponentViewsubclass, codegen descriptor (plain Yoga, no custom ShadowNode), whole files guarded byRCT_NEW_ARCH_ENABLED(podspec globsios/**unconditionally). Masking goes through a plain-ObjCAmpMaskApplierseam (amp_isBlocked) to keep the Swift-pod import out of ObjC++.prepareForRecycleresets masking state.fabric/AmpMaskViewManager.kt(ReactViewManager+ codegenAMPMaskComponentViewManagerDelegate) registered by the newarch package in place of the legacy manager; oldarch source set untouched. No build.gradle changes.type: "modules"→"all"+ iOScomponentProviderentry. Triple registration on iOS (componentProvider /AMPMaskComponentViewCls/+load) for RN-version coverage.moduleNameMappermock for the codegen deep path; component + spec tests.jest55/55,typecheck,lint,buildall green.Semantics notes
maskvalues fail safe to masking on the Fabric path (legacy Paper no-ops on unknown — unchanged there).🤖 Generated with Claude Code
Note
Medium Risk
Changes session-replay privacy masking on New Architecture (nested mask/unmask and Fabric recycling); incorrect behavior could briefly expose or mis-layout sensitive UI, though old-arch paths are untouched and recycle handling is fail-closed.
Overview
<AmpMaskView>becomes a real Fabric component on React Native’s New Architecture while the public API and native name (AMPMaskComponentView) stay the same; legacy Paper managers are unchanged on old architecture.JS now wires the view through a codegen spec (
codegenNativeComponent) instead ofrequireNativeComponentalone, withcodegenConfigexpanded totype: "all"plus an iOScomponentProvider. iOS adds a FabricRCTViewComponentViewsubclass that applies masking viaAmpMaskApplier, resets to masked inprepareForRecycle, and registers for multiple RN versions. Android newarch registersAmpMaskViewManagerwith the codegen delegate instead of the legacy view manager in the newarch package only.The Fabric path fails closed on unknown
maskvalues (masking) and on recycle reset. Docs note layout parity with<View>; the example app adds Masking and Recycle screens for manual verification. Jest gains acodegenNativeComponentmock and component/spec tests; lint scripts includetsx.Reviewed by Cursor Bugbot for commit 09196ff. Bugbot is set up for automated code reviews on this repo. Configure here.