Skip to content

Commit 6c5372c

Browse files
asalsyscursoragent
andauthored
refactor(web3auth): remove unnecessary useNavigation generic types (MetaMask#26571)
## **Description** **Reason for the change:** This is a preparatory change for upgrading the React Navigation library to v6. With global default types now applied to the navigation API, we no longer need to apply generic types to the `useNavigation` hook in most cases. **Improvement/Solution:** - Removed generic types from `useNavigation` hook invocations (now uses default global navigation types) - Applied `NavigationProp<NavigatableRootParamList>` where deeply nested navigation is required - Part of a series of PRs splitting the cleanup by codebase ownership **Note on exceptions:** Generic types may still be needed when navigating to deeper nested stacks. For those instances, we apply a recursive navigation type pattern. **References:** - Parent issue: MetaMask#23763 - React Navigation nesting docs: https://reactnavigation.org/docs/nesting-navigators/ ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: MetaMask#23763 (partial) ## **Manual testing steps** ```gherkin Feature: Navigation functionality after useNavigation cleanup Scenario: Web3auth/Onboarding navigation works correctly Given the app is running And the user is going through onboarding or social login flows When user navigates between onboarding screens Then the navigation should complete successfully And no TypeScript errors should occur ``` **Additional verification:** - Run `yarn lint:tsc` to verify no TypeScript errors are introduced - Run unit tests for affected components ## **Screenshots/Recordings** N/A - No visual changes (internal refactoring/type cleanup) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches navigation behavior across onboarding/social login and vault recovery flows by switching from `replace` to dispatching `StackActions.replace`, which can subtly change how navigation state updates. Risk is mitigated by expanded/updated unit tests covering the new action dispatching behavior. > > **Overview** > Refactors several onboarding/social-login and vault-recovery screens to stop using explicit `useNavigation` generic typings (and removes related `@react-navigation/stack` type imports) in preparation for React Navigation v6. > > Standardizes route replacement to use `navigation.dispatch(StackActions.replace(...))` instead of `navigation.replace(...)` in `Onboarding`, `SocialLoginIosUser`, and the vault recovery flow (`RestoreWallet`, `WalletResetNeeded`, `WalletRestored`), including the offline social-login error-sheet path. > > Updates and adds unit tests to match the new dispatch-based replace behavior (mocking `dispatch` and asserting `REPLACE` actions), and refactors `SocialLoginErrorSheet` tests for more robust rendering and async assertions. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6920402. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 7a47028 commit 6c5372c

13 files changed

Lines changed: 506 additions & 169 deletions

File tree

app/components/Views/OAuthRehydration/index.tsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,10 @@ import {
6767
import { useNetInfo } from '@react-native-community/netinfo';
6868
import { SuccessErrorSheetParams } from '../SuccessErrorSheet/interface';
6969
import { usePromptSeedlessRelogin } from '../../hooks/SeedlessHooks';
70-
import {
71-
ParamListBase,
72-
RouteProp,
73-
useNavigation,
74-
useRoute,
75-
} from '@react-navigation/native';
70+
import { RouteProp, useNavigation, useRoute } from '@react-navigation/native';
7671
import { useStyles } from '../../../component-library/hooks/useStyles';
7772
import stylesheet from './styles';
7873
import ReduxService from '../../../core/redux';
79-
import { StackNavigationProp } from '@react-navigation/stack';
8074
import OAuthService from '../../../core/OAuthService/OAuthService';
8175
import trackOnboarding from '../../../util/metrics/TrackOnboarding/trackOnboarding';
8276
import {
@@ -142,7 +136,7 @@ const OAuthRehydration: React.FC<OAuthRehydrationProps> = ({
142136
[loading, isDeletingInProgress],
143137
);
144138

145-
const navigation = useNavigation<StackNavigationProp<ParamListBase>>();
139+
const navigation = useNavigation();
146140
const {
147141
styles,
148142
theme: { themeAppearance },

app/components/Views/Onboarding/index.test.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,11 @@ const mockNav = {
258258
replace: mockReplace,
259259
reset: jest.fn(),
260260
setOptions: jest.fn(),
261+
dispatch: jest.fn((action) => {
262+
if (action.type === 'REPLACE') {
263+
mockReplace(action.payload.name, action.payload.params);
264+
}
265+
}),
261266
};
262267
jest.mock('@react-navigation/stack', () => ({
263268
createStackNavigator: () => ({
@@ -662,7 +667,10 @@ describe('Onboarding', () => {
662667
});
663668

664669
expect(Authentication.resetVault).toHaveBeenCalled();
665-
expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.HOME_NAV);
670+
expect(mockReplace).toHaveBeenCalledWith(
671+
Routes.ONBOARDING.HOME_NAV,
672+
undefined,
673+
);
666674
});
667675

668676
it('navigates to LOGIN when unlock is pressed and password is set', async () => {

app/components/Views/Onboarding/index.tsx

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ import {
5656
useNavigation,
5757
useRoute,
5858
RouteProp,
59-
ParamListBase,
59+
StackActions,
6060
} from '@react-navigation/native';
61-
import { StackNavigationProp } from '@react-navigation/stack';
6261
import {
6362
TraceName,
6463
TraceOperation,
@@ -123,7 +122,7 @@ interface OnboardingRouteParams {
123122
}
124123

125124
const Onboarding = () => {
126-
const navigation = useNavigation<StackNavigationProp<ParamListBase>>();
125+
const navigation = useNavigation();
127126
const route =
128127
useRoute<RouteProp<{ params: OnboardingRouteParams }, 'params'>>();
129128
const dispatch = useDispatch();
@@ -298,7 +297,7 @@ const Onboarding = () => {
298297
const onLogin = useCallback(async (): Promise<void> => {
299298
if (!passwordSet) {
300299
await Authentication.resetVault();
301-
navigation.replace(Routes.ONBOARDING.HOME_NAV);
300+
navigation.dispatch(StackActions.replace(Routes.ONBOARDING.HOME_NAV));
302301
} else {
303302
await Authentication.lockApp({ navigateToLogin: true });
304303
}
@@ -636,22 +635,26 @@ const Onboarding = () => {
636635
try {
637636
const netState = await netInfoFetch();
638637
if (!netState.isConnected || netState.isInternetReachable === false) {
639-
navigation.replace(Routes.MODAL.ROOT_MODAL_FLOW, {
640-
screen: Routes.SHEET.SUCCESS_ERROR_SHEET,
641-
params: {
642-
title: strings(`error_sheet.no_internet_connection_title`),
643-
description: strings(
644-
`error_sheet.no_internet_connection_description`,
645-
),
646-
descriptionAlign: 'left',
647-
buttonLabel: strings(`error_sheet.no_internet_connection_button`),
648-
primaryButtonLabel: strings(
649-
`error_sheet.no_internet_connection_button`,
650-
),
651-
closeOnPrimaryButtonPress: true,
652-
type: 'error',
653-
},
654-
});
638+
navigation.dispatch(
639+
StackActions.replace(Routes.MODAL.ROOT_MODAL_FLOW, {
640+
screen: Routes.SHEET.SUCCESS_ERROR_SHEET,
641+
params: {
642+
title: strings(`error_sheet.no_internet_connection_title`),
643+
description: strings(
644+
`error_sheet.no_internet_connection_description`,
645+
),
646+
descriptionAlign: 'left',
647+
buttonLabel: strings(
648+
`error_sheet.no_internet_connection_button`,
649+
),
650+
primaryButtonLabel: strings(
651+
`error_sheet.no_internet_connection_button`,
652+
),
653+
closeOnPrimaryButtonPress: true,
654+
type: 'error',
655+
},
656+
}),
657+
);
655658
return;
656659
}
657660
} catch (error) {
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import React from 'react';
2+
import { Image, ActivityIndicator } from 'react-native';
3+
import { fireEvent, waitFor } from '@testing-library/react-native';
4+
import RestoreWallet from './RestoreWallet';
5+
import Routes from '../../../constants/navigation/Routes';
6+
import renderWithProvider from '../../../util/test/renderWithProvider';
7+
import { MetaMetricsEvents } from '../../../core/Analytics';
8+
import EngineService from '../../../core/EngineService';
9+
import { strings } from '../../../../locales/i18n';
10+
11+
const mockReplace = jest.fn();
12+
const mockDispatch = jest.fn((action) => {
13+
if (action.type === 'REPLACE') {
14+
mockReplace(action.payload.name, action.payload.params);
15+
}
16+
});
17+
18+
jest.mock('@react-navigation/native', () => {
19+
const actualNav = jest.requireActual('@react-navigation/native');
20+
return {
21+
...actualNav,
22+
useNavigation: () => ({
23+
dispatch: mockDispatch,
24+
}),
25+
};
26+
});
27+
28+
jest.mock('../../../util/navigation/navUtils', () => ({
29+
createNavigationDetails: jest.fn(
30+
(routeName: string, nestedRouteName?: string) =>
31+
(params?: Record<string, unknown>) => [
32+
nestedRouteName ?? routeName,
33+
params,
34+
],
35+
),
36+
useParams: jest.fn(() => ({
37+
previousScreen: 'Login',
38+
})),
39+
}));
40+
41+
const mockTrackEvent = jest.fn();
42+
const mockCreateEventBuilder = jest.fn(() => ({
43+
addProperties: jest.fn().mockReturnThis(),
44+
build: jest.fn().mockReturnValue({ category: 'test' }),
45+
}));
46+
47+
jest.mock('../../../components/hooks/useMetrics', () => ({
48+
useMetrics: () => ({
49+
trackEvent: mockTrackEvent,
50+
createEventBuilder: mockCreateEventBuilder,
51+
}),
52+
}));
53+
54+
jest.mock('../../../util/metrics', () => jest.fn(() => ({ device: 'test' })));
55+
56+
jest.mock('../../../core/EngineService', () => ({
57+
initializeVaultFromBackup: jest.fn(),
58+
}));
59+
60+
describe('RestoreWallet', () => {
61+
beforeEach(() => {
62+
jest.clearAllMocks();
63+
});
64+
65+
describe('rendering', () => {
66+
it('renders title text', () => {
67+
const { getByText } = renderWithProvider(<RestoreWallet />);
68+
69+
expect(
70+
getByText(strings('restore_wallet.restore_needed_title')),
71+
).toBeOnTheScreen();
72+
});
73+
74+
it('renders description text', () => {
75+
const { getByText } = renderWithProvider(<RestoreWallet />);
76+
77+
expect(
78+
getByText(strings('restore_wallet.restore_needed_description')),
79+
).toBeOnTheScreen();
80+
});
81+
82+
it('renders Restore button', () => {
83+
const { getByText } = renderWithProvider(<RestoreWallet />);
84+
85+
expect(
86+
getByText(strings('restore_wallet.restore_needed_action')),
87+
).toBeOnTheScreen();
88+
});
89+
90+
it('renders device image', () => {
91+
const { UNSAFE_getByType } = renderWithProvider(<RestoreWallet />);
92+
93+
const imageElement = UNSAFE_getByType(Image);
94+
expect(imageElement).toBeTruthy();
95+
});
96+
});
97+
98+
describe('analytics tracking', () => {
99+
it('tracks screen viewed event on mount', () => {
100+
renderWithProvider(<RestoreWallet />);
101+
102+
expect(mockCreateEventBuilder).toHaveBeenCalledWith(
103+
MetaMetricsEvents.VAULT_CORRUPTION_RESTORE_WALLET_SCREEN_VIEWED,
104+
);
105+
expect(mockTrackEvent).toHaveBeenCalled();
106+
});
107+
});
108+
109+
describe('handleOnNext', () => {
110+
it('navigates to WalletRestored when restore succeeds', async () => {
111+
(EngineService.initializeVaultFromBackup as jest.Mock).mockResolvedValue({
112+
success: true,
113+
});
114+
const { getByText } = renderWithProvider(<RestoreWallet />);
115+
116+
fireEvent.press(
117+
getByText(strings('restore_wallet.restore_needed_action')),
118+
);
119+
120+
await waitFor(() => {
121+
expect(mockCreateEventBuilder).toHaveBeenCalledWith(
122+
MetaMetricsEvents.VAULT_CORRUPTION_RESTORE_WALLET_BUTTON_PRESSED,
123+
);
124+
expect(EngineService.initializeVaultFromBackup).toHaveBeenCalled();
125+
expect(mockDispatch).toHaveBeenCalled();
126+
expect(mockReplace).toHaveBeenCalledWith(
127+
Routes.VAULT_RECOVERY.WALLET_RESTORED,
128+
undefined,
129+
);
130+
});
131+
});
132+
133+
it('navigates to WalletResetNeeded when restore fails', async () => {
134+
(EngineService.initializeVaultFromBackup as jest.Mock).mockResolvedValue({
135+
success: false,
136+
});
137+
const { getByText } = renderWithProvider(<RestoreWallet />);
138+
139+
fireEvent.press(
140+
getByText(strings('restore_wallet.restore_needed_action')),
141+
);
142+
143+
await waitFor(() => {
144+
expect(EngineService.initializeVaultFromBackup).toHaveBeenCalled();
145+
expect(mockDispatch).toHaveBeenCalled();
146+
expect(mockReplace).toHaveBeenCalledWith(
147+
Routes.VAULT_RECOVERY.WALLET_RESET_NEEDED,
148+
undefined,
149+
);
150+
});
151+
});
152+
153+
it('shows loading indicator while restoring', async () => {
154+
let resolveRestore: (value: { success: boolean }) => void;
155+
const restorePromise = new Promise<{ success: boolean }>((resolve) => {
156+
resolveRestore = resolve;
157+
});
158+
(EngineService.initializeVaultFromBackup as jest.Mock).mockReturnValue(
159+
restorePromise,
160+
);
161+
const { getByText, UNSAFE_getByType } = renderWithProvider(
162+
<RestoreWallet />,
163+
);
164+
165+
fireEvent.press(
166+
getByText(strings('restore_wallet.restore_needed_action')),
167+
);
168+
169+
expect(UNSAFE_getByType(ActivityIndicator)).toBeTruthy();
170+
171+
// @ts-expect-error resolveRestore is assigned in Promise constructor
172+
resolveRestore({ success: true });
173+
174+
await waitFor(() => {
175+
expect(mockDispatch).toHaveBeenCalled();
176+
});
177+
});
178+
});
179+
});

app/components/Views/RestoreWallet/RestoreWallet.tsx

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,13 @@ import {
1414
import Routes from '../../../constants/navigation/Routes';
1515
import EngineService from '../../../core/EngineService';
1616
import { SafeAreaView } from 'react-native-safe-area-context';
17-
import { useNavigation } from '@react-navigation/native';
17+
import { useNavigation, StackActions } from '@react-navigation/native';
1818
import { useAppThemeFromContext } from '../../../util/theme';
1919
import { createWalletResetNeededNavDetails } from './WalletResetNeeded';
2020
import { createWalletRestoredNavDetails } from './WalletRestored';
2121
import { MetaMetricsEvents } from '../../../core/Analytics';
2222

2323
import generateDeviceAnalyticsMetaData from '../../../util/metrics';
24-
import { StackNavigationProp } from '@react-navigation/stack';
2524
import { useMetrics } from '../../../components/hooks/useMetrics';
2625

2726
/* eslint-disable import/no-commonjs, @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports */
@@ -50,9 +49,7 @@ const RestoreWallet = () => {
5049

5150
const [loading, setLoading] = useState<boolean>(false);
5251

53-
// TODO: Replace "any" with type
54-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
55-
const { replace } = useNavigation<StackNavigationProp<any>>();
52+
const navigation = useNavigation();
5653

5754
const deviceMetaData = useMemo(() => generateDeviceAnalyticsMetaData(), []);
5855
const { previousScreen } = useParams<RestoreWalletParams>();
@@ -79,13 +76,17 @@ const RestoreWallet = () => {
7976
);
8077
const restoreResult = await EngineService.initializeVaultFromBackup();
8178
if (restoreResult.success) {
82-
replace(...createWalletRestoredNavDetails());
79+
navigation.dispatch(
80+
StackActions.replace(...createWalletRestoredNavDetails()),
81+
);
8382
setLoading(false);
8483
} else {
85-
replace(...createWalletResetNeededNavDetails());
84+
navigation.dispatch(
85+
StackActions.replace(...createWalletResetNeededNavDetails()),
86+
);
8687
setLoading(false);
8788
}
88-
}, [deviceMetaData, replace, trackEvent, createEventBuilder]);
89+
}, [deviceMetaData, navigation, trackEvent, createEventBuilder]);
8990

9091
return (
9192
<SafeAreaView style={styles.screen}>

0 commit comments

Comments
 (0)