refactor: remove ScreenComponent any casts for Redux/shared-Props components#28189
Conversation
…ponents Migrate OnboardingSuccess, AccountStatus, and RevealPrivateCredential from prop-based route access to useRoute() hook. - OnboardingSuccess: switch to useRoute(), remove ScreenProps interface - AccountStatus: switch to useRoute(), keep saveOnboardingEvent from Redux connect() - RevealPrivateCredential: switch to useRoute() and useNavigation(), make cancel optional, derive hasNavigation from cancel presence - Remove 6 as-casts from App.tsx (2 OnboardingSuccess, 3 AccountStatus, 1 RevealPrivateCredential) - Update all 3 test files to mock useRoute() Made-with: Cursor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
542eab8 to
d69d3cd
Compare
| const hasNavigation = !!navigation; | ||
| const navigation = useNavigation(); | ||
| const route = useRoute<RevealPrivateCredentialRouteProp>(); | ||
| const hasNavigation = !cancel; |
There was a problem hiding this comment.
hasNavigation logic changes ErrorBoundary embedded behavior
Medium Severity
Changing hasNavigation from !!navigation (prop-based) to !cancel alters behavior when both cancel and a navigation context exist. In ErrorBoundary/index.js, RevealPrivateCredential receives a cancel callback alongside a valid navigation context. Previously hasNavigation was true; now it's false. This causes handleLearnMoreClick to open an external browser instead of an in-app webview, and headerNavigationBack/navigateBack to call cancel() instead of navigation methods — a silent behavioral regression in the error-recovery SRP backup flow.
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 is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| }: IRevealPrivateCredentialProps) => { | ||
| const hasNavigation = !!navigation; | ||
| const navigation = useNavigation(); | ||
| const route = useRoute<RevealPrivateCredentialRouteProp>(); |
There was a problem hiding this comment.
Component crashes outside NavigationContainer after hook migration
High Severity
RevealPrivateCredential now unconditionally calls useNavigation() and useRoute(), which throw when rendered outside a NavigationContainer. The ErrorBoundary component (ErrorBoundary/index.js) imports and renders RevealPrivateCredential — error boundaries replace their children with fallback UI, potentially outside the NavigationContainer. Previously, the component accepted nullable navigation and route props to handle this case gracefully. The test 'renders when useNavigation returns null' masks this by mocking useNavigation to return null, but in production useNavigation() throws — it never returns null. This could cause a crash during error recovery (double fault).
Additional Locations (1)
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag selection rationale:
The changes are primarily TypeScript/hook refactoring with no new features, but the navigation behavior changes in RevealPrivateCredential warrant validation of the SRP reveal flow end-to-end. Performance Test Selection: |
|
|
✅ E2E Fixture Validation — Schema is up to date |
Description
Migrate 3 complex components from prop-based route access to
useRoute()hook (PR 11 of 13).Components:
OnboardingSuccess— simple migration, route was already optionalAccountStatus— Reduxconnect()component; removedroutefrom connected props, keptsaveOnboardingEventRevealPrivateCredential— most complex; switchedrouteandnavigationto hooks, madecanceloptional, derivedhasNavigationfromcancelpresence to preserve the dual-mode behavior (screen vs embedded)Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
N/A — pure refactoring. All 127 existing tests pass across 3 suites.
Screenshots/Recordings
Before
N/A
After
N/A
Pre-merge author checklist
Pre-merge reviewer checklist
Made with Cursor
Note
Medium Risk
Refactors several navigation-critical screens to use
useRoute()/useNavigation()and changes back-navigation behavior inRevealPrivateCredential, which could surface as runtime navigation/params issues if any route assumptions are wrong.Overview
Migrates
OnboardingSuccess,AccountStatus(Reduxconnect),OnboardingSheet, andRevealPrivateCredentialfrom prop-basedroute/navigationaccess touseRoute()/useNavigation(), removing severalScreenComponentcasts inApp.tsx.Updates navigation typing by adding
OnboardingSheetroute params toNavigationServicetypes, and adjustsRevealPrivateCredentialto useStackActions(pop/popToTop) vianavigation.dispatchwhile makingcanceloptional for embedded vs screen usage. Tests are refactored accordingly to mockuseRoute()params instead of passingrouteprops.Written by Cursor Bugbot for commit a1f46d0. This will update automatically on new commits. Configure here.