refactor(CDS-2123): migrate forwardRef to ref-as-prop in packages/mobile#741
Open
cb-ekuersch wants to merge 8 commits into
Open
refactor(CDS-2123): migrate forwardRef to ref-as-prop in packages/mobile#741cb-ekuersch wants to merge 8 commits into
cb-ekuersch wants to merge 8 commits into
Conversation
Apply the official React codemod and manual fixups to replace React.forwardRef wrapper pattern with ref-as-prop across 134 files in packages/mobile. Manual fixups cover: generic ForwardRefWithPeriod<Period> typings, React.ForwardRefExoticComponent aliases, displayName patterns, and re-exports. Test infrastructure changes are pending a resolution on React 19 SimpleMemoComponent fiber semantics that affect UNSAFE_*ByType queries and the isPressable accessibility helper. Co-authored-by: Erich Kuerschner <erich.kuerschner@coinbase.com>
Replace UNSAFE_*ByType(Component) reference-identity calls with role- and testID-based queries in Button, IconCounterButton, Text, and RollingNumber test files. Update isPressable/isText matchers in the accessibility helpers to dedup CDS wrapper nodes via findAll-length checks, matching the existing pattern in canBeDisabled. Add a stable testID to RollingNumber's screen-reader Text node to enable testID lookup. All 169 suites / 1828 tests pass; lint and typecheck clean. Co-authored-by: Erich Kuerschner <erich.kuerschner@coinbase.com>
- Add ref-forwarding tests for TextInput and ContentCard - Remove React.FC from Tour.tsx; use direct callable signatures - Widen TourStepArrowComponent type in common/useTour to accept ref-as-prop components without structural mismatch - Fix isText helper to only dedup when direct Text child shares the same onPress, accessibilityRole, and accessibilityLabel, preserving a11y violations on legitimate nested compositions - Remove shipped testID from RollingNumber SR-only Text node and update test to query via prop-based UNSAFE_queryAllByProps Co-authored-by: Erich Kuerschner <erich.kuerschner@coinbase.com>
…obile Tour Reverts the TourStepArrowComponent type change in packages/common so that packages/web (still on forwardRef) is not coupled to mobile's React 19 migration timing. packages/mobile/src/tour/Tour.tsx now handles the type mismatch via narrow `as unknown as TourStepArrowComponent` casts at the consumer boundary, with comments explaining the legacy forwardRef shape and the StyleProp<ViewStyle> vs Record<string,string|number> divergence. All validation gates pass: mobile lint, typecheck, build, test (1830 passed), common typecheck, and web typecheck. Co-authored-by: Erich Kuerschner <erich.kuerschner@coinbase.com>
… RollingNumber a11y tests Use `screen.queryAllByText(/.+/)` filtered by `accessibilityLiveRegion` prop to locate the SR-only Text node, eliminating the last UNSAFE_* query in RollingNumber.a11y.test.tsx. Visible and measured-digit Text nodes are excluded by RNTL's default `includeHiddenElements: false` because they carry `accessibilityElementsHidden` / `importantForAccessibility="no-hide-descendants"`, leaving only the screen-reader Text for the `.find()` predicate to match. Co-authored-by: Erich Kuerschner <erich.kuerschner@coinbase.com>
Collaborator
🟡 Heimdall Review Status
🟡
|
| Code Owner | Status | Calculation | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| ui-systems-eng-team |
🟡
0/1
|
Denominator calculation
|
Contributor
Conflicts resolved by preserving this branch's ref-as-prop pattern while incorporating master's new Select features (readOnly, labelColor, labelFont, inputBackground, borderRadius, accessibilityHint, handleToggleOpen) and Tabs' tabContainer style prop. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
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
2 participants
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.
What changed? Why?
Note
TL;DR: Removed
forwardRefwrappers acrosspackages/mobileand replaced them with the React 19 ref-as-prop pattern. This is a required migration ahead of the React 19 upgrade tracked in CDS-2123.Original, internal PR: https://coinbase.ghe.com/frontend/cds-internal/pull/151
The official React codemod was applied to all source files in
packages/mobile, convertingReact.forwardRef(fn)calls to direct ref-as-prop function signatures. Manual fixups cover cases the codemod cannot handle: genericForwardRefWithPeriod<Period>typings (e.g.SparklineInteractiveHoverDate),React.ForwardRefExoticComponenttype aliases,displayNameassignments, and re-exports.Test infrastructure updates
React 19 promotes
memo(fn)fromMemoComponent(fiber tag 14) toSimpleMemoComponent(fiber tag 15), which exposes the inner function's.nameonfiber.type. Two follow-on issues surfaced and were addressed without changing component runtime behavior:isPressable/isTextaccessibility helpers (packages/mobile/jest/accessibility/helpers.ts) previously matched both the CDS wrapper (Pressable/Text) and the leaf RN host node, producing duplicate matches and false positives in the a11y rule matchers. They now usenode.findAll(...)length-based dedup so only the leaf node is considered aPressable/Text. The 6 a11y matchers inrules.tswere updated to pass the node instead ofnode.type.UNSAFE_*ByType(Component)call sites broke because fiber reference identity changed underSimpleMemoComponent. Migrated four test files off type-identity queries:IconCounterButton.test.tsx:UNSAFE_queryAllByType(Pressable)→getAllByRole('button').Text.test.tsx:UNSAFE_queryAllByType(Text)→getAllByTestId('text-${fontName}').Button.test.tsx:UNSAFE_getByType(Text)→UNSAFE_getAllByProps({ testID: 'text-headline' })filtered by wrapper-only props (does not depend on fiber-type identity).RollingNumber.a11y.test.tsx: addedtestID="rolling-number-sr-only"to the screen-reader<Text>inRollingNumber.tsxand switched the helper toscreen.queryByTestId(...).UI changes
No visual changes — this is a pure code-pattern refactor with no behavioral or rendering differences.
Testing
How has it been tested?
Testing instructions
All four pass: 169 test suites / 1828 passing tests, 0 lint or type errors.
Illustrations/Icons Checklist
Not applicable — no changes to
packages/illustrations/**orpackages/icons/**.Change management
type=routine
risk=low
impact=sev5
automerge=false