From 57035b3773c0ccb802ea58870bab02c8e21b49a7 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Tue, 27 Aug 2024 14:22:10 -0400 Subject: [PATCH] =?UTF-8?q?feat(rr6):=20Upgrade=20React=20Router=203=20?= =?UTF-8?q?=E2=86=92=206=20(#70632)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is tracked by https://github.com/getsentry/frontend-tsc/issues/23 --- package.json | 2 + static/app/bootstrap/initializeSdk.tsx | 35 +++- static/app/components/links/link.tsx | 12 ++ static/app/components/links/listLink.tsx | 38 +++- static/app/main.tsx | 26 ++- static/app/routes.tsx | 21 ++- static/app/types/system.tsx | 5 + static/app/utils/browserHistory.tsx | 73 +++++++- .../app/utils/reactRouter6Compat/location.tsx | 57 ++++++ .../app/utils/reactRouter6Compat/router.tsx | 171 ++++++++++++++++++ static/app/utils/useLocation.tsx | 26 ++- static/app/utils/useNavigate.tsx | 31 ++++ static/app/utils/useParams.tsx | 17 +- static/app/utils/useRouter.tsx | 67 ++++++- static/app/utils/useRoutes.tsx | 49 ++++- .../settings/components/settingsNavItem.tsx | 64 ++++++- tests/js/sentry-test/reactTestingLibrary.tsx | 67 +++++-- tests/js/setup.ts | 6 + yarn.lock | 42 ++++- 19 files changed, 752 insertions(+), 57 deletions(-) create mode 100644 static/app/utils/reactRouter6Compat/location.tsx create mode 100644 static/app/utils/reactRouter6Compat/router.tsx diff --git a/package.json b/package.json index 42dac4025c4b67..9b8aa0577dfa6b 100644 --- a/package.json +++ b/package.json @@ -125,6 +125,7 @@ "invariant": "^2.2.4", "ios-device-list": "1.1.37", "jed": "^1.1.0", + "jest-fetch-mock": "^3.0.3", "js-beautify": "^1.15.1", "js-cookie": "3.0.1", "less": "^4.1.3", @@ -156,6 +157,7 @@ "react-mentions": "4.4.10", "react-popper": "^2.3.0", "react-router": "3.2.6", + "react-router-dom": "^6.23.0", "react-select": "4.3.1", "react-sparklines": "1.7.0", "react-virtualized": "^9.22.5", diff --git a/static/app/bootstrap/initializeSdk.tsx b/static/app/bootstrap/initializeSdk.tsx index d7669e84e955a3..3cdcb8727c5bcb 100644 --- a/static/app/bootstrap/initializeSdk.tsx +++ b/static/app/bootstrap/initializeSdk.tsx @@ -9,6 +9,13 @@ import type {Config} from 'sentry/types/system'; import {addExtraMeasurements, addUIElementTag} from 'sentry/utils/performanceForSentry'; import normalizeUrl from 'sentry/utils/url/normalizeUrl'; import {getErrorDebugIds} from 'sentry/utils/getErrorDebugIds'; +import { + createRoutesFromChildren, + matchRoutes, + useLocation, + useNavigationType, +} from 'react-router-dom'; +import {useEffect} from 'react'; const SPA_MODE_ALLOW_URLS = [ 'localhost', @@ -49,20 +56,30 @@ const shouldOverrideBrowserProfiling = window?.__initialData?.user?.isSuperuser; * (e.g. `static/views/integrationPipeline`) */ function getSentryIntegrations(routes?: Function) { + const reactRouterIntegration = window.__SENTRY_USING_REACT_ROUTER_SIX + ? Sentry.reactRouterV6BrowserTracingIntegration({ + useEffect: useEffect, + useLocation: useLocation, + useNavigationType: useNavigationType, + createRoutesFromChildren: createRoutesFromChildren, + matchRoutes: matchRoutes, + }) + : Sentry.reactRouterV3BrowserTracingIntegration({ + history: browserHistory as any, + routes: typeof routes === 'function' ? createRoutes(routes()) : [], + match, + enableLongAnimationFrame: true, + _experiments: { + enableInteractions: false, + }, + }); + const integrations = [ Sentry.extraErrorDataIntegration({ // 6 is arbitrary, seems like a nice number depth: 6, }), - Sentry.reactRouterV3BrowserTracingIntegration({ - history: browserHistory as any, - routes: typeof routes === 'function' ? createRoutes(routes()) : [], - match, - enableLongAnimationFrame: true, - _experiments: { - enableInteractions: false, - }, - }), + reactRouterIntegration, Sentry.browserProfilingIntegration(), Sentry.thirdPartyErrorFilterIntegration({ filterKeys: ['sentry-spa'], diff --git a/static/app/components/links/link.tsx b/static/app/components/links/link.tsx index 40d8f0d8b23d1d..68c109124d5a23 100644 --- a/static/app/components/links/link.tsx +++ b/static/app/components/links/link.tsx @@ -1,8 +1,10 @@ import {forwardRef} from 'react'; import {Link as RouterLink} from 'react-router'; +import {Link as Router6Link} from 'react-router-dom'; import styled from '@emotion/styled'; import type {LocationDescriptor} from 'history'; +import {locationDescriptorToTo} from 'sentry/utils/reactRouter6Compat/location'; import normalizeUrl from 'sentry/utils/url/normalizeUrl'; import {useLocation} from 'sentry/utils/useLocation'; @@ -46,6 +48,16 @@ function BaseLink({disabled, to, forwardedRef, ...props}: LinkProps): React.Reac to = normalizeUrl(to, location); if (!disabled && location) { + if (window.__SENTRY_USING_REACT_ROUTER_SIX) { + return ( + + ); + } + return ; } diff --git a/static/app/components/links/listLink.tsx b/static/app/components/links/listLink.tsx index 9b11e0b134fc56..bab5c58f3c48c7 100644 --- a/static/app/components/links/listLink.tsx +++ b/static/app/components/links/listLink.tsx @@ -1,14 +1,19 @@ import {Link as RouterLink} from 'react-router'; +import {NavLink} from 'react-router-dom'; import styled from '@emotion/styled'; import classNames from 'classnames'; import type {LocationDescriptor} from 'history'; +import {locationDescriptorToTo} from 'sentry/utils/reactRouter6Compat/location'; import normalizeUrl from 'sentry/utils/url/normalizeUrl'; +import {useLocation} from 'sentry/utils/useLocation'; import useRouter from 'sentry/utils/useRouter'; -type LinkProps = Omit, 'to'>; - -type Props = LinkProps & { +interface ListLinkProps + extends Omit< + React.DetailedHTMLProps, HTMLAnchorElement>, + 'href' | 'target' | 'as' | 'css' | 'ref' + > { /** * Link target. We don't want to expose the ToLocationFunction on this component. */ @@ -19,7 +24,7 @@ type Props = LinkProps & { * Should be should be supplied by the parent component */ isActive?: (location: LocationDescriptor, indexOnly?: boolean) => boolean; -}; +} function ListLink({ children, @@ -29,18 +34,33 @@ function ListLink({ index = false, disabled = false, ...props -}: Props) { +}: ListLinkProps) { const router = useRouter(); + const location = useLocation(); const targetLocation = typeof to === 'string' ? {pathname: to} : to; const target = normalizeUrl(targetLocation); - const active = isActive?.(target, index) ?? router.isActive(target, index); + const active = + isActive?.(target, index) ?? + // XXX(epurkhiser): our shim for router.isActive will throw an error in + // react-router 6. Fallback to manually checking if the path is active + (window.__SENTRY_USING_REACT_ROUTER_SIX + ? location.pathname === (typeof target === 'string' ? target : target.pathname) + : router.isActive(target, index)); + + const link = window.__SENTRY_USING_REACT_ROUTER_SIX ? ( + + {children} + + ) : ( + + {children} + + ); return ( - - {children} - + {link} ); } diff --git a/static/app/main.tsx b/static/app/main.tsx index 900292d8535daa..69c4c83e197d66 100644 --- a/static/app/main.tsx +++ b/static/app/main.tsx @@ -1,13 +1,18 @@ import {Router, RouterContext} from 'react-router'; +import {createBrowserRouter, RouterProvider} from 'react-router-dom'; +import {wrapCreateBrowserRouter} from '@sentry/react'; import {ReactQueryDevtools} from '@tanstack/react-query-devtools'; import DemoHeader from 'sentry/components/demo/demoHeader'; import {OnboardingContextProvider} from 'sentry/components/onboarding/onboardingContext'; import {ThemeAndStyleProvider} from 'sentry/components/themeAndStyleProvider'; import {USE_REACT_QUERY_DEVTOOL} from 'sentry/constants'; -import {routes} from 'sentry/routes'; +import {routes, routes6} from 'sentry/routes'; import ConfigStore from 'sentry/stores/configStore'; -import {browserHistory} from 'sentry/utils/browserHistory'; +import { + browserHistory, + DANGEROUS_SET_REACT_ROUTER_6_HISTORY, +} from 'sentry/utils/browserHistory'; import { DEFAULT_QUERY_CLIENT_CONFIG, QueryClient, @@ -29,15 +34,26 @@ function renderRouter(props: any) { const queryClient = new QueryClient(DEFAULT_QUERY_CLIENT_CONFIG); +const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createBrowserRouter); +const router = sentryCreateBrowserRouter(routes6); + +if (window.__SENTRY_USING_REACT_ROUTER_SIX) { + DANGEROUS_SET_REACT_ROUTER_6_HISTORY(router); +} + function Main() { return ( {ConfigStore.get('demoMode') && } - - {routes()} - + {window.__SENTRY_USING_REACT_ROUTER_SIX ? ( + + ) : ( + + {routes()} + + )} {USE_REACT_QUERY_DEVTOOL && ( diff --git a/static/app/routes.tsx b/static/app/routes.tsx index 4c59731a6acdd7..f4c8329800ec7a 100644 --- a/static/app/routes.tsx +++ b/static/app/routes.tsx @@ -28,6 +28,7 @@ import RouteNotFound from 'sentry/views/routeNotFound'; import SettingsWrapper from 'sentry/views/settings/components/settingsWrapper'; import {IndexRoute, Route} from './components/route'; +import {buildReactRouter6Routes} from './utils/reactRouter6Compat/router'; const hook = (name: HookName) => HookStore.get(name).map(cb => cb()); @@ -690,15 +691,13 @@ function buildRoutes() { )} /> )} - {USING_CUSTOMER_DOMAIN && ( - import('sentry/views/settings/organizationGeneralSettings') - )} - /> - )} + import('sentry/views/settings/organizationGeneralSettings') + )} + /> router.navigate(locationDescriptorToTo(to)), + replace: to => router.navigate(locationDescriptorToTo(to), {replace: true}), + go: n => router.navigate(n), + goBack: () => router.navigate(-1), + goForward: () => router.navigate(1), + + // XXX(epurkhiser): react-router 6's BrowserHistory does not let you create + // multiple listeners. This implementation is similar but allows multiple + // listeners. + listen: _listener => { + // eslint-disable-next-line no-console + console.error('browserHistory.listen not implemented on react-router 6 shim'); + return () => {}; + }, + + listenBefore: _hook => { + // eslint-disable-next-line no-console + console.error('browserHistory.listenBefore not implemented on react-router 6 shim'); + return () => {}; + }, + transitionTo: _location => { + // eslint-disable-next-line no-console + console.error('browserHistory.transitionTo not implemented on react-router 6 shim'); + }, + createKey: () => { + // eslint-disable-next-line no-console + console.error('browserHistory.createKey not implemented on react-router 6 shim'); + return ''; + }, + createPath: () => { + // eslint-disable-next-line no-console + console.error('browserHistory.createPath not implemented on react-router 6 shim'); + return ''; + }, + createHref: () => { + // eslint-disable-next-line no-console + console.error('browserHistory.createHref not implemented on react-router 6 shim'); + return ''; + }, + createLocation: () => { + // eslint-disable-next-line no-console + console.error( + 'browserHistory.createLocation not implemented on react-router 6 shim' + ); + return undefined as any; + }, + getCurrentLocation: () => { + // eslint-disable-next-line no-console + console.error( + 'browserHistory.getCurrentLocation not implemented on react-router 6 shim' + ); + return undefined as any; + }, + }; + + browserHistory = compat6BrowserHistory; +} diff --git a/static/app/utils/reactRouter6Compat/location.tsx b/static/app/utils/reactRouter6Compat/location.tsx new file mode 100644 index 00000000000000..95cc76a3d77704 --- /dev/null +++ b/static/app/utils/reactRouter6Compat/location.tsx @@ -0,0 +1,57 @@ +import type {Location as Location6, To} from 'react-router-dom'; +import type {Location as Location3, LocationDescriptor, Query} from 'history'; +import * as qs from 'query-string'; + +/** + * Translates a react-router 3 LocationDescriptor to a react-router 6 To. + */ +export function locationDescriptorToTo(path: LocationDescriptor): To { + if (typeof path === 'string') { + return path; + } + + const to: To = { + pathname: path.pathname, + }; + + if (path.hash) { + to.hash = path.hash; + } + if (path.search) { + to.search = path.search; + } + if (path.query) { + to.search = `?${qs.stringify(path.query)}`; + } + + // XXX(epurkhiser): We ignore the location state param + + return to; +} + +type DefaultQuery = { + [key: string]: T | T[] | null | undefined; +}; + +/** + * Translate react-router 6 Location object to a react-router3 Location + */ +export function location6ToLocation3( + location: Location6 +): Location3 { + const {pathname, search, hash, state, key} = location; + + return { + pathname: pathname, + search: search, + query: qs.parse(search) as Q, + hash: hash, + state, + key, + + // XXX(epurkhiser): It would be possible to extract this from the + // react-router 6 browserHistory object. But beecause of how we're + // shimming it it's a little hard, so for now just mock + action: 'POP', + } satisfies Location3; +} diff --git a/static/app/utils/reactRouter6Compat/router.tsx b/static/app/utils/reactRouter6Compat/router.tsx new file mode 100644 index 00000000000000..2c9a99d511ffe7 --- /dev/null +++ b/static/app/utils/reactRouter6Compat/router.tsx @@ -0,0 +1,171 @@ +import {Children, isValidElement} from 'react'; +import { + generatePath, + Navigate, + type NavigateProps, + Outlet, + type RouteObject, + useOutletContext, +} from 'react-router-dom'; + +import {USING_CUSTOMER_DOMAIN} from 'sentry/constants'; +import {useLocation} from 'sentry/utils/useLocation'; +import {useParams} from 'sentry/utils/useParams'; +import useRouter from 'sentry/utils/useRouter'; +import {useRoutes} from 'sentry/utils/useRoutes'; +import withDomainRedirect from 'sentry/utils/withDomainRedirect'; +import withDomainRequired from 'sentry/utils/withDomainRequired'; + +function isValidComponent( + element: JSX.Element +): element is React.ReactElement> { + return typeof element.type !== 'string'; +} + +/** + * Because some of our views use cloneElement to inject route props into the + * children views, we need to capture those props and pass them as outlet + * context. The WithReactRouter3Props HoC component will inject the outlet + * context into the view + */ +function OurOutlet(props: any) { + return ; +} + +/** + * HoC which injects params and a route object that emulate react-router3 + */ +function withReactRouter3Props(Component: React.ComponentType) { + function WithReactRouter3Props() { + const params = useParams(); + const router = useRouter(); + const routes = useRoutes(); + const location = useLocation(); + const outletContext = useOutletContext<{}>(); + + return ( + + + + ); + } + + return WithReactRouter3Props; +} + +function NoOp({children}: {children: JSX.Element}) { + return children; +} + +interface RedirectProps extends Omit { + /** + * Our compat redirect only supports string to + */ + to: string; +} + +/** + * Wraps a declarative `Navigate` component to interpolate the params of `to` + */ +function Redirect({to, ...rest}: RedirectProps) { + const params = useParams(); + + return ; +} +Redirect.displayName = 'Redirect'; + +function getElement(Component: React.ComponentType | undefined) { + if (!Component) { + return undefined; + } + + const WrappedComponent = withReactRouter3Props(Component); + + return ; +} + +/** + * Transforms a react-router 3 style route tree into a valid react-router 6 + * router tree. + */ +export function buildReactRouter6Routes(tree: JSX.Element) { + const routes: RouteObject[] = []; + + Children.forEach(tree, routeNode => { + if (!isValidElement(routeNode)) { + return; + } + if (!isValidComponent(routeNode)) { + return; + } + + const isRoute = routeNode.type.displayName === 'Route'; + const isIndexRoute = routeNode.type.displayName === 'IndexRoute'; + + const isRedirect = routeNode.type.displayName === 'Redirect'; + const isIndexRedirect = routeNode.type.displayName === 'IndexRedirect'; + + if (isIndexRedirect) { + routes.push({index: true, element: }); + } + if (isRedirect) { + routes.push({ + path: routeNode.props.from, + element: , + }); + } + + // Elements that are not Route components are likely fragments, just + // traverse into their children in this case. + if (!isRoute && !isIndexRoute) { + routes.push(...buildReactRouter6Routes(routeNode.props.children)); + return; + } + + const {path, component: Component, children, name, withOrgPath} = routeNode.props; + const element = getElement(Component); + + // XXX(epurkhiser): Store the name prop that we use for breadcrumbs in the + // handle. This is a react-router 6 concept for where to put arbitrary + // route data + const handle = {name}; + + if (isIndexRoute) { + routes.push({index: true, element, handle}); + return; + } + + if (!withOrgPath) { + routes.push({path, element, handle, children: buildReactRouter6Routes(children)}); + return; + } + + // XXX(epurkhiser): This duplicates the customer domain logic in + // components/route.tsx. When the route has withOrgPath we create two + // route. + + if (USING_CUSTOMER_DOMAIN) { + routes.push({ + path, + element: getElement(withDomainRequired(Component ?? NoOp) as any), + children: buildReactRouter6Routes(children), + handle, + }); + } + + routes.push({ + path: `/organizations/:orgId${path}`, + element: getElement(withDomainRedirect(Component ?? NoOp)), + children: buildReactRouter6Routes(children), + handle, + }); + }); + + return routes; +} diff --git a/static/app/utils/useLocation.tsx b/static/app/utils/useLocation.tsx index 242abbbbf1fdf4..c9f5c737be37bb 100644 --- a/static/app/utils/useLocation.tsx +++ b/static/app/utils/useLocation.tsx @@ -1,12 +1,34 @@ +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'; + type DefaultQuery = { [key: string]: T | T[] | null | undefined; }; export function useLocation(): Location { - const route = useRouteContext(); - return route.location; + // 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 = window.__SENTRY_USING_REACT_ROUTER_SIX && NODE_ENV !== 'test'; + + if (!useReactRouter6) { + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + return useRouteContext().location; + } + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const router6Location = useReactRouter6Location(); + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const location = useMemo( + () => location6ToLocation3(router6Location), + [router6Location] + ); + + return location; } diff --git a/static/app/utils/useNavigate.tsx b/static/app/utils/useNavigate.tsx index c3a5254e2a56a7..006c598ad708ca 100644 --- a/static/app/utils/useNavigate.tsx +++ b/static/app/utils/useNavigate.tsx @@ -1,8 +1,11 @@ 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'; import useRouter from './useRouter'; type NavigateOptions = { @@ -22,13 +25,41 @@ interface ReactRouter3Navigate { * @see https://reactrouter.com/hooks/use-navigate */ export function useNavigate() { + // 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 = window.__SENTRY_USING_REACT_ROUTER_SIX && NODE_ENV !== 'test'; + + if (useReactRouter6) { + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration + const router6Navigate = useReactRouter6Navigate(); + + // XXX(epurkhiser): Translate legacy LocationDescriptor to To in the + // navigate helper. + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration + const navigate = useCallback( + (to: LocationDescriptor | number, options: NavigateOptions = {}) => + typeof to === 'number' + ? router6Navigate(to) + : router6Navigate(locationDescriptorToTo(to), options), + [router6Navigate] + ); + + return navigate; + } + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration const router = useRouter(); + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration const hasMountedRef = useRef(false); + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration useEffect(() => { hasMountedRef.current = true; }); + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration const navigate = useCallback( (to: LocationDescriptor | number, options: NavigateOptions = {}) => { if (!hasMountedRef.current) { diff --git a/static/app/utils/useParams.tsx b/static/app/utils/useParams.tsx index 68855df6baa3d4..18c61dc06ef8ce 100644 --- a/static/app/utils/useParams.tsx +++ b/static/app/utils/useParams.tsx @@ -1,10 +1,23 @@ import {useMemo} from 'react'; +import {useParams as useReactRouter6Params} from 'react-router-dom'; -import {CUSTOMER_DOMAIN, USING_CUSTOMER_DOMAIN} from 'sentry/constants'; +import {CUSTOMER_DOMAIN, NODE_ENV, USING_CUSTOMER_DOMAIN} from 'sentry/constants'; import {useRouteContext} from 'sentry/utils/useRouteContext'; export function useParams

>(): P { - const contextParams = useRouteContext().params; + // 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 = window.__SENTRY_USING_REACT_ROUTER_SIX && NODE_ENV !== 'test'; + + let contextParams: any; + + if (useReactRouter6) { + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + contextParams = useReactRouter6Params(); + } else { + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + contextParams = useRouteContext().params; + } // Memoize params as mutating for customer domains causes other hooks // that depend on `useParams()` to refresh infinitely. diff --git a/static/app/utils/useRouter.tsx b/static/app/utils/useRouter.tsx index 8352aad84d40f6..7d82f70a5cb8fe 100644 --- a/static/app/utils/useRouter.tsx +++ b/static/app/utils/useRouter.tsx @@ -1,5 +1,16 @@ +import {useMemo} from 'react'; +import type {InjectedRouter} from 'react-router'; +import type {RouteHook} from 'react-router/lib/Router'; +import type {LocationDescriptor} from 'history'; + +import {NODE_ENV} from 'sentry/constants'; import {useRouteContext} from 'sentry/utils/useRouteContext'; +import {useLocation} from './useLocation'; +import {useNavigate} from './useNavigate'; +import {useParams} from './useParams'; +import {useRoutes} from './useRoutes'; + /** * @deprecated Please do not use this. Use a specific hook instead. Including * use{Location,Params,Routes,Navigate}. @@ -7,8 +18,60 @@ import {useRouteContext} from 'sentry/utils/useRouteContext'; * react-router 6 does not include this hook. */ function useRouter() { - const route = useRouteContext(); - return route.router; + // 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 = window.__SENTRY_USING_REACT_ROUTER_SIX && NODE_ENV !== 'test'; + + if (!useReactRouter6) { + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + return useRouteContext().router; + } + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const navigate = useNavigate(); + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const location = useLocation(); + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const params = useParams(); + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const routes = useRoutes(); + + // XXX(epurkhiser): We emulate the react-router 3 `router` interface here + // biome-ignore lint/correctness/useHookAtTopLevel: react-router 6 migration + const router = useMemo( + () => + ({ + go: delta => navigate(delta), + push: path => navigate(path), + replace: path => navigate(path, {replace: true}), + goBack: () => navigate(-1), + goForward: () => navigate(1), + location, + params, + routes, + + // TODO(epurkhiser): These need correct shims + isActive: (_location: LocationDescriptor, _indexOnly?: boolean) => { + // eslint-disable-next-line no-console + console.error('isActive not implemented for react-router 6 migration'); + return false; + }, + createPath: (_pathOrLoc: LocationDescriptor, _query?: any) => { + throw new Error('createpath not implemented for react-router 6 migration'); + }, + createHref: (_pathOrLoc: LocationDescriptor, _query?: any) => { + throw new Error('createHref not implemented for react-router 6 migration'); + }, + setRouteLeaveHook: (_route: any, _callback: RouteHook) => () => { + throw new Error( + 'setRouteLeave hook not implemented for react-router6 migration' + ); + }, + }) satisfies InjectedRouter, + [location, navigate, params, routes] + ); + + return router; } export default useRouter; diff --git a/static/app/utils/useRoutes.tsx b/static/app/utils/useRoutes.tsx index c2dcfe0d4be089..fec958e59a5f00 100644 --- a/static/app/utils/useRoutes.tsx +++ b/static/app/utils/useRoutes.tsx @@ -1,6 +1,51 @@ +import {useMemo} from 'react'; +import type {PlainRoute} from 'react-router'; +import {useMatches} from 'react-router-dom'; + +import {NODE_ENV} from 'sentry/constants'; import {useRouteContext} from 'sentry/utils/useRouteContext'; export function useRoutes() { - const route = useRouteContext(); - return route.routes; + // 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 = window.__SENTRY_USING_REACT_ROUTER_SIX && NODE_ENV !== 'test'; + + if (!useReactRouter6) { + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration + return useRouteContext().routes; + } + + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration + const matches = useMatches(); + + // XXX(epurkhiser): This transforms react-router 6 style matches back to old + // style react-router 3 rroute matches. + // + // biome-ignore lint/correctness/useHookAtTopLevel: react-router-6 migration + return useMemo( + () => + matches.reduce((acc, match) => { + const currentPrefix = acc.map(route => route.path ?? '').join(''); + + // In react-router 6 each pathname is the full matching path. In + // react-router 3 each match object just has the matching part. We can + // reconstruct this by removing the prefix from the previous routes as + // we build them + let path = match.pathname.slice(currentPrefix.length); + + // XXX: Another difference we have to account for is trailing slashes. + // I'm not 100% sure why but react-router 6 seems to trim slashes. + // Let's ensure if we have a route that it ends with a slash + if (path !== '' && !path.endsWith('/')) { + path = `${path}/`; + } + + // We put things like `name` (for breadcrumbs) in the handle. Extract + // it out here + const extra: any = match.handle; + + return [...acc, {path, ...extra}]; + }, []), + [matches] + ); } diff --git a/static/app/views/settings/components/settingsNavItem.tsx b/static/app/views/settings/components/settingsNavItem.tsx index fdca5fa6cd6fd2..4eec9723589cbd 100644 --- a/static/app/views/settings/components/settingsNavItem.tsx +++ b/static/app/views/settings/components/settingsNavItem.tsx @@ -1,7 +1,9 @@ import type {ReactElement} from 'react'; import {Fragment} from 'react'; import {Link as RouterLink} from 'react-router'; +import {NavLink as Router6NavLink} from 'react-router-dom'; import styled from '@emotion/styled'; +import type {LocationDescriptor} from 'history'; import Badge from 'sentry/components/badge/badge'; import FeatureBadge from 'sentry/components/badge/featureBadge'; @@ -9,17 +11,18 @@ import HookOrDefault from 'sentry/components/hookOrDefault'; import {Tooltip} from 'sentry/components/tooltip'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; +import {locationDescriptorToTo} from 'sentry/utils/reactRouter6Compat/location'; type Props = { label: React.ReactNode; - to: React.ComponentProps['to']; + to: LocationDescriptor; badge?: string | number | null | ReactElement; id?: string; index?: boolean; onClick?: (e: React.MouseEvent) => void; }; -function SettingsNavItem({badge, label, index, id, ...props}: Props) { +function SettingsNavItem({badge, label, index, id, to, ...props}: Props) { const LabelHook = HookOrDefault({ hookName: 'sidebar:item-label', defaultComponent: ({children}) => {children}, @@ -43,14 +46,69 @@ function SettingsNavItem({badge, label, index, id, ...props}: Props) { renderedBadge = badge; } + if (window.__SENTRY_USING_REACT_ROUTER_SIX) { + return ( + + {label} + {badge ? renderedBadge : null} + + ); + } + return ( - + {label} {badge ? renderedBadge : null} ); } +const StyledNavItem6 = styled(Router6NavLink)` + display: block; + color: ${p => p.theme.gray300}; + font-size: 14px; + line-height: 30px; + position: relative; + + &.active { + color: ${p => p.theme.textColor}; + + &:before { + background: ${p => p.theme.active}; + } + } + + &:hover, + &:focus, + &:active { + color: ${p => p.theme.textColor}; + outline: none; + } + + &:focus-visible { + outline: none; + background: ${p => p.theme.backgroundSecondary}; + padding-left: 15px; + margin-left: -15px; + border-radius: 3px; + + &:before { + left: -15px; + } + } + + &:before { + position: absolute; + content: ''; + display: block; + top: 4px; + left: -30px; + height: 20px; + width: 4px; + background: transparent; + border-radius: 0 2px 2px 0; + } +`; const StyledNavItem = styled(RouterLink)` display: block; color: ${p => p.theme.gray300}; diff --git a/tests/js/sentry-test/reactTestingLibrary.tsx b/tests/js/sentry-test/reactTestingLibrary.tsx index 4be922a96f2b35..7ae40c93c03782 100644 --- a/tests/js/sentry-test/reactTestingLibrary.tsx +++ b/tests/js/sentry-test/reactTestingLibrary.tsx @@ -1,9 +1,12 @@ import {Component} from 'react'; import type {InjectedRouter} from 'react-router'; +import {Router} from 'react-router-dom'; import {cache} from '@emotion/css'; // eslint-disable-line @emotion/no-vanilla import {CacheProvider, ThemeProvider} from '@emotion/react'; import * as rtl from '@testing-library/react'; // eslint-disable-line no-restricted-imports import userEvent from '@testing-library/user-event'; // eslint-disable-line no-restricted-imports +import {createMemoryHistory} from 'history'; +import * as qs from 'query-string'; import {makeTestQueryClient} from 'sentry-test/queryClient'; @@ -57,23 +60,63 @@ function makeAllTheProviders(providers: ProviderOptions) { const optionalOrganization = providers.organization === null ? null : organization; return function ({children}: {children?: React.ReactNode}) { + const content = ( + + + {children} + + + ); + + // Inject legacy react-router 3 style router mocked navigation functions + // into the memory history used in react router 6 + + const history = createMemoryHistory(); + 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; + } + + // 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); + }; + + // TODO(__SENTRY_USING_REACT_ROUTER_SIX): For some reason getsentry is + // infering the type of this wrong. Unclear why that is happening + const Router6 = Router as any; + + const routerContainer = window.__SENTRY_USING_REACT_ROUTER_SIX ? ( + + {content} + + ) : ( + content + ); + return ( - - - {children} - - + {routerContainer} diff --git a/tests/js/setup.ts b/tests/js/setup.ts index 7c0797f5c61794..7d19630af0feaf 100644 --- a/tests/js/setup.ts +++ b/tests/js/setup.ts @@ -3,6 +3,7 @@ import '@testing-library/jest-dom'; /* eslint-env node */ import type {ReactElement} from 'react'; import {configure as configureRtl} from '@testing-library/react'; // eslint-disable-line no-restricted-imports +import {enableFetchMocks} from 'jest-fetch-mock'; import {webcrypto} from 'node:crypto'; import {TextDecoder, TextEncoder} from 'node:util'; import {ConfigFixture} from 'sentry-fixture/config'; @@ -20,6 +21,11 @@ import * as performanceForSentry from 'sentry/utils/performanceForSentry'; */ setLocale(DEFAULT_LOCALE_DATA); +/** + * Setup fetch mocks (needed to define the `Request` global) + */ +enableFetchMocks(); + /** * XXX(epurkhiser): Gross hack to fix a bug in jsdom which makes testing of * framer-motion SVG components fail diff --git a/yarn.lock b/yarn.lock index f5959538dcf237..8d594ea851c92c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2931,6 +2931,11 @@ dependencies: "@react-types/shared" "^3.22.1" +"@remix-run/router@1.19.1": + version "1.19.1" + resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.19.1.tgz#984771bfd1de2715f42394c87fb716c1349e014f" + integrity sha512-S45oynt/WH19bHbIXjtli6QmwNYvaz+vtnubvNpNDvUOoA/OWh6j1OikIP3G+v5GHdxyC6EXoChG3HgYGEUfcg== + "@rsdoctor/client@0.3.11": version "0.3.11" resolved "https://registry.yarnpkg.com/@rsdoctor/client/-/client-0.3.11.tgz#010fe58c0872f05e2d327f8c83053ab43fc40c23" @@ -5474,6 +5479,13 @@ cronstrue@^2.50.0: resolved "https://registry.yarnpkg.com/cronstrue/-/cronstrue-2.50.0.tgz#eabba0f915f186765258b707b7a3950c663b5573" integrity sha512-ULYhWIonJzlScCCQrPUG5uMXzXxSixty4djud9SS37DoNxDdkeRocxzHuAo4ImRBUK+mAuU5X9TSwEDccnnuPg== +cross-fetch@^3.0.4: + version "3.1.8" + resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.8.tgz#0327eba65fd68a7d119f8fb2bf9334a1a7956f82" + integrity sha512-cvA+JwZoU0Xq+h6WkMvAUqPEYy92Obet6UdKLfW60qn99ftItKjB5T+BkyWOFWe2pUyfQ+IJHmpOTznqk1M6Kg== + dependencies: + node-fetch "^2.6.12" + cross-spawn@^7.0.0, cross-spawn@^7.0.2, cross-spawn@^7.0.3: version "7.0.3" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.3.tgz#f73a85b9d5d41d045551c177e2882d4ac85728a6" @@ -8158,6 +8170,14 @@ jest-fail-on-console@3.3.0: resolved "https://registry.yarnpkg.com/jest-fail-on-console/-/jest-fail-on-console-3.3.0.tgz#5952b4706faf91584009b4e27f7d60a930c85b11" integrity sha512-J9rnFQvQwkcGJw01zCEKe2Uag+E926lFgIyaQGep2LqhQH7OCRHyD+tm/jnNoKlSRnOBO60DmzMjeQAVI3f5cw== +jest-fetch-mock@^3.0.3: + version "3.0.3" + resolved "https://registry.yarnpkg.com/jest-fetch-mock/-/jest-fetch-mock-3.0.3.tgz#31749c456ae27b8919d69824f1c2bd85fe0a1f3b" + integrity sha512-Ux1nWprtLrdrH4XwE7O7InRY6psIi3GOsqNESJgMJ+M5cv4A8Lh7SN9d2V2kKRZ8ebAfcd1LNyZguAOb6JiDqw== + dependencies: + cross-fetch "^3.0.4" + promise-polyfill "^8.1.3" + jest-get-type@^29.6.3: version "29.6.3" resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-29.6.3.tgz#36f499fdcea197c1045a127319c0481723908fd1" @@ -9198,7 +9218,7 @@ node-abort-controller@^3.0.1: resolved "https://registry.yarnpkg.com/node-abort-controller/-/node-abort-controller-3.0.1.tgz#f91fa50b1dee3f909afabb7e261b1e1d6b0cb74e" integrity sha512-/ujIVxthRs+7q6hsdjHMaj8hRG9NuWmwrz+JdRwZ14jdFoKSkm+vDsCbF9PLpnSqjaWQJuTmVtcWHNLr+vrOFw== -node-fetch@^2.6.7: +node-fetch@^2.6.12, node-fetch@^2.6.7: version "2.7.0" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.7.0.tgz#d0f0fa6e3e2dc1d27efcd8ad99d550bda94d187d" integrity sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A== @@ -10071,6 +10091,11 @@ progress@^2.0.3: resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.3.tgz#7e8cf8d8f5b8f239c1bc68beb4eb78567d572ef8" integrity sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA== +promise-polyfill@^8.1.3: + version "8.3.0" + resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-8.3.0.tgz#9284810268138d103807b11f4e23d5e945a4db63" + integrity sha512-H5oELycFml5yto/atYqmjyigJoAo3+OXwolYiH7OfQuYlAqhxNvTfiNMbV9hsC6Yp83yE5r2KTVmtrG6R9i6Pg== + prompts@^2.0.1: version "2.4.0" resolved "https://registry.yarnpkg.com/prompts/-/prompts-2.4.0.tgz#4aa5de0723a231d1ee9121c40fdf663df73f61d7" @@ -10311,6 +10336,14 @@ react-resizable@^3.0.4: prop-types "15.x" react-draggable "^4.0.3" +react-router-dom@^6.23.0: + version "6.26.1" + resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-6.26.1.tgz#a408892b41767a49dc94b3564b0e7d8e3959f623" + integrity sha512-veut7m41S1fLql4pLhxeSW3jlqs+4MtjRLj0xvuCEXsxusJCbs6I8yn9BxzzDX2XDgafrccY6hwjmd/bL54tFw== + dependencies: + "@remix-run/router" "1.19.1" + react-router "6.26.1" + react-router@3.2.6: version "3.2.6" resolved "https://registry.yarnpkg.com/react-router/-/react-router-3.2.6.tgz#cad202796a7bba3efc2100da453b3379c9d4aeb4" @@ -10325,6 +10358,13 @@ react-router@3.2.6: react-is "^16.13.0" warning "^3.0.0" +react-router@6.26.1: + version "6.26.1" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.26.1.tgz#88c64837e05ffab6899a49df2a1484a22471e4ce" + integrity sha512-kIwJveZNwp7teQRI5QmwWo39A5bXRyqpH0COKKmPnyD2vBvDwgFXSqDUYtt1h+FEyfnE8eXr7oe0MxRzVwCcvQ== + dependencies: + "@remix-run/router" "1.19.1" + react-select@4.3.1: version "4.3.1" resolved "https://registry.yarnpkg.com/react-select/-/react-select-4.3.1.tgz#389fc07c9bc7cf7d3c377b7a05ea18cd7399cb81"