refactor: resolve race condition in Home navigation with v5-compat

Author: DDDDDanica

ui/components/app/srp-quiz-modal/SRPQuiz/SRPQuiz.tsx

@@ -1,7 +1,7 @@
 /* eslint-disable @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports, import/no-commonjs */
 import React, { useCallback, useContext, useEffect, useState } from 'react';
 import { useSelector } from 'react-redux';
-import type { NavigateFunction } from 'react-router-dom-v5-compat';
+import type { To } from 'react-router-dom-v5-compat';
 import {
   MetaMetricsEventCategory,
   MetaMetricsEventKeyType,
@@ -64,7 +64,13 @@ export type SRPQuizProps = {
   isOpen: boolean;
   onClose: () => void;
   closeAfterCompleting?: boolean;
-  navigate: NavigateFunction;
+  navigate: {
+    (
+      to: To,
+      options?: { replace?: boolean; state?: Record<string, unknown> },
+    ): void;
+    (delta: number): void;
+  };
 };
 
 // TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860

ui/components/multichain/account-details/account-details.tsx

@@ -3,6 +3,7 @@ import React, { useCallback, useContext, useState } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { KeyringObject, KeyringTypes } from '@metamask/keyring-controller';
 import { AvatarAccountSize } from '@metamask/design-system-react';
+import type { To } from 'react-router-dom-v5-compat';
 import {
   MetaMetricsEventCategory,
   MetaMetricsEventKeyType,
@@ -51,10 +52,13 @@ import { AccountDetailsKey } from './account-details-key';
 
 type AccountDetailsProps = {
   address: string;
-  navigate?: (
-    to: string | number,
-    options?: { replace?: boolean; state?: Record<string, unknown> },
-  ) => void;
+  navigate?: {
+    (
+      to: To,
+      options?: { replace?: boolean; state?: Record<string, unknown> },
+    ): void;
+    (delta: number): void;
+  };
 };
 
 export const AccountDetails = ({ address, navigate }: AccountDetailsProps) => {

ui/pages/confirmations/confirm-transaction/confirm-transaction.component.js

@@ -161,30 +161,33 @@ const ConfirmTransaction = ({
   }, []);
 
   useEffect(() => {
-    if (
-      paramsTransactionId &&
-      transactionId &&
-      prevParamsTransactionId !== paramsTransactionId
-    ) {
-      const { txData: { txParams: { data } = {}, origin } = {} } = transaction;
-
-      dispatch(clearConfirmTransaction());
-      dispatch(setTransactionToConfirm(paramsTransactionId));
-      if (origin !== ORIGIN_METAMASK) {
-        dispatch(getContractMethodData(data, use4ByteResolution));
-      }
-    } else if (prevTransactionId && !transactionId && !totalUnapproved) {
-      dispatch(setDefaultHomeActiveTabName('activity')).then(() => {
+    const handleNavigation = async () => {
+      if (
+        paramsTransactionId &&
+        transactionId &&
+        prevParamsTransactionId !== paramsTransactionId
+      ) {
+        const { txData: { txParams: { data } = {}, origin } = {} } = transaction;
+
+        dispatch(clearConfirmTransaction());
+        dispatch(setTransactionToConfirm(paramsTransactionId));
+        if (origin !== ORIGIN_METAMASK) {
+          dispatch(getContractMethodData(data, use4ByteResolution));
+        }
+      } else if (prevTransactionId && !transactionId && !totalUnapproved) {
+        await dispatch(setDefaultHomeActiveTabName('activity'));
         navigate(DEFAULT_ROUTE, { replace: true });
-      });
-    } else if (
-      prevTransactionId &&
-      transactionId &&
-      prevTransactionId !== transactionId &&
-      paramsTransactionId !== transactionId
-    ) {
-      navigate(mostRecentOverviewPage, { replace: true });
-    }
+      } else if (
+        prevTransactionId &&
+        transactionId &&
+        prevTransactionId !== transactionId &&
+        paramsTransactionId !== transactionId
+      ) {
+        navigate(mostRecentOverviewPage, { replace: true });
+      }
+    };
+
+    handleNavigation();
   }, [
     dispatch,
     navigate,

ui/pages/home/home.component.js

@@ -1,6 +1,6 @@
 import React, { PureComponent } from 'react';
 import PropTypes from 'prop-types';
-import { Navigate, Routes, Route } from 'react-router-dom-v5-compat';
+import { Navigate } from 'react-router-dom-v5-compat';
 import { Text, TextVariant, TextColor } from '@metamask/design-system-react';
 import { COHORT_NAMES } from '@metamask/subscription-controller';
 import {
@@ -929,18 +929,28 @@ export default class Home extends PureComponent {
       !showShieldEntryModal &&
       !showRecoveryPhrase;
 
+    const { location } = this.props;
+
+    // Handle connected routes
+    if (location?.pathname === CONNECTED_ROUTE) {
+      return (
+        <ScrollContainer className="main-container main-container--has-shadow">
+          <ConnectedSites navigate={this.props.navigate} />
+        </ScrollContainer>
+      );
+    }
+
+    if (location?.pathname === CONNECTED_ACCOUNTS_ROUTE) {
+      return (
+        <ScrollContainer className="main-container main-container--has-shadow">
+          <ConnectedAccounts navigate={this.props.navigate} />
+        </ScrollContainer>
+      );
+    }
+
+    // Render normal home content
     return (
       <ScrollContainer className="main-container main-container--has-shadow">
-        <Routes>
-          <Route
-            path={CONNECTED_ROUTE}
-            element={<ConnectedSites navigate={this.props.navigate} />}
-          />
-          <Route
-            path={CONNECTED_ACCOUNTS_ROUTE}
-            element={<ConnectedAccounts navigate={this.props.navigate} />}
-          />
-        </Routes>
         <div className="home__container">
           {dataCollectionForMarketing === null &&
           participateInMetaMetrics === true

ui/pages/routes/routes.component.tsx

@@ -11,6 +11,7 @@ import {
   useHistory,
   useLocation,
 } from 'react-router-dom';
+import type { To } from 'react-router-dom-v5-compat';
 import IdleTimer from 'react-idle-timer';
 import type { ApprovalType } from '@metamask/controller-utils';
 
@@ -169,13 +170,13 @@ import {
 } from './utils';
 
 // V5-compat navigate function type for bridging v5 routes with v5-compat components
-type V5CompatNavigate = (
-  to: string | number,
-  options?: {
-    replace?: boolean;
-    state?: Record<string, unknown>;
-  },
-) => void;
+type V5CompatNavigate = {
+  (
+    to: To,
+    options?: { replace?: boolean; state?: Record<string, unknown> },
+  ): void;
+  (delta: number): void;
+};
 
 /**
  * Creates a v5-compat navigate function from v5 history
@@ -186,15 +187,18 @@ type V5CompatNavigate = (
 const createV5CompatNavigate = (
   history: RouteComponentProps['history'],
 ): V5CompatNavigate => {
-  return (to, options = {}) => {
+  return ((
+    to: To | number,
+    options?: { replace?: boolean; state?: Record<string, unknown> },
+  ) => {
     if (typeof to === 'number') {
       history.go(to);
-    } else if (options.replace) {
-      history.replace(to, options.state);
+    } else if (options?.replace) {
+      history.replace(to as string, options.state);
     } else {
-      history.push(to, options.state);
+      history.push(to as string, options?.state);
     }
-  };
+  }) as V5CompatNavigate;
 };
 
 /**
@@ -692,12 +696,11 @@ export default function Routes() {
       <Suspense fallback={null}>
         {/* since the loading time is less than 200ms, we decided not to show a spinner fallback or anything */}
         <Switch>
-          <RouteWithLayout path={ONBOARDING_ROUTE} layout={LegacyLayout}>
-            {createV5CompatRoute(OnboardingFlow, {
-              includeNavigate: true,
-              includeLocation: true,
-            })}
-          </RouteWithLayout>
+          <RouteWithLayout
+            path={ONBOARDING_ROUTE}
+            component={OnboardingFlow}
+            layout={LegacyLayout}
+          />
           <RouteWithLayout
             path={LOCK_ROUTE}
             component={Lock}
@@ -863,11 +866,17 @@ export default function Routes() {
             })}
           </Route>
           <Route path={`${CONFIRMATION_V_NEXT_ROUTE}/:id?`}>
-            {createV5CompatRoute<{ id?: string }>(ConfirmationPage, {
-              wrapper: AuthenticatedV5Compat,
-              includeParams: true,
-              paramsAsProps: false,
-            })}
+            {(props: RouteComponentProps<{ id?: string }>) => {
+              const renderFn = createV5CompatRoute<{ id?: string }>(
+                ConfirmationPage,
+                {
+                  wrapper: AuthenticatedV5Compat,
+                  includeParams: true,
+                  paramsAsProps: false,
+                },
+              );
+              return renderFn(props);
+            }}
           </Route>
           <RouteWithLayout
             authenticated
@@ -1136,12 +1145,13 @@ export default function Routes() {
             component={ShieldPlan}
             layout={LegacyLayout}
           />
-          <RouteWithLayout
-            authenticated
-            path={DEFAULT_ROUTE}
-            component={Home}
-            layout={RootLayout}
-          />
+          <RouteWithLayout path={DEFAULT_ROUTE} layout={RootLayout}>
+            {createV5CompatRoute(Home, {
+              wrapper: AuthenticatedV5Compat,
+              includeNavigate: true,
+              includeLocation: true,
+            })}
+          </RouteWithLayout>
         </Switch>
       </Suspense>
     );