Routing: Remove Switch from Grafana routes (#94795)

* Routing: Use Routes instead of Switch

* Update routeProps

* Update GrafanaRoute.test

* Update DashboardScenePage.tsx

* Update DashboardPageProxy.test.tsx

* Remove exact paths

* Update parent routes

* Move route wrapper

* Update type

* Fix plugin paths

* Switch to the location hook
This commit is contained in:
Alex Khomenko 2024-10-17 18:11:31 +03:00 committed by GitHub
parent b2036ffcbf
commit ccedc41c57
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 35 additions and 65 deletions

View File

@ -1,10 +1,9 @@
import { Action, KBarProvider } from 'kbar';
import { Component, ComponentType } from 'react';
import { Provider } from 'react-redux';
import { Switch, RouteComponentProps } from 'react-router-dom';
import { CompatRoute, Navigate } from 'react-router-dom-v5-compat';
import { Route, Routes } from 'react-router-dom-v5-compat';
import { config, locationService, navigationLogger, reportInteraction } from '@grafana/runtime';
import { config, navigationLogger, reportInteraction } from '@grafana/runtime';
import { ErrorBoundaryAlert, GlobalStyles, PortalContainer } from '@grafana/ui';
import { getAppRoutes } from 'app/routes/routes';
import { store } from 'app/store/store';
@ -13,10 +12,9 @@ import { loadAndInitAngularIfEnabled } from './angular/loadAndInitAngularIfEnabl
import { GrafanaApp } from './app';
import { GrafanaContext } from './core/context/GrafanaContext';
import { SidecarContext } from './core/context/SidecarContext';
import { GrafanaRoute } from './core/navigation/GrafanaRoute';
import { GrafanaRouteWrapper } from './core/navigation/GrafanaRoute';
import { RouteDescriptor } from './core/navigation/types';
import { sidecarService } from './core/services/SidecarService';
import { contextSrv } from './core/services/context_srv';
import { ThemeProvider } from './core/utils/ConfigProvider';
import { LiveConnectionWarning } from './features/live/LiveConnectionWarning';
import { ExtensionRegistriesProvider } from './features/plugins/extensions/ExtensionRegistriesContext';
@ -55,30 +53,18 @@ export class AppWrapper extends Component<AppWrapperProps, AppWrapperState> {
}
renderRoute = (route: RouteDescriptor) => {
const roles = route.roles ? route.roles() : [];
return (
<CompatRoute
exact={route.exact === undefined ? true : route.exact}
sensitive={route.sensitive === undefined ? false : route.sensitive}
<Route
caseSensitive={route.sensitive === undefined ? false : route.sensitive}
path={route.path}
key={route.path}
render={(props: RouteComponentProps) => {
const location = locationService.getLocation();
// TODO[Router]: test this logic
if (roles?.length) {
if (!roles.some((r: string) => contextSrv.hasRole(r))) {
return <Navigate replace to="/" />;
}
}
return <GrafanaRoute {...props} route={route} location={location} />;
}}
element={<GrafanaRouteWrapper route={route} />}
/>
);
};
renderRoutes() {
return <Switch>{getAppRoutes().map((r) => this.renderRoute(r))}</Switch>;
return <Routes>{getAppRoutes().map((r) => this.renderRoute(r))}</Routes>;
}
render() {

View File

@ -18,25 +18,6 @@ const mockLocation = {
function setup(overrides: Partial<Props>) {
const props: Props = {
location: mockLocation,
history: {
length: 0,
action: 'PUSH',
location: mockLocation,
push: jest.fn(),
replace: jest.fn(),
go: jest.fn(),
goBack: jest.fn(),
goForward: jest.fn(),
block: jest.fn(),
listen: jest.fn(),
createHref: jest.fn(),
},
match: {
params: {},
isExact: false,
path: '',
url: '',
},
route: {
path: '/',
component: () => <div />,

View File

@ -1,4 +1,5 @@
import { Suspense, useEffect, useLayoutEffect } from 'react';
import { Navigate, useLocation } from 'react-router-dom-v5-compat';
// @ts-ignore
import Drop from 'tether-drop';
@ -6,12 +7,13 @@ import { locationSearchToObject, navigationLogger, reportPageview } from '@grafa
import { ErrorBoundary } from '@grafana/ui';
import { useGrafana } from '../context/GrafanaContext';
import { contextSrv } from '../services/context_srv';
import { GrafanaRouteError } from './GrafanaRouteError';
import { GrafanaRouteLoading } from './GrafanaRouteLoading';
import { GrafanaRouteComponentProps, RouteDescriptor } from './types';
export interface Props extends Omit<GrafanaRouteComponentProps, 'queryParams'> {}
export interface Props extends Pick<GrafanaRouteComponentProps, 'route' | 'location'> {}
export function GrafanaRoute(props: Props) {
const { chrome, keybindings } = useGrafana();
@ -60,6 +62,17 @@ export function GrafanaRoute(props: Props) {
);
}
export function GrafanaRouteWrapper({ route }: Pick<Props, 'route'>) {
const location = useLocation();
const roles = route.roles ? route.roles() : [];
if (roles?.length) {
if (!roles.some((r: string) => contextSrv.hasRole(r))) {
return <Navigate replace to="/" />;
}
}
return <GrafanaRoute route={route} location={location} />;
}
function getPageClasses(route: RouteDescriptor) {
return route.pageClass ? route.pageClass.split(' ') : [];
}

View File

@ -1,6 +1,4 @@
import { createMemoryHistory } from 'history';
import { merge } from 'lodash';
import { match } from 'react-router-dom';
import { GrafanaRouteComponentProps } from '../types';
@ -8,14 +6,12 @@ export function getRouteComponentProps<T extends {} = {}, Q extends Record<strin
overrides: Partial<GrafanaRouteComponentProps> = {}
): GrafanaRouteComponentProps<T, Q> {
const defaults: GrafanaRouteComponentProps<T, Q> = {
history: createMemoryHistory(),
location: {
hash: '',
pathname: '',
state: {},
search: '',
},
match: { params: {} } as match<T>,
route: {
path: '',
component: () => null,

View File

@ -1,14 +1,15 @@
import * as React from 'react';
import { RouteComponentProps } from 'react-router';
import { Location } from 'history';
import { ComponentType } from 'react';
import { UrlQueryMap } from '@grafana/data';
export interface GrafanaRouteComponentProps<T extends {} = {}, Q = UrlQueryMap> extends RouteComponentProps<T> {
export interface GrafanaRouteComponentProps<T extends {} = {}, Q = UrlQueryMap> {
route: RouteDescriptor;
queryParams: Q;
location: Location;
}
export type GrafanaRouteComponent<T extends {} = any> = React.ComponentType<GrafanaRouteComponentProps<T>>;
export type GrafanaRouteComponent<T extends {} = any> = ComponentType<GrafanaRouteComponentProps<T>>;
export interface RouteDescriptor {
path: string;
@ -18,6 +19,5 @@ export interface RouteDescriptor {
/** Can be used like an id for the route if the same component is used by many routes */
routeName?: string;
chromeless?: boolean;
exact?: boolean;
sensitive?: boolean;
}

View File

@ -16,7 +16,6 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] {
},
{
path: '/alerting/home',
exact: false,
component: importAlertingComponent(
() => import(/* webpackChunkName: "AlertingHome" */ 'app/features/alerting/unified/home/Home')
),

View File

@ -1,13 +1,12 @@
import { SafeDynamicImport } from 'app/core/components/DynamicImports/SafeDynamicImport';
import { RouteDescriptor } from 'app/core/navigation/types';
import { ROUTES } from './constants';
import { ROUTE_BASE_ID } from './constants';
export function getRoutes(): RouteDescriptor[] {
return [
{
path: ROUTES.Base,
exact: false,
path: `/${ROUTE_BASE_ID}/*`,
component: SafeDynamicImport(
() => import(/* webpackChunkName: "Connections"*/ 'app/features/connections/Connections')
),

View File

@ -185,7 +185,7 @@ describe('DashboardScenePage', () => {
getDashboardScenePageStateManager().clearDashboardCache();
loadDashboardMock.mockResolvedValue({ dashboard: updatedDashboard, meta: {} });
props.history.location.state = { routeReloadCounter: 1 };
props.location.state = { routeReloadCounter: 1 };
rerender(props);

View File

@ -19,14 +19,14 @@ import { getDashboardScenePageStateManager } from './DashboardScenePageStateMana
export interface Props
extends Omit<GrafanaRouteComponentProps<DashboardPageRouteParams, DashboardPageRouteSearchParams>, 'match'> {}
export function DashboardScenePage({ route, queryParams, history }: Props) {
export function DashboardScenePage({ route, queryParams, location }: Props) {
const params = useParams();
const { type, slug, uid } = params;
const prevMatch = usePrevious({ params });
const stateManager = getDashboardScenePageStateManager();
const { dashboard, isLoading, loadError } = stateManager.useState();
// After scene migration is complete and we get rid of old dashboard we should refactor dashboardWatcher so this route reload is not need
const routeReloadCounter = (history.location.state as any)?.routeReloadCounter;
const routeReloadCounter = (location.state as any)?.routeReloadCounter;
useEffect(() => {
if (route.routeName === DashboardRoutes.Normal && type === 'snapshot') {

View File

@ -91,7 +91,6 @@ function setup(props: Partial<DashboardPageProxyProps> & { uid?: string }) {
return render(
<DashboardPageProxy
location={locationService.getLocation()}
history={locationService.getHistory()}
queryParams={{}}
route={{ routeName: DashboardRoutes.Home, component: () => null, path: '/' }}
{...props}

View File

@ -19,8 +19,7 @@ export function getAppPluginRoutes(): RouteDescriptor[] {
const isSensitive = isStandalonePluginPage(navItem.id) && !navItem.url?.startsWith('/a/'); // Have case-sensitive URLs only for standalone pages that have custom URLs
return {
path,
exact: false, // route everything under this path to the plugin, so it can define more routes under this path
path: `${path}/*`,
sensitive: isSensitive,
component: () => <AppRootPage pluginId={navItem.pluginId} pluginNavSection={pluginNavSection} />,
};
@ -31,8 +30,7 @@ export function getAppPluginRoutes(): RouteDescriptor[] {
// Fallback route for plugins that don't have any pages under includes
{
path: '/a/:pluginId',
exact: false, // route everything under this path to the plugin, so it can define more routes under this path
path: '/a/:pluginId/*',
component: () => <AppRootPage pluginNavSection={navIndex.home} />,
},
];

View File

@ -366,7 +366,7 @@ export function getAppRoutes(): RouteDescriptor[] {
: () => <Navigate replace to="/admin" />,
},
{
path: '/admin/storage/:path*',
path: '/admin/storage/:path/*',
roles: () => ['Admin'],
component: SafeDynamicImport(
() => import(/* webpackChunkName: "StoragePage" */ 'app/features/storage/StoragePage')
@ -516,9 +516,8 @@ export function getAppRoutes(): RouteDescriptor[] {
),
},
config.featureToggles.exploreMetrics && {
path: '/explore/metrics',
path: '/explore/metrics/*',
chromeless: false,
exact: false,
roles: () => contextSrv.evaluatePermission([AccessControlAction.DataSourcesExplore]),
component: SafeDynamicImport(
() => import(/* webpackChunkName: "DataTrailsPage"*/ 'app/features/trails/DataTrailsPage')