diff --git a/ui/contexts/metametrics.js b/ui/contexts/metametrics.js index d37cdccec602..da4f3173eb9f 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'; +// 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'; @@ -20,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'; @@ -157,11 +166,24 @@ export function MetaMetricsProvider({ children }) { */ useEffect(() => { const environmentType = getEnvironmentType(); - const match = matchPath(location.pathname, { - path: getPaths(), - exact: true, - strict: true, - }); + // 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) { + // Normalize empty string paths to '/' - they're aliases for the Home route + const normalizedPath = path === '' ? DEFAULT_ROUTE : path; + match = matchPath( + { + path: normalizedPath, + end: true, + caseSensitive: false, // Match v5 behavior (case-insensitive by default) + }, + 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) { @@ -172,10 +194,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 ) ) { @@ -184,7 +206,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( { @@ -201,8 +224,14 @@ export function MetaMetricsProvider({ children }) { }, ); } - previousMatch.current = match?.path; - }, [location, context]); + previousMatch.current = match?.pattern?.path; + }, [ + location.pathname, + location.search, + location.hash, + context.page, + context.referrer, + ]); // For backwards compatibility, attach the new methods as properties to trackEvent const trackEventWithMethods = trackEvent; 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..84f0f951f614 --- /dev/null +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.test.tsx @@ -0,0 +1,136 @@ +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'; + +jest.mock('../../../ducks/metamask/metamask', () => ({ + getCompletedOnboarding: (state: { + metamask: { completedOnboarding: boolean }; + }) => state.metamask.completedOnboarding, +})); + +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(); + }); +}); 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..4ee9460834ab --- /dev/null +++ b/ui/helpers/higher-order-components/initialized/initialized-v5-compat.tsx @@ -0,0 +1,34 @@ +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 = { + 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(getCompletedOnboarding); + + 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..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 { useLocation } 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 { @@ -154,8 +154,11 @@ async function updateStateFromUrl( } } -export const DeepLink = () => { - const location = useLocation(); +type DeepLinkProps = { + location: RouterLocation; +}; + +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..bd6597203ad4 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,22 +45,25 @@ 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); // 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( @@ -82,7 +85,7 @@ const DeFiPage = () => { }; if (!protocolPosition) { - return ; + return ; } return ( @@ -100,7 +103,7 @@ const DeFiPage = () => { size={ButtonIconSize.Sm} ariaLabel={t('back')} iconName={IconName.ArrowLeft} - onClick={() => history.push(DEFAULT_ROUTE)} + onClick={() => navigate(DEFAULT_ROUTE)} /> @@ -119,7 +122,7 @@ const DeFiPage = () => { {protocolPosition.protocolDetails.name} {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..e68371ff9293 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'; @@ -207,6 +208,79 @@ const createV5CompatNavigate = ( }; }; +/** + * 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 = Record< + string, + string | undefined + >, +>( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + 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: Record = {}; + + 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, { children: 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..." @@ -617,9 +691,24 @@ 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 */} - + + {createV5CompatRoute(UnlockPage, { + wrapper: InitializedV5Compat, + includeNavigate: true, + includeLocation: true, + })} + + + {createV5CompatRoute(DeepLink, { + includeLocation: true, + })} + - + + {createV5CompatRoute<{ keyringId?: string }>( + RevealSeedConfirmation, + { + wrapper: AuthenticatedV5Compat, + includeNavigate: true, + includeParams: true, + }, + )} + - {(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, + })} + + + {createV5CompatRoute<{ + chainId: string; + protocolId: string; + }>(DeFiPage, { + wrapper: AuthenticatedV5Compat, + includeNavigate: true, + includeParams: true, + paramsAsProps: false, + })} - {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.component.js b/ui/pages/unlock-page/unlock-page.component.js index 8694293ac9f1..2d5f021ae6aa 100644 --- a/ui/pages/unlock-page/unlock-page.component.js +++ b/ui/pages/unlock-page/unlock-page.component.js @@ -64,13 +64,17 @@ class UnlockPage extends Component { static propTypes = { /** - * History router for redirect after action + * navigate function for redirect after action */ - history: PropTypes.object.isRequired, + navigate: PropTypes.func.isRequired, /** * Location router for redirect after action */ location: PropTypes.object.isRequired, + /** + * Navigation state from v5-compat navigation context + */ + navState: PropTypes.object, /** * If isUnlocked is true will redirect to most recent route in history */ @@ -149,16 +153,18 @@ class UnlockPage extends Component { } UNSAFE_componentWillMount() { - const { isUnlocked, history, location } = this.props; + const { isUnlocked, navigate, location, navState } = this.props; if (isUnlocked) { // 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); } } @@ -415,7 +421,7 @@ class UnlockPage extends Component { }; onForgotPasswordOrLoginWithDiffMethods = async () => { - 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..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 { withRouter } from 'react-router-dom'; +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 @@ -23,6 +24,8 @@ import { getTheme, } from '../../selectors'; import { getCompletedOnboarding } from '../../ducks/metamask/metamask'; +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) => { @@ -57,15 +60,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 +80,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 +95,36 @@ 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. + * 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 }; + +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'; diff --git a/ui/pages/unlock-page/unlock-page.test.js b/ui/pages/unlock-page/unlock-page.test.js index 92f3e0ac2bf0..bb247dd9e308 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,102 @@ 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 } }; + + renderWithProvider(, store, { + pathname, + state: locationState, + }); - it('should redirect to history location when unlocked', () => { + 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' } }); + fireEvent.click(loginButton); + await Promise.resolve(); // Wait for async operations + + expect(mockTryUnlockMetamask).toHaveBeenCalledTimes(1); + 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' } }); @@ -192,10 +251,8 @@ 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('should show login error modal when authentication error is thrown', async () => { @@ -205,15 +262,11 @@ 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 }, }); - jest.spyOn(history, 'push'); mockTryUnlockMetamask.mockImplementationOnce( jest.fn(() => { @@ -227,10 +280,11 @@ describe('Unlock Page', () => { const mockForceUpdateMetamaskState = jest.fn(); const { queryByTestId } = renderWithProvider( - - - , + , store, + { + pathname, + }, ); const passwordField = queryByTestId('unlock-password'); const loginButton = queryByTestId('unlock-submit');