Skip to content

Commit

Permalink
[APM] Remove useLocation and some minor route improvements (#76343)
Browse files Browse the repository at this point in the history
* [APM] Remove useLocation and some minor route improvements

* Replace `useLocation` and our `LocationContext` with `useLocation` from React Router. We can do this since we're now using the platform history, added in #76287.
* Pass in `RouteComponentProps` where appropriate to routes to use `history` and `location`.

This is in the service of #51963, but doesn't do anything with `useUrlParams` or any of the other changes specified in that issue.
  • Loading branch information
smith authored Sep 15, 2020
1 parent 98e6475 commit 3d91165
Show file tree
Hide file tree
Showing 34 changed files with 196 additions and 160 deletions.
4 changes: 4 additions & 0 deletions x-pack/plugins/apm/public/application/application.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import { createCallApmApi } from '../services/rest/createCallApmApi';
import { renderApp } from './';
import { disableConsoleWarning } from '../utils/testHelpers';

jest.mock('../services/rest/index_pattern', () => ({
createStaticIndexPattern: () => Promise.resolve(undefined),
}));

describe('renderApp', () => {
let mockConsole: jest.SpyInstance;

Expand Down
17 changes: 7 additions & 10 deletions x-pack/plugins/apm/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import { ScrollToTopOnPathChange } from '../components/app/Main/ScrollToTopOnPat
import { ApmPluginContext } from '../context/ApmPluginContext';
import { LicenseProvider } from '../context/LicenseContext';
import { LoadingIndicatorProvider } from '../context/LoadingIndicatorContext';
import { LocationProvider } from '../context/LocationContext';
import { UrlParamsProvider } from '../context/UrlParamsContext';
import { useBreadcrumbs } from '../hooks/use_breadcrumbs';
import { ApmPluginSetupDeps } from '../plugin';
Expand Down Expand Up @@ -99,15 +98,13 @@ export function ApmAppRoot({
<KibanaContextProvider services={{ ...core, ...plugins }}>
<i18nCore.Context>
<Router history={history}>
<LocationProvider>
<UrlParamsProvider>
<LoadingIndicatorProvider>
<LicenseProvider>
<App />
</LicenseProvider>
</LoadingIndicatorProvider>
</UrlParamsProvider>
</LocationProvider>
<UrlParamsProvider>
<LoadingIndicatorProvider>
<LicenseProvider>
<App />
</LicenseProvider>
</LoadingIndicatorProvider>
</UrlParamsProvider>
</Router>
</i18nCore.Context>
</KibanaContextProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { MockUrlParamsContextProvider } from '../../../../../context/UrlParamsCo
import { mockMoment, toJson } from '../../../../../utils/testHelpers';
import { ErrorGroupList } from '../index';
import props from './props.json';
import { MemoryRouter } from 'react-router-dom';

jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => {
return {
Expand All @@ -26,9 +27,11 @@ describe('ErrorGroupOverview -> List', () => {
it('should render empty state', () => {
const storeState = {};
const wrapper = mount(
<MockUrlParamsContextProvider>
<ErrorGroupList items={[]} serviceName="opbeans-python" />
</MockUrlParamsContextProvider>,
<MemoryRouter>
<MockUrlParamsContextProvider>
<ErrorGroupList items={[]} serviceName="opbeans-python" />
</MockUrlParamsContextProvider>
</MemoryRouter>,
storeState
);

Expand All @@ -37,11 +40,13 @@ describe('ErrorGroupOverview -> List', () => {

it('should render with data', () => {
const wrapper = mount(
<MockApmPluginContextWrapper>
<MockUrlParamsContextProvider>
<ErrorGroupList items={props.items} serviceName="opbeans-python" />
</MockUrlParamsContextProvider>
</MockApmPluginContextWrapper>
<MemoryRouter>
<MockApmPluginContextWrapper>
<MockUrlParamsContextProvider>
<ErrorGroupList items={props.items} serviceName="opbeans-python" />
</MockUrlParamsContextProvider>
</MockApmPluginContextWrapper>
</MemoryRouter>
);

expect(toJson(wrapper)).toMatchSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,33 +113,33 @@ function ServiceDetailsTransactions(
return <ServiceDetails {...props} tab="transactions" />;
}

function SettingsAgentConfiguration() {
function SettingsAgentConfiguration(props: RouteComponentProps<{}>) {
return (
<Settings>
<Settings {...props}>
<AgentConfigurations />
</Settings>
);
}

function SettingsAnomalyDetection() {
function SettingsAnomalyDetection(props: RouteComponentProps<{}>) {
return (
<Settings>
<Settings {...props}>
<AnomalyDetection />
</Settings>
);
}

function SettingsApmIndices() {
function SettingsApmIndices(props: RouteComponentProps<{}>) {
return (
<Settings>
<Settings {...props}>
<ApmIndices />
</Settings>
);
}

function SettingsCustomizeUI() {
function SettingsCustomizeUI(props: RouteComponentProps<{}>) {
return (
<Settings>
<Settings {...props}>
<CustomizeUI />
</Settings>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import { AgentConfigurationCreateEdit } from '../../../Settings/AgentConfigurati

type EditAgentConfigurationRouteHandler = RouteComponentProps<{}>;

export function EditAgentConfigurationRouteHandler({
history,
}: EditAgentConfigurationRouteHandler) {
const { search } = history.location;
export function EditAgentConfigurationRouteHandler(
props: EditAgentConfigurationRouteHandler
) {
const { search } = props.history.location;

// typescript complains because `pageStop` does not exist in `APMQueryParams`
// Going forward we should move away from globally declared query params and this is a first step
Expand All @@ -34,7 +34,7 @@ export function EditAgentConfigurationRouteHandler({
);

return (
<Settings>
<Settings {...props}>
<AgentConfigurationCreateEdit
pageStep={pageStep || 'choose-settings-step'}
existingConfigResult={res}
Expand All @@ -45,17 +45,17 @@ export function EditAgentConfigurationRouteHandler({

type CreateAgentConfigurationRouteHandlerProps = RouteComponentProps<{}>;

export function CreateAgentConfigurationRouteHandler({
history,
}: CreateAgentConfigurationRouteHandlerProps) {
const { search } = history.location;
export function CreateAgentConfigurationRouteHandler(
props: CreateAgentConfigurationRouteHandlerProps
) {
const { search } = props.history.location;

// Ignoring here because we specifically DO NOT want to add the query params to the global route handler
// @ts-expect-error
const { pageStep } = toQuery(search);

return (
<Settings>
<Settings {...props}>
<AgentConfigurationCreateEdit
pageStep={pageStep || 'choose-service-step'}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,22 @@

import { render, wait, waitForElement } from '@testing-library/react';
import { CoreStart } from 'kibana/public';
import { merge } from 'lodash';
import React, { FunctionComponent, ReactChild } from 'react';
import { MemoryRouter } from 'react-router-dom';
import { createKibanaReactContext } from 'src/plugins/kibana_react/public';
import { merge } from 'lodash';
import { ServiceOverview } from '..';
import { EuiThemeProvider } from '../../../../../../observability/public';
import { ApmPluginContextValue } from '../../../../context/ApmPluginContext';
import {
mockApmPluginContextValue,
MockApmPluginContextWrapper,
} from '../../../../context/ApmPluginContext/MockApmPluginContext';
import * as useAnomalyDetectionJobs from '../../../../hooks/useAnomalyDetectionJobs';
import { FETCH_STATUS } from '../../../../hooks/useFetcher';
import * as useLocalUIFilters from '../../../../hooks/useLocalUIFilters';
import * as urlParamsHooks from '../../../../hooks/useUrlParams';
import * as useAnomalyDetectionJobs from '../../../../hooks/useAnomalyDetectionJobs';
import { SessionStorageMock } from '../../../../services/__test__/SessionStorageMock';
import { EuiThemeProvider } from '../../../../../../../legacy/common/eui_styled_components';

const KibanaReactContext = createKibanaReactContext({
usageCollection: { reportUiStats: () => {} },
Expand All @@ -44,13 +45,15 @@ function wrapper({ children }: { children: ReactChild }) {
}) as unknown) as ApmPluginContextValue;

return (
<KibanaReactContext.Provider>
<MemoryRouter>
<EuiThemeProvider>
<MockApmPluginContextWrapper value={mockPluginContext as any}>
{children}
</MockApmPluginContextWrapper>
<KibanaReactContext.Provider>
<MockApmPluginContextWrapper value={mockPluginContext}>
{children}
</MockApmPluginContextWrapper>
</KibanaReactContext.Provider>
</EuiThemeProvider>
</KibanaReactContext.Provider>
</MemoryRouter>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ import {
import { i18n } from '@kbn/i18n';
import { isEmpty } from 'lodash';
import React, { useState } from 'react';
import { useLocation } from 'react-router-dom';
import { getOptionLabel } from '../../../../../../common/agent_configuration/all_option';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { AgentConfigurationListAPIResponse } from '../../../../../../server/lib/settings/agent_configuration/list_configurations';
import { useApmPluginContext } from '../../../../../hooks/useApmPluginContext';
import { FETCH_STATUS } from '../../../../../hooks/useFetcher';
import { useLocation } from '../../../../../hooks/useLocation';
import { useTheme } from '../../../../../hooks/useTheme';
import { px, units } from '../../../../../style/variables';
import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import {
import { i18n } from '@kbn/i18n';
import { isEmpty } from 'lodash';
import React from 'react';
import { useLocation } from 'react-router-dom';
import { useTrackPageview } from '../../../../../../observability/public';
import { useApmPluginContext } from '../../../../hooks/useApmPluginContext';
import { useFetcher } from '../../../../hooks/useFetcher';
import { useLocation } from '../../../../hooks/useLocation';
import { createAgentConfigurationHref } from '../../../shared/Links/apm/agentConfigurationLinks';
import { AgentConfigurationList } from './List';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,27 @@ import { render } from '@testing-library/react';
import { MockApmPluginContextWrapper } from '../../../context/ApmPluginContext/MockApmPluginContext';
import React, { ReactNode } from 'react';
import { Settings } from './';
import { LocationContext } from '../../../context/LocationContext';
import { createMemoryHistory } from 'history';
import { MemoryRouter, RouteComponentProps } from 'react-router-dom';

const { location } = createMemoryHistory();

function Wrapper({ children }: { children?: ReactNode }) {
const { location } = createMemoryHistory();
return (
<LocationContext.Provider value={location}>
<MemoryRouter>
<MockApmPluginContextWrapper>{children}</MockApmPluginContextWrapper>
</LocationContext.Provider>
</MemoryRouter>
);
}

describe('Settings', () => {
it('renders', async () => {
const routerProps = ({
location,
} as unknown) as RouteComponentProps<{}>;
expect(() =>
render(
<Settings>
<Settings {...routerProps}>
<div />
</Settings>,
{ wrapper: Wrapper }
Expand Down
24 changes: 14 additions & 10 deletions x-pack/plugins/apm/public/components/app/Settings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,29 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { ReactNode } from 'react';
import { i18n } from '@kbn/i18n';
import {
EuiButtonEmpty,
EuiPage,
EuiSideNav,
EuiPageSideBar,
EuiPageBody,
EuiPageSideBar,
EuiSideNav,
} from '@elastic/eui';
import { HomeLink } from '../../shared/Links/apm/HomeLink';
import { useLocation } from '../../../hooks/useLocation';
import { getAPMHref } from '../../shared/Links/apm/APMLink';
import { i18n } from '@kbn/i18n';
import React, { ReactNode } from 'react';
import { RouteComponentProps } from 'react-router-dom';
import { useApmPluginContext } from '../../../hooks/useApmPluginContext';
import { getAPMHref } from '../../shared/Links/apm/APMLink';
import { HomeLink } from '../../shared/Links/apm/HomeLink';

interface SettingsProps extends RouteComponentProps<{}> {
children: ReactNode;
}

export function Settings(props: { children: ReactNode }) {
export function Settings({ children, location }: SettingsProps) {
const { core } = useApmPluginContext();
const { basePath } = core.http;
const canAccessML = !!core.application.capabilities.ml?.canAccessML;
const { search, pathname } = useLocation();
const { search, pathname } = location;

function getSettingsHref(path: string) {
return getAPMHref({ basePath, path: `/settings${path}`, search });
Expand Down Expand Up @@ -94,7 +98,7 @@ export function Settings(props: { children: ReactNode }) {
]}
/>
</EuiPageSideBar>
<EuiPageBody>{props.children}</EuiPageBody>
<EuiPageBody>{children}</EuiPageBody>
</EuiPage>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { useTrackPageview } from '../../../../../observability/public';
import { Projection } from '../../../../common/projections';
import { ChartsSyncContextProvider } from '../../../context/ChartsSyncContext';
import { FETCH_STATUS } from '../../../hooks/useFetcher';
import { useLocation } from '../../../hooks/useLocation';
import { useTransactionCharts } from '../../../hooks/useTransactionCharts';
import { useTransactionDistribution } from '../../../hooks/useTransactionDistribution';
import { useUrlParams } from '../../../hooks/useUrlParams';
Expand All @@ -32,9 +31,11 @@ import { WaterfallWithSummmary } from './WaterfallWithSummmary';

type TransactionDetailsProps = RouteComponentProps<{ serviceName: string }>;

export function TransactionDetails({ match }: TransactionDetailsProps) {
export function TransactionDetails({
location,
match,
}: TransactionDetailsProps) {
const { serviceName } = match.params;
const location = useLocation();
const { urlParams } = useUrlParams();
const {
data: distributionData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { Location } from 'history';
import { first } from 'lodash';
import React, { useMemo } from 'react';
import { useLocation } from 'react-router-dom';
import { useTrackPageview } from '../../../../../observability/public';
import { Projection } from '../../../../common/projections';
import { ChartsSyncContextProvider } from '../../../context/ChartsSyncContext';
import { IUrlParams } from '../../../context/UrlParamsContext/types';
import { useLocation } from '../../../hooks/useLocation';
import { useServiceTransactionTypes } from '../../../hooks/useServiceTransactionTypes';
import { useTransactionCharts } from '../../../hooks/useTransactionCharts';
import { useTransactionList } from '../../../hooks/useTransactionList';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { createMemoryHistory } from 'history';
import React, { ReactNode } from 'react';
import { Router } from 'react-router-dom';
import { MockApmPluginContextWrapper } from '../../../../context/ApmPluginContext/MockApmPluginContext';
import { LocationProvider } from '../../../../context/LocationContext';
import {
UrlParamsContext,
useUiFilters,
Expand Down Expand Up @@ -46,11 +45,9 @@ function mountDatePicker(params?: IUrlParams) {
return mount(
<MockApmPluginContextWrapper>
<Router history={history}>
<LocationProvider>
<MockUrlParamsProvider params={params}>
<DatePicker />
</MockUrlParamsProvider>
</LocationProvider>
<MockUrlParamsProvider params={params}>
<DatePicker />
</MockUrlParamsProvider>
</Router>
</MockApmPluginContextWrapper>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ import { EuiSelect } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { History } from 'history';
import React from 'react';
import { useHistory, useParams } from 'react-router-dom';
import { useHistory, useLocation, useParams } from 'react-router-dom';
import {
ENVIRONMENT_ALL,
ENVIRONMENT_NOT_DEFINED,
} from '../../../../common/environment_filter_values';
import { useEnvironments } from '../../../hooks/useEnvironments';
import { useLocation } from '../../../hooks/useLocation';
import { useUrlParams } from '../../../hooks/useUrlParams';
import { fromQuery, toQuery } from '../Links/url_helpers';

Expand Down
Loading

0 comments on commit 3d91165

Please sign in to comment.