From 47ba5a025c822ae535f65cbc5fb3d9f4be5ae20d Mon Sep 17 00:00:00 2001 From: dddddanica Date: Thu, 6 Nov 2025 16:29:00 +0000 Subject: [PATCH 01/13] refactor(556x): migrate unlock, deeplink, defi page to react-router-dom-v5-compat --- ui/contexts/metametrics.js | 15 +- .../initialized/initialized-v5-compat.tsx | 37 ++++ ui/pages/deep-link/deep-link.tsx | 9 +- .../components/defi-details-list.test.tsx | 2 +- .../components/defi-details-page.test.tsx | 21 +- .../defi/components/defi-details-page.tsx | 19 +- ui/pages/keychains/reveal-seed.js | 17 +- ui/pages/keychains/reveal-seed.test.js | 42 ++-- ui/pages/lock/lock.component.js | 8 +- ui/pages/lock/lock.container.js | 4 +- ui/pages/lock/lock.test.js | 19 +- ui/pages/routes/routes.component.tsx | 92 ++++++-- ui/pages/unlock-page/unlock-page.component.js | 26 ++- ui/pages/unlock-page/unlock-page.container.js | 35 ++- ui/pages/unlock-page/unlock-page.test.js | 204 +++++++++++++----- 15 files changed, 384 insertions(+), 166 deletions(-) create mode 100644 ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index d37cdccec602..fd6567083f63 100644 --- a/ui/contexts/metametrics.js +++ b/ui/contexts/metametrics.js @@ -12,7 +12,7 @@ import React, { useContext, } from 'react'; import PropTypes from 'prop-types'; -import { matchPath, useLocation } from 'react-router-dom'; +import { matchPath, useLocation } from 'react-router-dom-v5-compat'; import { useSelector } from 'react-redux'; import { omit } from 'lodash'; @@ -157,11 +157,14 @@ export function MetaMetricsProvider({ children }) { */ useEffect(() => { const environmentType = getEnvironmentType(); - const match = matchPath(location.pathname, { - path: getPaths(), - exact: true, - strict: true, - }); + const match = matchPath( + { + path: getPaths(), + end: true, + caseSensitive: true, + }, + location.pathname, + ); // Start by checking for a missing match route. If this falls through to // the else if, then we know we have a matched route for tracking. if (!match) { diff --git a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx new file mode 100644 index 000000000000..8521f946203b --- /dev/null +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx @@ -0,0 +1,37 @@ +import React from 'react'; +import { useSelector } from 'react-redux'; +import { Navigate } from 'react-router-dom-v5-compat'; +import { ONBOARDING_ROUTE } from '../../constants/routes'; + +type InitializedV5CompatProps = { + children: React.ReactNode; +}; + +/** + * InitializedV5Compat - A wrapper component for v5-compat routes that require initialization + * + * This component checks if the user has completed onboarding. + * If not, it redirects to the onboarding route using v5-compat Navigate. + * + * Unlike the v5 Initialized HOC, this returns the element directly (not wrapped in Route) + * because v5-compat Routes handle their children differently. + * + * @param props - Component props + * @param props.children - Child components to render when initialized + * @returns Navigate component or children + */ +const InitializedV5Compat = ({ children }: InitializedV5CompatProps) => { + const completedOnboarding = useSelector( + (state: { metamask: { completedOnboarding: boolean } }) => + state.metamask.completedOnboarding, + ); + + if (!completedOnboarding) { + return ; + } + + return <>{children}; +}; + +export default InitializedV5Compat; + diff --git a/ui/pages/deep-link/deep-link.tsx b/ui/pages/deep-link/deep-link.tsx index fa7dd737c1e8..65975c53ed9a 100644 --- a/ui/pages/deep-link/deep-link.tsx +++ b/ui/pages/deep-link/deep-link.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useRef, useState } from 'react'; -import { useLocation } from 'react-router-dom'; +import type { Location } from 'react-router-dom'; import log from 'loglevel'; import { useDispatch, useSelector } from 'react-redux'; import { @@ -154,8 +154,11 @@ async function updateStateFromUrl( } } -export const DeepLink = () => { - const location = useLocation(); +type DeepLinkProps = { + location: Location; +}; + +export const DeepLink = ({ location }: DeepLinkProps) => { const t = useI18nContext() as TranslateFunction; const dispatch = useDispatch(); // it's technically not possible for a natural flow to reach this page diff --git a/ui/pages/defi/components/defi-details-list.test.tsx b/ui/pages/defi/components/defi-details-list.test.tsx index eb5158b54781..c7cce1096d5b 100644 --- a/ui/pages/defi/components/defi-details-list.test.tsx +++ b/ui/pages/defi/components/defi-details-list.test.tsx @@ -22,7 +22,7 @@ jest.mock('react-router-dom-v5-compat', () => { describe('DeFiDetailsPage', () => { const store = configureMockStore([thunk])(mockState); - beforeAll(() => { + beforeEach(() => { jest.clearAllMocks(); }); diff --git a/ui/pages/defi/components/defi-details-page.test.tsx b/ui/pages/defi/components/defi-details-page.test.tsx index e8d9a6dc9607..7b2d72092fd8 100644 --- a/ui/pages/defi/components/defi-details-page.test.tsx +++ b/ui/pages/defi/components/defi-details-page.test.tsx @@ -6,16 +6,7 @@ import { renderWithProvider } from '../../../../test/lib/render-helpers-navigate import mockState from '../../../../test/data/mock-state.json'; import DeFiPage from './defi-details-page'; -const mockUseParams = jest - .fn() - .mockReturnValue({ chainId: CHAIN_IDS.MAINNET, protocolId: 'aave' }); - -jest.mock('react-router-dom', () => { - return { - ...jest.requireActual('react-router-dom'), - useParams: () => mockUseParams(), - }; -}); +const mockNavigate = jest.fn(); describe('DeFiDetailsPage', () => { const mockStore = { @@ -79,7 +70,7 @@ describe('DeFiDetailsPage', () => { const store = configureMockStore([thunk])(mockStore); - beforeAll(() => { + beforeEach(() => { jest.clearAllMocks(); }); @@ -89,7 +80,13 @@ describe('DeFiDetailsPage', () => { }); it('renders defi asset page', () => { - const { container } = renderWithProvider(, store); + const { container } = renderWithProvider( + , + store, + ); expect(container).toMatchSnapshot(); }); diff --git a/ui/pages/defi/components/defi-details-page.tsx b/ui/pages/defi/components/defi-details-page.tsx index 975f3702c0dd..a2c1300b0f5a 100644 --- a/ui/pages/defi/components/defi-details-page.tsx +++ b/ui/pages/defi/components/defi-details-page.tsx @@ -1,5 +1,5 @@ import React, { useMemo } from 'react'; -import { useHistory, useParams, Redirect } from 'react-router-dom'; +import { Navigate } from 'react-router-dom-v5-compat'; import { useSelector } from 'react-redux'; import { Display, @@ -45,16 +45,17 @@ const useExtractUnderlyingTokens = ( ); }, [positions]); -const DeFiPage = () => { - const { chainId, protocolId } = useParams<{ - chainId: '0x' & string; - protocolId: string; - }>() as { chainId: '0x' & string; protocolId: string }; +type DeFiPageProps = { + navigate: (to: string | number) => void; + params: { chainId: string; protocolId: string }; +}; + +const DeFiPage = ({ navigate, params }: DeFiPageProps) => { const { formatCurrencyWithMinThreshold } = useFormatters(); + const { chainId, protocolId } = params; const defiPositions = useSelector(getDefiPositions); const selectedAccount = useSelector(getSelectedAccount); - const history = useHistory(); const t = useI18nContext(); const { privacyMode } = useSelector(getPreferences); @@ -82,7 +83,7 @@ const DeFiPage = () => { }; if (!protocolPosition) { - return ; + return ; } return ( @@ -100,7 +101,7 @@ const DeFiPage = () => { size={ButtonIconSize.Sm} ariaLabel={t('back')} iconName={IconName.ArrowLeft} - onClick={() => history.push(DEFAULT_ROUTE)} + onClick={() => navigate(DEFAULT_ROUTE)} /> diff --git a/ui/pages/keychains/reveal-seed.js b/ui/pages/keychains/reveal-seed.js index 91189c9a7397..30214264f4ab 100644 --- a/ui/pages/keychains/reveal-seed.js +++ b/ui/pages/keychains/reveal-seed.js @@ -1,7 +1,7 @@ import qrCode from 'qrcode-generator'; import React, { useContext, useEffect, useState, useCallback } from 'react'; +import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; -import { useHistory, useParams } from 'react-router-dom'; import { getErrorMessage } from '../../../shared/modules/error'; import { MetaMetricsEventCategory, @@ -44,9 +44,7 @@ import { endTrace, trace, TraceName } from '../../../shared/lib/trace'; const PASSWORD_PROMPT_SCREEN = 'PASSWORD_PROMPT_SCREEN'; const REVEAL_SEED_SCREEN = 'REVEAL_SEED_SCREEN'; -export default function RevealSeedPage() { - const history = useHistory(); - const { keyringId } = useParams(); +function RevealSeedPage({ navigate, keyringId }) { const dispatch = useDispatch(); const t = useI18nContext(); const trackEvent = useContext(MetaMetricsContext); @@ -267,7 +265,7 @@ export default function RevealSeedPage() { hd_entropy_index: hdEntropyIndex, }, }); - history.goBack(); + navigate(-1); }} > {t('cancel')} @@ -316,7 +314,7 @@ export default function RevealSeedPage() { key_type: MetaMetricsEventKeyType.Srp, }, }); - history.goBack(); + navigate(-1); }} > {t('close')} @@ -409,3 +407,10 @@ export default function RevealSeedPage() { ); } + +RevealSeedPage.propTypes = { + navigate: PropTypes.func.isRequired, + keyringId: PropTypes.string, +}; + +export default RevealSeedPage; diff --git a/ui/pages/keychains/reveal-seed.test.js b/ui/pages/keychains/reveal-seed.test.js index 85db34343d56..2306f4960234 100644 --- a/ui/pages/keychains/reveal-seed.test.js +++ b/ui/pages/keychains/reveal-seed.test.js @@ -2,7 +2,7 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { fireEvent, waitFor } from '@testing-library/react'; import thunk from 'redux-thunk'; -import { renderWithProvider } from '../../../test/lib/render-helpers'; +import { renderWithProvider } from '../../../test/lib/render-helpers-navigate'; import mockState from '../../../test/data/mock-state.json'; import { MetaMetricsContext } from '../../contexts/metametrics'; import { @@ -29,9 +29,7 @@ const mockRequestRevealSeedWords = jest .fn() .mockImplementation(mockSuccessfulSrpReveal); const mockShowModal = jest.fn(); -const mockUseParams = jest - .fn() - .mockReturnValue({ keyringId: 'ULID01234567890ABCDEFGHIJKLMN' }); +const mockNavigate = jest.fn(); const password = 'password'; jest.mock('../../store/actions.ts', () => ({ @@ -40,15 +38,6 @@ jest.mock('../../store/actions.ts', () => ({ mockRequestRevealSeedWords(userPassword, keyringId), })); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useHistory: () => ({ - push: jest.fn(), - goBack: jest.fn(), - }), - useParams: () => mockUseParams(), -})); - const mockStateWithModal = { ...mockState, appState: { @@ -68,19 +57,26 @@ const mockStateWithModal = { describe('Reveal Seed Page', () => { const mockStore = configureMockStore([thunk])(mockStateWithModal); + beforeEach(() => { + jest.clearAllMocks(); + }); + afterEach(() => { jest.clearAllMocks(); }); it('should match snapshot', () => { - const { container } = renderWithProvider(, mockStore); + const { container } = renderWithProvider( + , + mockStore, + ); expect(container).toMatchSnapshot(); }); it('form submit', async () => { const { queryByTestId, queryByText } = renderWithProvider( - , + , mockStore, ); @@ -97,7 +93,7 @@ describe('Reveal Seed Page', () => { it('shows hold to reveal', async () => { const { queryByTestId, queryByText } = renderWithProvider( - , + , mockStore, ); @@ -116,7 +112,7 @@ describe('Reveal Seed Page', () => { mockRequestRevealSeedWords.mockImplementation(mockUnsuccessfulSrpReveal); const { queryByTestId, queryByText } = renderWithProvider( - , + , mockStore, ); @@ -138,7 +134,7 @@ describe('Reveal Seed Page', () => { const { queryByTestId, queryByText } = renderWithProvider(
- +
, store, ); @@ -168,7 +164,7 @@ describe('Reveal Seed Page', () => { renderWithProvider( - + , store, ); @@ -344,7 +340,7 @@ describe('Reveal Seed Page', () => { const mockTrackEvent = jest.fn(); const { queryByText } = renderWithProvider( - + , mockStore, ); @@ -375,9 +371,8 @@ describe('Reveal Seed Page', () => { describe('multi-srp', () => { it('passes the keyringId to the requestRevealSeedWords action', async () => { const keyringId = 'ULID01234567890ABCDEFGHIJKLMN'; - mockUseParams.mockReturnValue({ keyringId }); const { queryByTestId, queryByText } = renderWithProvider( - , + , mockStore, ); @@ -397,9 +392,8 @@ describe('Reveal Seed Page', () => { it('passes undefined for keyringId if there is no param', async () => { const keyringId = undefined; - mockUseParams.mockReturnValue({ keyringId }); const { queryByTestId, queryByText } = renderWithProvider( - , + , mockStore, ); diff --git a/ui/pages/lock/lock.component.js b/ui/pages/lock/lock.component.js index 4b801f5404da..8a1178b6c633 100644 --- a/ui/pages/lock/lock.component.js +++ b/ui/pages/lock/lock.component.js @@ -5,18 +5,18 @@ import { DEFAULT_ROUTE } from '../../helpers/constants/routes'; export default class Lock extends PureComponent { static propTypes = { - history: PropTypes.object, + navigate: PropTypes.func, isUnlocked: PropTypes.bool, lockMetamask: PropTypes.func, }; componentDidMount() { - const { lockMetamask, isUnlocked, history } = this.props; + const { lockMetamask, isUnlocked, navigate } = this.props; if (isUnlocked) { - lockMetamask().then(() => history.push(DEFAULT_ROUTE)); + lockMetamask().then(() => navigate(DEFAULT_ROUTE)); } else { - history.replace(DEFAULT_ROUTE); + navigate(DEFAULT_ROUTE, { replace: true }); } } diff --git a/ui/pages/lock/lock.container.js b/ui/pages/lock/lock.container.js index c029d8ddcee8..72da89709d56 100644 --- a/ui/pages/lock/lock.container.js +++ b/ui/pages/lock/lock.container.js @@ -1,7 +1,7 @@ import { compose } from 'redux'; import { connect } from 'react-redux'; -import { withRouter } from 'react-router-dom'; import { lockMetamask } from '../../store/actions'; +import withRouterHooks from '../../helpers/higher-order-components/with-router-hooks/with-router-hooks'; import Lock from './lock.component'; const mapStateToProps = (state) => { @@ -21,6 +21,6 @@ const mapDispatchToProps = (dispatch) => { }; export default compose( - withRouter, + withRouterHooks, connect(mapStateToProps, mapDispatchToProps), )(Lock); diff --git a/ui/pages/lock/lock.test.js b/ui/pages/lock/lock.test.js index afdd6e238659..100439dc9bbc 100644 --- a/ui/pages/lock/lock.test.js +++ b/ui/pages/lock/lock.test.js @@ -1,29 +1,30 @@ import React from 'react'; import sinon from 'sinon'; -import { renderWithProvider } from '../../../test/lib/render-helpers'; +import { renderWithProvider } from '../../../test/lib/render-helpers-navigate'; import Lock from './lock.component'; +const mockUseNavigate = jest.fn(); + describe('Lock', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); it('replaces history with default route when isUnlocked false', () => { const props = { isUnlocked: false, - history: { - replace: sinon.spy(), - }, + navigate: mockUseNavigate, }; renderWithProvider(); - expect(props.history.replace.getCall(0).args[0]).toStrictEqual('/'); + expect(mockUseNavigate).toHaveBeenCalledWith('/', { replace: true }); }); it('locks and pushes history with default route when isUnlocked true', async () => { const props = { isUnlocked: true, lockMetamask: sinon.stub(), - history: { - push: sinon.spy(), - }, + navigate: mockUseNavigate, }; props.lockMetamask.resolves(); @@ -31,6 +32,6 @@ describe('Lock', () => { renderWithProvider(); expect(await props.lockMetamask.calledOnce).toStrictEqual(true); - expect(props.history.push.getCall(0).args[0]).toStrictEqual('/'); + expect(mockUseNavigate).toHaveBeenCalledWith('/'); }); }); diff --git a/ui/pages/routes/routes.component.tsx b/ui/pages/routes/routes.component.tsx index ab921d7221eb..b1019097ebff 100644 --- a/ui/pages/routes/routes.component.tsx +++ b/ui/pages/routes/routes.component.tsx @@ -18,6 +18,7 @@ import { useAppSelector } from '../../store/store'; import Authenticated from '../../helpers/higher-order-components/authenticated'; import AuthenticatedV5Compat from '../../helpers/higher-order-components/authenticated/authenticated-v5-compat'; import Initialized from '../../helpers/higher-order-components/initialized'; +import InitializedV5Compat from '../../helpers/higher-order-components/initialized/initialized-v5-compat'; import Loading from '../../components/ui/loading-screen'; import { Modal } from '../../components/app/modals'; import Alert from '../../components/ui/alert'; @@ -617,9 +618,34 @@ export default function Routes() { {/** @ts-expect-error TODO: Replace `component` prop with `element` once `react-router` is upgraded to v6 */} - - {/** @ts-expect-error TODO: Replace `component` prop with `element` once `react-router` is upgraded to v6 */} - + + {(props: RouteComponentProps) => { + const { history: v5History, location: v5Location } = props; + const navigate = createV5CompatNavigate(v5History); + + const UnlockPageComponent = UnlockPage as React.ComponentType<{ + navigate: V5CompatNavigate; + location: RouteComponentProps['location']; + }>; + return ( + + + + ); + }} + + + {(props: RouteComponentProps) => { + const { location: v5Location } = props; + const DeepLinkComponent = DeepLink as React.ComponentType<{ + location: RouteComponentProps['location']; + }>; + return ; + }} + - + + {(props: RouteComponentProps<{ keyringId?: string }>) => { + const { history: v5History, match } = props; + const navigate = createV5CompatNavigate(v5History); + + const RevealSeedConfirmationComponent = + RevealSeedConfirmation as React.ComponentType<{ + navigate: V5CompatNavigate; + keyringId?: string; + }>; + return ( + ( + + )} + /> + ); + }} + - + + {( + props: RouteComponentProps<{ + chainId: string; + protocolId: string; + }>, + ) => { + const { history: v5History, match } = props; + const navigate = createV5CompatNavigate(v5History); + + const DeFiPageComponent = DeFiPage as React.ComponentType<{ + navigate: V5CompatNavigate; + params: { chainId: string; protocolId: string }; + }>; + return ( + ( + + )} + /> + ); + }} + { - const { isSocialLoginFlow, history, isOnboardingCompleted } = this.props; + const { isSocialLoginFlow, navigate, isOnboardingCompleted } = this.props; // in `onboarding_unlock` route, if the user is on a social login flow and onboarding is not completed, // we can redirect to `onboarding_welcome` route to select a different login method @@ -431,7 +437,7 @@ class UnlockPage extends Component { await this.props.loginWithDifferentMethod(); await this.props.forceUpdateMetamaskState(); - history.replace(ONBOARDING_WELCOME_ROUTE); + navigate(ONBOARDING_WELCOME_ROUTE, { replace: true }); return; } @@ -463,7 +469,7 @@ class UnlockPage extends Component { this.setState({ showLoginErrorModal: false }); await this.props.resetWallet(); await this.props.forceUpdateMetamaskState(); - this.props.history.replace(DEFAULT_ROUTE); + this.props.navigate(DEFAULT_ROUTE, { replace: true }); }; render() { diff --git a/ui/pages/unlock-page/unlock-page.container.js b/ui/pages/unlock-page/unlock-page.container.js index d247d4dcb571..18aa9cdd8f73 100644 --- a/ui/pages/unlock-page/unlock-page.container.js +++ b/ui/pages/unlock-page/unlock-page.container.js @@ -1,5 +1,4 @@ import { connect } from 'react-redux'; -import { withRouter } from 'react-router-dom'; import { compose } from 'redux'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths @@ -24,6 +23,9 @@ import { } from '../../selectors'; import { getCompletedOnboarding } from '../../ducks/metamask/metamask'; import UnlockPage from './unlock-page.component'; +import withRouterHooks from "../../helpers/higher-order-components/with-router-hooks/with-router-hooks"; +import React from 'react'; +import { useNavState } from '../../contexts/navigation-state'; const mapStateToProps = (state) => { const { @@ -57,15 +59,16 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { ...restDispatchProps } = dispatchProps; const { - history, + navigate, onSubmit: ownPropsSubmit, location, + navState, ...restOwnProps } = ownProps; const onImport = async () => { await propsMarkPasswordForgotten(); - history.push(RESTORE_VAULT_ROUTE); + navigate(RESTORE_VAULT_ROUTE); if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) { global.platform.openExtensionInBrowser?.(RESTORE_VAULT_ROUTE); @@ -76,11 +79,13 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { await propsTryUnlockMetamask(password); // Redirect to the intended route if available, otherwise DEFAULT_ROUTE let redirectTo = DEFAULT_ROUTE; - if (location.state?.from?.pathname) { - const search = location.state.from.search || ''; - redirectTo = location.state.from.pathname + search; + // Read from both v5 location.state and v5-compat navState + const fromLocation = location.state?.from || navState?.from; + if (fromLocation?.pathname) { + const search = fromLocation.search || ''; + redirectTo = fromLocation.pathname + search; } - history.push(redirectTo); + navigate(redirectTo); }; return { @@ -89,12 +94,22 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { ...restOwnProps, onRestore: onImport, onSubmit: ownPropsSubmit || onSubmit, - history, + navigate, location, + navState, }; }; -export default compose( - withRouter, +const UnlockPageConnected = compose( + withRouterHooks, connect(mapStateToProps, mapDispatchToProps, mergeProps), )(UnlockPage); + +// Inject navState from NavigationStateContext for v5-compat navigation +// eslint-disable-next-line react/prop-types +const UnlockPageWithNavState = (props) => { + const navState = useNavState(); + return ; +}; + +export default UnlockPageWithNavState; diff --git a/ui/pages/unlock-page/unlock-page.test.js b/ui/pages/unlock-page/unlock-page.test.js index 92f3e0ac2bf0..d0522992213a 100644 --- a/ui/pages/unlock-page/unlock-page.test.js +++ b/ui/pages/unlock-page/unlock-page.test.js @@ -2,14 +2,27 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { fireEvent, waitFor } from '@testing-library/react'; import thunk from 'redux-thunk'; -import { Router } from 'react-router-dom'; -import { createMemoryHistory } from 'history'; import { SeedlessOnboardingControllerErrorMessage } from '@metamask/seedless-onboarding-controller'; -import { renderWithProvider } from '../../../test/lib/render-helpers'; +import { renderWithProvider } from '../../../test/lib/render-helpers-navigate'; import { ONBOARDING_WELCOME_ROUTE } from '../../helpers/constants/routes'; import { FirstTimeFlowType } from '../../../shared/constants/onboarding'; import UnlockPage from '.'; +const mockUseNavigate = jest.fn(); +jest.mock('react-router-dom-v5-compat', () => { + return { + ...jest.requireActual('react-router-dom-v5-compat'), + useNavigate: () => mockUseNavigate, + }; +}); + +const mockUseNavState = jest.fn(() => null); +jest.mock('../../contexts/navigation-state', () => ({ + useNavState: () => mockUseNavState(), + useSetNavState: () => jest.fn(), + NavigationStateProvider: ({ children }) => children, +})); + const mockTryUnlockMetamask = jest.fn(() => { return async () => { return Promise.resolve(); @@ -47,6 +60,11 @@ describe('Unlock Page', () => { }; const mockStore = configureMockStore([thunk])(mockState); + beforeEach(() => { + jest.clearAllMocks(); + mockUseNavState.mockReturnValue(null); // Reset navState to null for each test + }); + it('should match snapshot', () => { const { container } = renderWithProvider(, mockStore); @@ -111,11 +129,6 @@ describe('Unlock Page', () => { }; const store = configureMockStore([thunk])(mockStateWithUnlock); - const history = createMemoryHistory({ - initialEntries: [{ pathname: '/unlock' }], - }); - - jest.spyOn(history, 'replace'); const mockLoginWithDifferentMethod = jest.fn(); const mockForceUpdateMetamaskState = jest.fn(); @@ -125,10 +138,9 @@ describe('Unlock Page', () => { }; const { queryByText } = renderWithProvider( - - - , + , store, + '/unlock', ); fireEvent.click(queryByText('Use a different login method')); @@ -136,55 +148,72 @@ describe('Unlock Page', () => { await waitFor(() => { expect(mockLoginWithDifferentMethod).toHaveBeenCalled(); expect(mockForceUpdateMetamaskState).toHaveBeenCalled(); - expect(history.replace).toHaveBeenCalledWith(ONBOARDING_WELCOME_ROUTE); + expect(mockUseNavigate).toHaveBeenCalledWith(ONBOARDING_WELCOME_ROUTE, { + replace: true, + }); }); }); + it('should redirect to history location when unlocked (from state)', () => { + const intendedPath = '/previous-route'; + const mockStateWithUnlock = { + metamask: { isUnlocked: true }, + }; + const store = configureMockStore([thunk])(mockStateWithUnlock); + + // Set up the router to have the location state that would come from a redirect + const pathname = '/unlock'; + const locationState = { from: { pathname: intendedPath } }; - it('should redirect to history location when unlocked', () => { + renderWithProvider(, store, { + pathname, + state: locationState, + }); + + expect(mockUseNavigate).toHaveBeenCalledTimes(1); + expect(mockUseNavigate).toHaveBeenCalledWith(intendedPath); + }); + + it('should redirect to context-stored location when unlocked (HashRouter v5-compat workaround)', () => { const intendedPath = '/previous-route'; const mockStateWithUnlock = { metamask: { isUnlocked: true }, }; const store = configureMockStore([thunk])(mockStateWithUnlock); - const history = createMemoryHistory({ - initialEntries: [ - { pathname: '/unlock', state: { from: { pathname: intendedPath } } }, - ], + + const pathname = '/unlock'; + + // Mock navState to provide the redirect location (v5-compat way) + mockUseNavState.mockReturnValueOnce({ + from: { pathname: intendedPath }, }); - jest.spyOn(history, 'push'); - renderWithProvider( - - - , - store, - ); - expect(history.push).toHaveBeenCalledTimes(1); - expect(history.push).toHaveBeenCalledWith(intendedPath); - expect(history.location.pathname).toBe(intendedPath); + + renderWithProvider(, store, { + pathname, + }); + + expect(mockUseNavigate).toHaveBeenCalledTimes(1); + expect(mockUseNavigate).toHaveBeenCalledWith(intendedPath); }); - it('changes password, submits, and redirects to the specified route', async () => { + it('changes password, submits, and redirects to the specified route (from location.state)', async () => { const intendedPath = '/intended-route'; const intendedSearch = '?abc=123'; const mockStateNonUnlocked = { metamask: { isUnlocked: false }, }; const store = configureMockStore([thunk])(mockStateNonUnlocked); - const history = createMemoryHistory({ - initialEntries: [ - { - pathname: '/unlock', - state: { from: { pathname: intendedPath, search: intendedSearch } }, - }, - ], + + // Set up the router to have the location state that would come from a redirect + const pathname = '/unlock'; + const locationState = { + from: { pathname: intendedPath, search: intendedSearch }, + }; + + const { queryByTestId } = renderWithProvider(, store, { + pathname, + state: locationState, }); - jest.spyOn(history, 'push'); - const { queryByTestId } = renderWithProvider( - - - , - store, - ); + const passwordField = queryByTestId('unlock-password'); const loginButton = queryByTestId('unlock-submit'); fireEvent.change(passwordField, { target: { value: 'a-password' } }); @@ -192,10 +221,38 @@ describe('Unlock Page', () => { await Promise.resolve(); // Wait for async operations expect(mockTryUnlockMetamask).toHaveBeenCalledTimes(1); - expect(history.push).toHaveBeenCalledTimes(1); - expect(history.push).toHaveBeenCalledWith(intendedPath + intendedSearch); - expect(history.location.pathname).toBe(intendedPath); - expect(history.location.search).toBe(intendedSearch); + expect(mockUseNavigate).toHaveBeenCalledTimes(1); + expect(mockUseNavigate).toHaveBeenCalledWith(intendedPath + intendedSearch); + }); + + it('changes password, submits, and redirects to the specified route (from navState)', async () => { + const intendedPath = '/intended-route'; + const intendedSearch = '?abc=123'; + const mockStateNonUnlocked = { + metamask: { isUnlocked: false }, + }; + const store = configureMockStore([thunk])(mockStateNonUnlocked); + + const pathname = '/unlock'; + + // Mock navState to provide the redirect location (v5-compat way) + mockUseNavState.mockReturnValue({ + from: { pathname: intendedPath, search: intendedSearch }, + }); + + const { queryByTestId } = renderWithProvider(, store, { + pathname, + }); + + const passwordField = queryByTestId('unlock-password'); + const loginButton = queryByTestId('unlock-submit'); + fireEvent.change(passwordField, { target: { value: 'a-password' } }); + fireEvent.click(loginButton); + await Promise.resolve(); // Wait for async operations + + expect(mockTryUnlockMetamask).toHaveBeenCalledTimes(1); + expect(mockUseNavigate).toHaveBeenCalledTimes(1); + expect(mockUseNavigate).toHaveBeenCalledWith(intendedPath + intendedSearch); }); it('should show login error modal when authentication error is thrown', async () => { @@ -205,15 +262,46 @@ describe('Unlock Page', () => { metamask: { isUnlocked: false, completedOnboarding: true }, }; const store = configureMockStore([thunk])(mockStateNonUnlocked); - const history = createMemoryHistory({ - initialEntries: [ - { - pathname: '/unlock', - state: { from: { pathname: intendedPath, search: intendedSearch } }, - }, - ], + const pathname = '/unlock'; + // Mock navState to provide the redirect location (v5-compat way) + mockUseNavState.mockReturnValue({ + from: { pathname: intendedPath, search: intendedSearch }, + }); + + mockTryUnlockMetamask.mockImplementationOnce( + jest.fn(() => { + return Promise.reject( + new Error( + SeedlessOnboardingControllerErrorMessage.AuthenticationError, + ), + ); + }), + ); + const mockForceUpdateMetamaskState = jest.fn(); + + const { queryByTestId } = renderWithProvider( + , + store, + { + pathname, + }, + ); + const passwordField = queryByTestId('unlock-password'); + const loginButton = queryByTestId('unlock-submit'); + fireEvent.change(passwordField, { target: { value: 'a-password' } }); + fireEvent.click(loginButton); + + await waitFor(() => { + expect(queryByTestId('login-error-modal')).toBeInTheDocument(); }); - jest.spyOn(history, 'push'); + }); + + it('should show login error modal when authentication error is thrown', async () => { + const pathname = '/unlock'; + const mockStateNonUnlocked = { + metamask: { isUnlocked: false, completedOnboarding: true }, + }; + const store = configureMockStore([thunk])(mockStateNonUnlocked); mockTryUnlockMetamask.mockImplementationOnce( jest.fn(() => { @@ -227,11 +315,13 @@ describe('Unlock Page', () => { const mockForceUpdateMetamaskState = jest.fn(); const { queryByTestId } = renderWithProvider( - - - , + , store, + { + pathname, + }, ); + const passwordField = queryByTestId('unlock-password'); const loginButton = queryByTestId('unlock-submit'); fireEvent.change(passwordField, { target: { value: 'a-password' } }); From 33daab70bd87ab3443e32bfd80312731108ba162 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Fri, 7 Nov 2025 16:00:44 +0000 Subject: [PATCH 02/13] refactor(556x): fix lint, e2e and Metametrics context getPath --- ui/contexts/metametrics.js | 24 ++++++++----- .../initialized/initialized-v5-compat.tsx | 1 - ui/pages/deep-link/deep-link.tsx | 4 +-- ui/pages/unlock-page/unlock-page.container.js | 6 ++-- ui/pages/unlock-page/unlock-page.test.js | 36 ------------------- 5 files changed, 21 insertions(+), 50 deletions(-) diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index fd6567083f63..355826bf5f3f 100644 --- a/ui/contexts/metametrics.js +++ b/ui/contexts/metametrics.js @@ -157,14 +157,22 @@ export function MetaMetricsProvider({ children }) { */ useEffect(() => { const environmentType = getEnvironmentType(); - const match = matchPath( - { - path: getPaths(), - end: true, - caseSensitive: true, - }, - location.pathname, - ); + // v6 matchPath doesn't support array of paths, so we loop to find first match + const paths = getPaths(); + let match = null; + for (const path of paths) { + match = matchPath( + { + path, + end: true, + caseSensitive: true, + }, + location.pathname, + ); + if (match) { + break; + } + } // Start by checking for a missing match route. If this falls through to // the else if, then we know we have a matched route for tracking. if (!match) { diff --git a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx index 8521f946203b..142160f98fe8 100644 --- a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx @@ -34,4 +34,3 @@ const InitializedV5Compat = ({ children }: InitializedV5CompatProps) => { }; export default InitializedV5Compat; - diff --git a/ui/pages/deep-link/deep-link.tsx b/ui/pages/deep-link/deep-link.tsx index 65975c53ed9a..122ebffeb179 100644 --- a/ui/pages/deep-link/deep-link.tsx +++ b/ui/pages/deep-link/deep-link.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useRef, useState } from 'react'; -import type { Location } from 'react-router-dom'; +import type { Location as RouterLocation } from 'react-router-dom-v5-compat'; import log from 'loglevel'; import { useDispatch, useSelector } from 'react-redux'; import { @@ -155,7 +155,7 @@ async function updateStateFromUrl( } type DeepLinkProps = { - location: Location; + location: RouterLocation; }; export const DeepLink = ({ location }: DeepLinkProps) => { diff --git a/ui/pages/unlock-page/unlock-page.container.js b/ui/pages/unlock-page/unlock-page.container.js index 18aa9cdd8f73..0113a7e814cb 100644 --- a/ui/pages/unlock-page/unlock-page.container.js +++ b/ui/pages/unlock-page/unlock-page.container.js @@ -1,4 +1,5 @@ import { connect } from 'react-redux'; +import React from 'react'; import { compose } from 'redux'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths @@ -22,10 +23,9 @@ import { getTheme, } from '../../selectors'; import { getCompletedOnboarding } from '../../ducks/metamask/metamask'; -import UnlockPage from './unlock-page.component'; -import withRouterHooks from "../../helpers/higher-order-components/with-router-hooks/with-router-hooks"; -import React from 'react'; +import withRouterHooks from '../../helpers/higher-order-components/with-router-hooks/with-router-hooks'; import { useNavState } from '../../contexts/navigation-state'; +import UnlockPage from './unlock-page.component'; const mapStateToProps = (state) => { const { diff --git a/ui/pages/unlock-page/unlock-page.test.js b/ui/pages/unlock-page/unlock-page.test.js index d0522992213a..bb247dd9e308 100644 --- a/ui/pages/unlock-page/unlock-page.test.js +++ b/ui/pages/unlock-page/unlock-page.test.js @@ -295,40 +295,4 @@ describe('Unlock Page', () => { expect(queryByTestId('login-error-modal')).toBeInTheDocument(); }); }); - - it('should show login error modal when authentication error is thrown', async () => { - const pathname = '/unlock'; - const mockStateNonUnlocked = { - metamask: { isUnlocked: false, completedOnboarding: true }, - }; - const store = configureMockStore([thunk])(mockStateNonUnlocked); - - mockTryUnlockMetamask.mockImplementationOnce( - jest.fn(() => { - return Promise.reject( - new Error( - SeedlessOnboardingControllerErrorMessage.AuthenticationError, - ), - ); - }), - ); - const mockForceUpdateMetamaskState = jest.fn(); - - const { queryByTestId } = renderWithProvider( - , - store, - { - pathname, - }, - ); - - const passwordField = queryByTestId('unlock-password'); - const loginButton = queryByTestId('unlock-submit'); - fireEvent.change(passwordField, { target: { value: 'a-password' } }); - fireEvent.click(loginButton); - - await waitFor(() => { - expect(queryByTestId('login-error-modal')).toBeInTheDocument(); - }); - }); }); From 0f8e39b9b0247203b4561aaf93a1da7d12dfe582 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Fri, 7 Nov 2025 16:25:38 +0000 Subject: [PATCH 03/13] refactor(556x): Fix nested Route issues and Storybook compatibility for UnlockPage --- ui/pages/routes/routes.component.tsx | 45 ++++++++++--------- ui/pages/unlock-page/README.mdx | 4 +- ui/pages/unlock-page/unlock-page.container.js | 3 ++ ui/pages/unlock-page/unlock-page.stories.js | 9 ++-- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/ui/pages/routes/routes.component.tsx b/ui/pages/routes/routes.component.tsx index b1019097ebff..4c339c525841 100644 --- a/ui/pages/routes/routes.component.tsx +++ b/ui/pages/routes/routes.component.tsx @@ -618,7 +618,13 @@ export default function Routes() { {/** @ts-expect-error TODO: Replace `component` prop with `element` once `react-router` is upgraded to v6 */} - + {(props: RouteComponentProps) => { const { history: v5History, location: v5Location } = props; const navigate = createV5CompatNavigate(v5History); @@ -666,15 +672,12 @@ export default function Routes() { keyringId?: string; }>; return ( - ( - - )} - /> + + + ); }} @@ -870,15 +873,12 @@ export default function Routes() { params: { chainId: string; protocolId: string }; }>; return ( - ( - - )} - /> + + + ); }} @@ -1121,7 +1121,12 @@ export default function Routes() { {renderRoutes()} {isUnlocked ? : null} - + {React.createElement( + ToastMaster as React.ComponentType<{ + location: RouteComponentProps['location']; + }>, + { location }, + )} ); } diff --git a/ui/pages/unlock-page/README.mdx b/ui/pages/unlock-page/README.mdx index 3d82cce15ce6..14aa0e69240f 100644 --- a/ui/pages/unlock-page/README.mdx +++ b/ui/pages/unlock-page/README.mdx @@ -1,6 +1,6 @@ import { Story, Canvas, ArgsTable } from '@storybook/addon-docs'; -import UnlockPage from '.'; +import UnlockPage from './unlock-page.component'; # Unlock Page @@ -14,7 +14,7 @@ Portal page for user to auth the access of their account | Name | Description | | -------------------------- | -------------------------------------------------------------------------- | -| `history` | History router for redirect after action `object` | +| `navigate` | Navigate function for redirect after action `object` | | `isUnlocked` | If isUnlocked is true will redirect to most recent route in history `bool` | | `onRestore` | onClick handler for "Forgot password?" link `func` | | `onSubmit` | onSumbit handler when form is submitted `func` | diff --git a/ui/pages/unlock-page/unlock-page.container.js b/ui/pages/unlock-page/unlock-page.container.js index 0113a7e814cb..1236eddbcc59 100644 --- a/ui/pages/unlock-page/unlock-page.container.js +++ b/ui/pages/unlock-page/unlock-page.container.js @@ -112,4 +112,7 @@ const UnlockPageWithNavState = (props) => { return ; }; +// Export the connected component for Storybook/testing +export { UnlockPageConnected }; + export default UnlockPageWithNavState; diff --git a/ui/pages/unlock-page/unlock-page.stories.js b/ui/pages/unlock-page/unlock-page.stories.js index cd6337f3d522..832cf26f3ce4 100644 --- a/ui/pages/unlock-page/unlock-page.stories.js +++ b/ui/pages/unlock-page/unlock-page.stories.js @@ -1,4 +1,3 @@ -import { createBrowserHistory } from 'history'; import React from 'react'; import README from './README.mdx'; import UnlockPage from './unlock-page.component'; @@ -13,7 +12,8 @@ export default { }, }, argTypes: { - history: { control: 'object' }, + navigate: { control: 'object' }, + location: { control: 'object' }, isUnlocked: { control: 'boolean' }, onRestore: { action: 'onRestore' }, onSubmit: { action: 'onSubmit' }, @@ -22,8 +22,9 @@ export default { }; export const DefaultStory = (args) => { - const history = createBrowserHistory(); - return ; + const navigate = (path) => console.log('Navigate to:', path); + const location = { pathname: '/unlock', search: '', state: null }; + return ; }; DefaultStory.storyName = 'Default'; From d7c8f494ad0238dd333a5eddd6e43549b271d1cc Mon Sep 17 00:00:00 2001 From: dddddanica Date: Sun, 9 Nov 2025 23:12:43 +0000 Subject: [PATCH 04/13] refactor(556x): fix lint and add mixed route in metametrics.js --- ui/contexts/metametrics.js | 30 +++++++++++++++---- .../defi/components/defi-details-page.tsx | 6 ++-- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index 355826bf5f3f..5711458c2aca 100644 --- a/ui/contexts/metametrics.js +++ b/ui/contexts/metametrics.js @@ -12,7 +12,12 @@ import React, { useContext, } from 'react'; import PropTypes from 'prop-types'; -import { matchPath, useLocation } from 'react-router-dom-v5-compat'; +// NOTE: Mixed v5/v5-compat imports during router migration +// - useLocation from v5: Works with the v5 HashRouter to detect navigation changes +// - matchPath from v5-compat: Provides v6 API (reversed args, pattern.path structure) +// When v6 migration is complete, change both imports to: import { useLocation, matchPath } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; +import { matchPath } from 'react-router-dom-v5-compat'; import { useSelector } from 'react-redux'; import { omit } from 'lodash'; @@ -148,6 +153,7 @@ export function MetaMetricsProvider({ children }) { // Used to prevent double tracking page calls const previousMatch = useRef(); + const previousPathname = useRef(); /** * Anytime the location changes, track a page change with segment. @@ -156,11 +162,21 @@ export function MetaMetricsProvider({ children }) { * which page the user is on and their navigation path. */ useEffect(() => { + // Only run if pathname actually changed + if (previousPathname.current === location.pathname) { + return; + } + previousPathname.current = location.pathname; + const environmentType = getEnvironmentType(); // v6 matchPath doesn't support array of paths, so we loop to find first match const paths = getPaths(); let match = null; for (const path of paths) { + // Skip empty string paths - they don't work correctly with v6 matchPath + if (path === '') { + continue; + } match = matchPath( { path, @@ -183,10 +199,10 @@ export function MetaMetricsProvider({ children }) { }, }); } else if ( - previousMatch.current !== match.path && + previousMatch.current !== match.pattern.path && !( environmentType === 'notification' && - match.path === '/' && + match.pattern.path === '/' && previousMatch.current === undefined ) ) { @@ -195,7 +211,8 @@ export function MetaMetricsProvider({ children }) { // this we keep track of the previousMatch, and we skip the event track // in the event that we are dealing with the initial load of the // homepage - const { path, params } = match; + const { pattern, params } = match; + const { path } = pattern; const name = PATH_NAME_MAP.get(path); trackMetaMetricsPage( { @@ -212,8 +229,9 @@ export function MetaMetricsProvider({ children }) { }, ); } - previousMatch.current = match?.path; - }, [location, context]); + previousMatch.current = match?.pattern?.path; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [location.pathname, location.search, location.hash]); // For backwards compatibility, attach the new methods as properties to trackEvent const trackEventWithMethods = trackEvent; diff --git a/ui/pages/defi/components/defi-details-page.tsx b/ui/pages/defi/components/defi-details-page.tsx index a2c1300b0f5a..bd6597203ad4 100644 --- a/ui/pages/defi/components/defi-details-page.tsx +++ b/ui/pages/defi/components/defi-details-page.tsx @@ -61,7 +61,9 @@ const DeFiPage = ({ navigate, params }: DeFiPageProps) => { // TODO: Get value in user's preferred currency const protocolPosition = - defiPositions[selectedAccount.address]?.[chainId]?.protocols[protocolId]; + defiPositions[selectedAccount.address]?.[ + chainId as keyof (typeof defiPositions)[string] + ]?.protocols[protocolId]; const extractedTokens = useMemo(() => { return Object.keys(protocolPosition?.positionTypes || {}).reduce( @@ -120,7 +122,7 @@ const DeFiPage = ({ navigate, params }: DeFiPageProps) => { {protocolPosition.protocolDetails.name} Date: Mon, 10 Nov 2025 16:21:53 +0000 Subject: [PATCH 05/13] refactor: revert back dependency in `ui/contexts/metametrics.js` --- ui/contexts/metametrics.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index 5711458c2aca..6dc4ebb32944 100644 --- a/ui/contexts/metametrics.js +++ b/ui/contexts/metametrics.js @@ -230,8 +230,13 @@ export function MetaMetricsProvider({ children }) { ); } previousMatch.current = match?.pattern?.path; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [location.pathname, location.search, location.hash]); + }, [ + location.pathname, + location.search, + location.hash, + context.page, + context.referrer, + ]); // For backwards compatibility, attach the new methods as properties to trackEvent const trackEventWithMethods = trackEvent; From 5d794d15a4f56e9b439b9bf560928ad5584b9fb0 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 16:29:10 +0000 Subject: [PATCH 06/13] refactor: added unit test for ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx --- .../initialized-v5-compat.test.tsx | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx diff --git a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx new file mode 100644 index 000000000000..5e684ea799f4 --- /dev/null +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx @@ -0,0 +1,131 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { MemoryRouter, Routes, Route } from 'react-router-dom-v5-compat'; +import configureStore from 'redux-mock-store'; +import { ONBOARDING_ROUTE } from '../../constants/routes'; +import InitializedV5Compat from './initialized-v5-compat'; + +const mockStore = configureStore(); + +describe('InitializedV5Compat', () => { + const MockChildComponent = () => ( +
Child Component
+ ); + const MockOnboardingComponent = () => ( +
Onboarding Page
+ ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should render children when onboarding is completed', () => { + const store = mockStore({ + metamask: { + completedOnboarding: true, + }, + }); + + render( + + + + + + + } + /> + } + /> + + + , + ); + + expect(screen.getByTestId('child-component')).toBeInTheDocument(); + expect(screen.getByText('Child Component')).toBeInTheDocument(); + expect(screen.queryByTestId('onboarding')).not.toBeInTheDocument(); + }); + + it('should redirect to onboarding route when onboarding is not completed', () => { + const store = mockStore({ + metamask: { + completedOnboarding: false, + }, + }); + + render( + + + + + + + } + /> + } + /> + + + , + ); + + expect(screen.queryByTestId('child-component')).not.toBeInTheDocument(); + expect(screen.getByTestId('onboarding')).toBeInTheDocument(); + expect(screen.getByText('Onboarding Page')).toBeInTheDocument(); + }); + + it('should accept and render complex React node structures as children', () => { + const store = mockStore({ + metamask: { + completedOnboarding: true, + }, + }); + + const NestedComponent = () => ( + Nested Content + ); + + render( + + + + +
+ +

Some text

+
+ + } + /> + } + /> + + + , + ); + + expect(screen.getByTestId('parent')).toBeInTheDocument(); + expect(screen.getByTestId('nested')).toBeInTheDocument(); + expect(screen.getByTestId('paragraph')).toBeInTheDocument(); + expect(screen.getByText('Nested Content')).toBeInTheDocument(); + expect(screen.getByText('Some text')).toBeInTheDocument(); + }); +}); + From 9c0a3ce8b335be43247865544fa66d459924d3bd Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 16:32:14 +0000 Subject: [PATCH 07/13] refactor: extract getCompletedOnboarding to selector pattern --- .../initialized/initialized-v5-compat.test.tsx | 4 ++++ .../initialized/initialized-v5-compat.tsx | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx index 5e684ea799f4..7960dec93230 100644 --- a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx @@ -6,6 +6,10 @@ import configureStore from 'redux-mock-store'; import { ONBOARDING_ROUTE } from '../../constants/routes'; import InitializedV5Compat from './initialized-v5-compat'; +jest.mock('../../../ducks/metamask/metamask', () => ({ + getCompletedOnboarding: (state: any) => state.metamask.completedOnboarding, +})); + const mockStore = configureStore(); describe('InitializedV5Compat', () => { diff --git a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx index 142160f98fe8..4ee9460834ab 100644 --- a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { useSelector } from 'react-redux'; import { Navigate } from 'react-router-dom-v5-compat'; +import { getCompletedOnboarding } from '../../../ducks/metamask/metamask'; import { ONBOARDING_ROUTE } from '../../constants/routes'; type InitializedV5CompatProps = { @@ -21,10 +22,7 @@ type InitializedV5CompatProps = { * @returns Navigate component or children */ const InitializedV5Compat = ({ children }: InitializedV5CompatProps) => { - const completedOnboarding = useSelector( - (state: { metamask: { completedOnboarding: boolean } }) => - state.metamask.completedOnboarding, - ); + const completedOnboarding = useSelector(getCompletedOnboarding); if (!completedOnboarding) { return ; From 8efbe21ef82331415e84e7c1399823f2fe2c6782 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 16:47:13 +0000 Subject: [PATCH 08/13] refactor: add createV5CompatRoute wrapper for wrapping the components temporarily --- ui/pages/routes/routes.component.tsx | 340 +++++++++++++-------------- 1 file changed, 157 insertions(+), 183 deletions(-) diff --git a/ui/pages/routes/routes.component.tsx b/ui/pages/routes/routes.component.tsx index 4c339c525841..5a920b89e49a 100644 --- a/ui/pages/routes/routes.component.tsx +++ b/ui/pages/routes/routes.component.tsx @@ -208,6 +208,87 @@ const createV5CompatNavigate = ( }; }; +/** + * Props that can be passed to v5-compat components + */ +type V5CompatComponentProps< + TParams extends Record, +> = { + navigate?: V5CompatNavigate; + location?: RouteComponentProps['location']; + match?: RouteComponentProps['match']; + params?: TParams; +} & Partial; + +/** + * Helper to create v5-compat route wrappers with less boilerplate. + * Handles authentication, navigation, and prop passing for v5-to-v5-compat transition. + * + * NOTE: This is temporary scaffolding for the v5-compat transition. + * It will be removed during the full v6 migration when routes use native v6 patterns. + * + * @param Component - The component to render + * @param options - Configuration options + * @param options.wrapper - Wrapper component (AuthenticatedV5Compat, InitializedV5Compat, or null for none) + * @param options.includeNavigate - Whether to pass navigate prop + * @param options.includeLocation - Whether to pass location prop + * @param options.includeParams - Whether to pass params from route match + * @param options.includeMatch - Whether to pass the entire match object + * @param options.paramsAsProps - Whether to spread params as individual props (default: true) + * @returns Route render function + */ +const createV5CompatRoute = < + TParams extends Record, +>( + Component: React.ComponentType>, + options: { + wrapper?: React.ComponentType<{ children: React.ReactNode }> | null; + includeNavigate?: boolean; + includeLocation?: boolean; + includeParams?: boolean; + includeMatch?: boolean; + paramsAsProps?: boolean; + } = {}, +) => { + const { + wrapper = null, + includeNavigate = false, + includeLocation = false, + includeParams = false, + includeMatch = false, + paramsAsProps = true, + } = options; + + return (props: RouteComponentProps) => { + const { history: v5History, location: v5Location, match } = props; + + const componentProps: Partial> = {}; + + if (includeNavigate) { + componentProps.navigate = createV5CompatNavigate(v5History); + } + if (includeLocation) { + componentProps.location = v5Location; + } + if (includeMatch) { + componentProps.match = match; + } + if (includeParams) { + if (paramsAsProps) { + Object.assign(componentProps, match.params); + } else { + componentProps.params = match.params; + } + } + + const element = ( + )} /> + ); + + return wrapper ? React.createElement(wrapper, {}, element) : element; + }; +}; + // TODO: Fix `as unknown as` casting once `mmLazy` is updated to handle named exports, wrapped components, and other React module types. // Casting is preferable over `@ts-expect-error` annotations in this case, // because it doesn't suppress competing error messages e.g. "Cannot find module..." @@ -625,32 +706,16 @@ export default function Routes() { // eslint-disable-next-line @typescript-eslint/no-explicit-any {...({ exact: true } as any)} > - {(props: RouteComponentProps) => { - const { history: v5History, location: v5Location } = props; - const navigate = createV5CompatNavigate(v5History); - - const UnlockPageComponent = UnlockPage as React.ComponentType<{ - navigate: V5CompatNavigate; - location: RouteComponentProps['location']; - }>; - return ( - - - - ); - }} + {createV5CompatRoute(UnlockPage, { + wrapper: InitializedV5Compat, + includeNavigate: true, + includeLocation: true, + })} - {(props: RouteComponentProps) => { - const { location: v5Location } = props; - const DeepLinkComponent = DeepLink as React.ComponentType<{ - location: RouteComponentProps['location']; - }>; - return ; - }} + {createV5CompatRoute(DeepLink, { + includeLocation: true, + })} - {(props: RouteComponentProps<{ keyringId?: string }>) => { - const { history: v5History, match } = props; - const navigate = createV5CompatNavigate(v5History); - - const RevealSeedConfirmationComponent = - RevealSeedConfirmation as React.ComponentType<{ - navigate: V5CompatNavigate; - keyringId?: string; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ keyringId?: string }>( + RevealSeedConfirmation, + { + wrapper: AuthenticatedV5Compat, + includeNavigate: true, + includeParams: true, + }, + )} @@ -700,17 +755,10 @@ export default function Routes() { /> - {(props: RouteComponentProps) => { - const { location: v5Location } = props; - const SwapsComponent = Swaps as React.ComponentType<{ - location: RouteComponentProps['location']; - }>; - return ( - - - - ); - }} + {createV5CompatRoute(Swaps, { + wrapper: AuthenticatedV5Compat, + includeLocation: true, + })} - {(props: RouteComponentProps<{ srcTxMetaId: string }>) => { - const { history: v5History, location: v5Location, match } = props; - const navigate = createV5CompatNavigate(v5History); - const CrossChainSwapTxDetailsComponent = - CrossChainSwapTxDetails as React.ComponentType<{ - location: RouteComponentProps['location']; - navigate: V5CompatNavigate; - params: { srcTxMetaId: string }; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ srcTxMetaId: string }>( + CrossChainSwapTxDetails, + { + wrapper: AuthenticatedV5Compat, + includeNavigate: true, + includeLocation: true, + includeParams: true, + paramsAsProps: false, // Pass as params object + }, + )} - {(props: RouteComponentProps) => { - const { location: v5Location } = props; - const CrossChainSwapComponent = - CrossChainSwap as React.ComponentType<{ - location: RouteComponentProps['location']; - }>; - return ( - - - - ); - }} + {createV5CompatRoute(CrossChainSwap, { + wrapper: AuthenticatedV5Compat, + includeLocation: true, + })} - {(props: RouteComponentProps<{ id: string }>) => { - const { - history: v5History, - location: v5Location, - match: v5Match, - } = props; - - const navigate = createV5CompatNavigate(v5History); - - const PermissionsConnectWithProps = - PermissionsConnect as React.ComponentType<{ - location: RouteComponentProps['location']; - navigate: V5CompatNavigate; - match: RouteComponentProps<{ id: string }>['match']; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ id: string }>(PermissionsConnect, { + wrapper: AuthenticatedV5Compat, + includeNavigate: true, + includeLocation: true, + includeMatch: true, + })} - {(props: RouteComponentProps<{ asset: string; id: string }>) => { - const NftFullImageComponent = - NftFullImage as React.ComponentType<{ - params: { asset: string; id: string }; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ asset: string; id: string }>(NftFullImage, { + wrapper: AuthenticatedV5Compat, + includeParams: true, + paramsAsProps: false, + })} - {( - props: RouteComponentProps<{ - chainId: string; - asset: string; - id: string; - }>, - ) => { - const AssetComponent = Asset as React.ComponentType<{ - params: { chainId: string; asset: string; id: string }; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ + chainId: string; + asset: string; + id: string; + }>(Asset, { + wrapper: AuthenticatedV5Compat, + includeParams: true, + paramsAsProps: false, + })} - {( - props: RouteComponentProps<{ - chainId: string; - asset: string; - }>, - ) => { - const AssetComponent = Asset as React.ComponentType<{ - params: { chainId: string; asset: string }; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ + chainId: string; + asset: string; + }>(Asset, { + wrapper: AuthenticatedV5Compat, + includeParams: true, + paramsAsProps: false, + })} - {(props: RouteComponentProps<{ chainId: string }>) => { - const AssetComponent = Asset as React.ComponentType<{ - params: { chainId: string }; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ chainId: string }>(Asset, { + wrapper: AuthenticatedV5Compat, + includeParams: true, + paramsAsProps: false, + })} - {( - props: RouteComponentProps<{ - chainId: string; - protocolId: string; - }>, - ) => { - const { history: v5History, match } = props; - const navigate = createV5CompatNavigate(v5History); - - const DeFiPageComponent = DeFiPage as React.ComponentType<{ - navigate: V5CompatNavigate; - params: { chainId: string; protocolId: string }; - }>; - return ( - - - - ); - }} + {createV5CompatRoute<{ + chainId: string; + protocolId: string; + }>(DeFiPage, { + wrapper: AuthenticatedV5Compat, + includeNavigate: true, + includeParams: true, + paramsAsProps: false, + })} Date: Mon, 10 Nov 2025 16:49:35 +0000 Subject: [PATCH 09/13] refactor: add prop types for navigate and location for ui/pages/unlock-page/unlock-page.container.js --- ui/pages/unlock-page/unlock-page.container.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ui/pages/unlock-page/unlock-page.container.js b/ui/pages/unlock-page/unlock-page.container.js index 1236eddbcc59..67a81bff5b7f 100644 --- a/ui/pages/unlock-page/unlock-page.container.js +++ b/ui/pages/unlock-page/unlock-page.container.js @@ -1,5 +1,6 @@ import { connect } from 'react-redux'; import React from 'react'; +import PropTypes from 'prop-types'; import { compose } from 'redux'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths @@ -105,13 +106,24 @@ const UnlockPageConnected = compose( connect(mapStateToProps, mapDispatchToProps, mergeProps), )(UnlockPage); -// Inject navState from NavigationStateContext for v5-compat navigation -// eslint-disable-next-line react/prop-types +/** + * Inject navState from NavigationStateContext for v5-compat navigation. + * This wrapper ensures the unlock page can read navigation state from both + * v5 location.state and v5-compat NavigationStateContext. + * + * @param {object} props - Component props (navigate, location from route) + * @returns {React.ReactElement} UnlockPage with navState injected + */ const UnlockPageWithNavState = (props) => { const navState = useNavState(); return ; }; +UnlockPageWithNavState.propTypes = { + navigate: PropTypes.func.isRequired, + location: PropTypes.object.isRequired, +}; + // Export the connected component for Storybook/testing export { UnlockPageConnected }; From 2da7985bbdcbc271709156589c863db1a594ca30 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 16:50:39 +0000 Subject: [PATCH 10/13] refactor: fix lint issues --- .../initialized/initialized-v5-compat.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx index 7960dec93230..84f0f951f614 100644 --- a/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx @@ -7,7 +7,9 @@ import { ONBOARDING_ROUTE } from '../../constants/routes'; import InitializedV5Compat from './initialized-v5-compat'; jest.mock('../../../ducks/metamask/metamask', () => ({ - getCompletedOnboarding: (state: any) => state.metamask.completedOnboarding, + getCompletedOnboarding: (state: { + metamask: { completedOnboarding: boolean }; + }) => state.metamask.completedOnboarding, })); const mockStore = configureStore(); @@ -132,4 +134,3 @@ describe('InitializedV5Compat', () => { expect(screen.getByText('Some text')).toBeInTheDocument(); }); }); - From 2195a52bfa8e17f6c82bf03777c257a129bbe844 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 17:14:39 +0000 Subject: [PATCH 11/13] refactor: fix few bugs around ui/contexts/metametrics.js --- ui/contexts/metametrics.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index 6dc4ebb32944..b1754d464c37 100644 --- a/ui/contexts/metametrics.js +++ b/ui/contexts/metametrics.js @@ -153,7 +153,6 @@ export function MetaMetricsProvider({ children }) { // Used to prevent double tracking page calls const previousMatch = useRef(); - const previousPathname = useRef(); /** * Anytime the location changes, track a page change with segment. @@ -162,12 +161,6 @@ export function MetaMetricsProvider({ children }) { * which page the user is on and their navigation path. */ useEffect(() => { - // Only run if pathname actually changed - if (previousPathname.current === location.pathname) { - return; - } - previousPathname.current = location.pathname; - const environmentType = getEnvironmentType(); // v6 matchPath doesn't support array of paths, so we loop to find first match const paths = getPaths(); @@ -181,7 +174,7 @@ export function MetaMetricsProvider({ children }) { { path, end: true, - caseSensitive: true, + caseSensitive: false, // Match v5 behavior (case-insensitive by default) }, location.pathname, ); From 025004cfbe62e2c3830479dbd3c1a5493d3ab268 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 18:27:18 +0000 Subject: [PATCH 12/13] refactor: fix lint --- ui/pages/routes/routes.component.tsx | 30 ++++++++++------------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/ui/pages/routes/routes.component.tsx b/ui/pages/routes/routes.component.tsx index 5a920b89e49a..e68371ff9293 100644 --- a/ui/pages/routes/routes.component.tsx +++ b/ui/pages/routes/routes.component.tsx @@ -208,18 +208,6 @@ const createV5CompatNavigate = ( }; }; -/** - * Props that can be passed to v5-compat components - */ -type V5CompatComponentProps< - TParams extends Record, -> = { - navigate?: V5CompatNavigate; - location?: RouteComponentProps['location']; - match?: RouteComponentProps['match']; - params?: TParams; -} & Partial; - /** * Helper to create v5-compat route wrappers with less boilerplate. * Handles authentication, navigation, and prop passing for v5-to-v5-compat transition. @@ -238,9 +226,13 @@ type V5CompatComponentProps< * @returns Route render function */ const createV5CompatRoute = < - TParams extends Record, + TParams extends Record = Record< + string, + string | undefined + >, >( - Component: React.ComponentType>, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Component: React.ComponentType, options: { wrapper?: React.ComponentType<{ children: React.ReactNode }> | null; includeNavigate?: boolean; @@ -262,7 +254,7 @@ const createV5CompatRoute = < return (props: RouteComponentProps) => { const { history: v5History, location: v5Location, match } = props; - const componentProps: Partial> = {}; + const componentProps: Record = {}; if (includeNavigate) { componentProps.navigate = createV5CompatNavigate(v5History); @@ -281,11 +273,11 @@ const createV5CompatRoute = < } } - const element = ( - )} /> - ); + const element = ; - return wrapper ? React.createElement(wrapper, {}, element) : element; + return wrapper + ? React.createElement(wrapper, { children: element }) + : element; }; }; From 3fcd4e9d653ee34e91fd12d3b351603298f62495 Mon Sep 17 00:00:00 2001 From: dddddanica Date: Mon, 10 Nov 2025 19:01:47 +0000 Subject: [PATCH 13/13] refactor: fix analytics Tracking Fails for Home Route --- ui/contexts/metametrics.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index b1754d464c37..da4f3173eb9f 100644 --- a/ui/contexts/metametrics.js +++ b/ui/contexts/metametrics.js @@ -25,7 +25,11 @@ import { captureException, captureMessage } from '../../shared/lib/sentry'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths import { getEnvironmentType } from '../../app/scripts/lib/util'; -import { PATH_NAME_MAP, getPaths } from '../helpers/constants/routes'; +import { + PATH_NAME_MAP, + getPaths, + DEFAULT_ROUTE, +} from '../helpers/constants/routes'; import { MetaMetricsContextProp } from '../../shared/constants/metametrics'; import { useSegmentContext } from '../hooks/useSegmentContext'; import { getParticipateInMetaMetrics } from '../selectors'; @@ -166,13 +170,11 @@ export function MetaMetricsProvider({ children }) { const paths = getPaths(); let match = null; for (const path of paths) { - // Skip empty string paths - they don't work correctly with v6 matchPath - if (path === '') { - continue; - } + // Normalize empty string paths to '/' - they're aliases for the Home route + const normalizedPath = path === '' ? DEFAULT_ROUTE : path; match = matchPath( { - path, + path: normalizedPath, end: true, caseSensitive: false, // Match v5 behavior (case-insensitive by default) },