feat(rr6): Add an option to disableRouterMocks (#78715)

evanpurkhiser · web-flow · commit a342e26477c4 · 2024-10-07T20:19:46.000Z
Allows us to write tests that actually use the routers functionality
diff --git a/static/app/utils/useLocation.tsx b/static/app/utils/useLocation.tsx
@@ -2,7 +2,6 @@ import {useMemo} from 'react';
 import {useLocation as useReactRouter6Location} from 'react-router-dom';
 import type {Location, Query} from 'history';
 
-import {NODE_ENV} from 'sentry/constants';
 import {useRouteContext} from 'sentry/utils/useRouteContext';
 
 import {location6ToLocation3} from './reactRouter6Compat/location';
@@ -14,11 +13,10 @@ type DefaultQuery<T = string> = {
 export function useLocation<Q extends Query = DefaultQuery>(): Location<Q> {
   // When running in test mode we still read from the legacy route context to
   // keep test compatability while we fully migrate to react router 6
-  const useReactRouter6 = NODE_ENV !== 'test';
+  const legacyRouterContext = useRouteContext();
 
-  if (!useReactRouter6) {
-    // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration
-    return useRouteContext().location;
+  if (legacyRouterContext) {
+    return legacyRouterContext.location;
   }
 
   // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration
diff --git a/static/app/utils/useNavigate.tsx b/static/app/utils/useNavigate.tsx
@@ -2,7 +2,6 @@ import {useCallback, useEffect, useRef} from 'react';
 import {useNavigate as useReactRouter6Navigate} from 'react-router-dom';
 import type {LocationDescriptor} from 'history';
 
-import {NODE_ENV} from 'sentry/constants';
 import normalizeUrl from 'sentry/utils/url/normalizeUrl';
 
 import {locationDescriptorToTo} from './reactRouter6Compat/location';
@@ -27,9 +26,9 @@ interface ReactRouter3Navigate {
 export function useNavigate(): ReactRouter3Navigate {
   // When running in test mode we still read from the legacy route context to
   // keep test compatability while we fully migrate to react router 6
-  const useReactRouter6 = NODE_ENV !== 'test';
+  const legacyRouterContext = useRouteContext();
 
-  if (useReactRouter6) {
+  if (!legacyRouterContext) {
     // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration
     const router6Navigate = useReactRouter6Navigate();
 
@@ -51,8 +50,7 @@ export function useNavigate(): ReactRouter3Navigate {
   // XXX(epurkihser): We are using react-router 3 here, to avoid recursive
   // dependencies we just use the useRouteContext instead of useRouter here
 
-  // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration
-  const {router} = useRouteContext();
+  const {router} = legacyRouterContext;
 
   // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration
   const hasMountedRef = useRef(false);
diff --git a/static/app/utils/useParams.spec.tsx b/static/app/utils/useParams.spec.tsx
@@ -89,7 +89,7 @@ describe('useParams', () => {
       let useParamsValue;
 
       function Component() {
-        const {params} = useRouteContext();
+        const {params} = useRouteContext()!;
         originalParams = params;
         useParamsValue = useParams();
         return (
@@ -127,7 +127,7 @@ describe('useParams', () => {
       let useParamsValue;
 
       function Component() {
-        const {params} = useRouteContext();
+        const {params} = useRouteContext()!;
         originalParams = params;
         useParamsValue = useParams();
         return (
diff --git a/static/app/utils/useParams.tsx b/static/app/utils/useParams.tsx
@@ -1,22 +1,22 @@
 import {useMemo} from 'react';
 import {useParams as useReactRouter6Params} from 'react-router-dom';
 
-import {CUSTOMER_DOMAIN, NODE_ENV, USING_CUSTOMER_DOMAIN} from 'sentry/constants';
-import {useRouteContext} from 'sentry/utils/useRouteContext';
+import {CUSTOMER_DOMAIN, USING_CUSTOMER_DOMAIN} from 'sentry/constants';
+
+import {useRouteContext} from './useRouteContext';
 
 export function useParams<P = Record<string, string>>(): P {
   // When running in test mode we still read from the legacy route context to
   // keep test compatability while we fully migrate to react router 6
-  const useReactRouter6 = NODE_ENV !== 'test';
+  const legacyRouterContext = useRouteContext();
 
   let contextParams: any;
 
-  if (useReactRouter6) {
+  if (!legacyRouterContext) {
     // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration
     contextParams = useReactRouter6Params();
   } else {
-    // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration
-    contextParams = useRouteContext().params;
+    contextParams = legacyRouterContext.params;
   }
 
   // Memoize params as mutating for customer domains causes other hooks
diff --git a/static/app/utils/useRouteContext.tsx b/static/app/utils/useRouteContext.tsx
@@ -3,9 +3,5 @@ import {useContext} from 'react';
 import {RouteContext} from 'sentry/views/routeContext';
 
 export function useRouteContext() {
-  const route = useContext(RouteContext);
-  if (route === null) {
-    throw new Error(`useRouteContext called outside of routes provider`);
-  }
-  return route;
+  return useContext(RouteContext);
 }
diff --git a/static/app/utils/useRouter.tsx b/static/app/utils/useRouter.tsx
@@ -1,13 +1,12 @@
 import {useMemo} from 'react';
 import type {LocationDescriptor} from 'history';
 
-import {NODE_ENV} from 'sentry/constants';
 import type {InjectedRouter} from 'sentry/types/legacyReactRouter';
-import {useRouteContext} from 'sentry/utils/useRouteContext';
 
 import {useLocation} from './useLocation';
 import {useNavigate} from './useNavigate';
 import {useParams} from './useParams';
+import {useRouteContext} from './useRouteContext';
 import {useRoutes} from './useRoutes';
 
 /**
@@ -19,11 +18,10 @@ import {useRoutes} from './useRoutes';
 function useRouter(): InjectedRouter<any, any> {
   // When running in test mode we still read from the legacy route context to
   // keep test compatability while we fully migrate to react router 6
-  const useReactRouter6 = NODE_ENV !== 'test';
+  const legacyRouterContext = useRouteContext();
 
-  if (!useReactRouter6) {
-    // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration
-    return useRouteContext().router;
+  if (legacyRouterContext) {
+    return legacyRouterContext.router;
   }
 
   // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration
diff --git a/static/app/utils/useRoutes.tsx b/static/app/utils/useRoutes.tsx
@@ -1,18 +1,17 @@
 import {useMemo} from 'react';
 import {useMatches} from 'react-router-dom';
 
-import {NODE_ENV} from 'sentry/constants';
 import type {PlainRoute} from 'sentry/types/legacyReactRouter';
-import {useRouteContext} from 'sentry/utils/useRouteContext';
+
+import {useRouteContext} from './useRouteContext';
 
 export function useRoutes(): PlainRoute<any>[] {
   // When running in test mode we still read from the legacy route context to
   // keep test compatability while we fully migrate to react router 6
-  const useReactRouter6 = NODE_ENV !== 'test';
+  const legacyRouterContext = useRouteContext();
 
-  if (!useReactRouter6) {
-    // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration
-    return useRouteContext().routes;
+  if (legacyRouterContext) {
+    return legacyRouterContext.routes;
   }
 
   // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration
diff --git a/static/app/views/routeContext.tsx b/static/app/views/routeContext.tsx
@@ -2,5 +2,10 @@ import {createContext} from 'react';
 
 import type {RouteContextInterface} from 'sentry/types/legacyReactRouter';
 
-// TODO(nisanthan): Better types. Context will be the `props` arg from the RouterProps render method. This is typed as `any` by react-router
+/**
+ * This is a legacy context that is primarily used in tests currently to allow
+ * for mocking the use{Location,Navigate,Routes,Params} hooks
+ *
+ * DO NOT use this outside of tests!
+ */
 export const RouteContext = createContext<RouteContextInterface | null>(null);
diff --git a/tests/js/sentry-test/reactTestingLibrary.tsx b/tests/js/sentry-test/reactTestingLibrary.tsx
@@ -22,29 +22,44 @@ import {instrumentUserEvent} from '../instrumentedEnv/userEventIntegration';
 import {initializeOrg} from './initializeOrg';
 
 interface ProviderOptions {
+  /**
+   * Do not shim the router use{Routes,Router,Navigate,Location} functions, and
+   * instead allow them to work as normal, rendering inside of a memory router.
+   *
+   * Wehn enabling this passing a `router` object *will do nothing*!
+   */
+  disableRouterMocks?: boolean;
   /**
    * Sets the OrganizationContext. You may pass null to provide no organization
    */
   organization?: Partial<Organization> | null;
   /**
-   * Sets the RouterContext
+   * Sets the RouterContext.
    */
   router?: Partial<InjectedRouter>;
 }
 
 interface Options extends ProviderOptions, rtl.RenderOptions {}
 
-function makeAllTheProviders(providers: ProviderOptions) {
+function makeAllTheProviders(options: ProviderOptions) {
   const {organization, router} = initializeOrg({
-    organization: providers.organization === null ? undefined : providers.organization,
-    router: providers.router,
+    organization: options.organization === null ? undefined : options.organization,
+    router: options.router,
   });
 
   // In some cases we may want to not provide an organization at all
-  const optionalOrganization = providers.organization === null ? null : organization;
+  const optionalOrganization = options.organization === null ? null : organization;
 
   return function ({children}: {children?: React.ReactNode}) {
     const content = (
+      <OrganizationContext.Provider value={optionalOrganization}>
+        <GlobalDrawer>{children}</GlobalDrawer>
+      </OrganizationContext.Provider>
+    );
+
+    const wrappedContent = options.disableRouterMocks ? (
+      content
+    ) : (
       <RouteContext.Provider
         value={{
           router,
@@ -53,41 +68,41 @@ function makeAllTheProviders(providers: ProviderOptions) {
           routes: router.routes,
         }}
       >
-        <OrganizationContext.Provider value={optionalOrganization}>
-          <GlobalDrawer>{children}</GlobalDrawer>
-        </OrganizationContext.Provider>
+        {content}
       </RouteContext.Provider>
     );
 
+    const history = createMemoryHistory();
+
     // Inject legacy react-router 3 style router mocked navigation functions
     // into the memory history used in react router 6
     //
     // TODO(epurkhiser): In a world without react-router 3 we should figure out
     // how to write our tests in a simpler way without all these shims
-
-    const history = createMemoryHistory();
-    Object.defineProperty(history, 'location', {get: () => router.location});
-    history.replace = router.replace;
-    history.push = (path: any) => {
-      if (typeof path === 'object' && path.search) {
-        path.query = qs.parse(path.search);
-        delete path.search;
-        delete path.hash;
-        delete path.state;
-        delete path.key;
-      }
-
-      // XXX(epurkhiser): This is a hack for react-router 3 to 6. react-router
-      // 6 will not convert objects into strings before pushing. We can detect
-      // this by looking for an empty hash, which we normally do not set for
-      // our browserHistory.push calls
-      if (typeof path === 'object' && path.hash === '') {
-        const queryString = path.query ? qs.stringify(path.query) : null;
-        path = `${path.pathname}${queryString ? `?${queryString}` : ''}`;
-      }
-
-      router.push(path);
-    };
+    if (!options.disableRouterMocks) {
+      Object.defineProperty(history, 'location', {get: () => router.location});
+      history.replace = router.replace;
+      history.push = (path: any) => {
+        if (typeof path === 'object' && path.search) {
+          path.query = qs.parse(path.search);
+          delete path.search;
+          delete path.hash;
+          delete path.state;
+          delete path.key;
+        }
+
+        // XXX(epurkhiser): This is a hack for react-router 3 to 6. react-router
+        // 6 will not convert objects into strings before pushing. We can detect
+        // this by looking for an empty hash, which we normally do not set for
+        // our browserHistory.push calls
+        if (typeof path === 'object' && path.hash === '') {
+          const queryString = path.query ? qs.stringify(path.query) : null;
+          path = `${path.pathname}${queryString ? `?${queryString}` : ''}`;
+        }
+
+        router.push(path);
+      };
+    }
 
     // By default react-router 6 catches exceptions and displays the stack
     // trace. For tests we want them to bubble out
@@ -98,7 +113,7 @@ function makeAllTheProviders(providers: ProviderOptions) {
     const routes: RouteObject[] = [
       {
         path: '*',
-        element: content,
+        element: wrappedContent,
         errorElement: <ErrorBoundary />,
       },
     ];
@@ -132,11 +147,12 @@ function makeAllTheProviders(providers: ProviderOptions) {
  */
 function render(
   ui: React.ReactElement,
-  {router, organization, ...rtlOptions}: Options = {}
+  {router, organization, disableRouterMocks, ...rtlOptions}: Options = {}
 ) {
   const AllTheProviders = makeAllTheProviders({
     organization,
     router,
+    disableRouterMocks,
   });
 
   return rtl.render(ui, {wrapper: AllTheProviders, ...rtlOptions});
