Skip to content

Commit

Permalink
[APM] APM & Observability plugin lint improvements (#72702) (#73266)
Browse files Browse the repository at this point in the history
* [APM] APM & Observability plugin lint improvements

This is a large change, but most of it is automatic `eslint --fix` changes.

* Apply the same ESLint ovderrides in APM and Observability plugins.
* Remove the `no-unused-vars` rule. We can turn on the TypeScript check if needed.
* Check both JS and TS files.
* Add a rule for react function component definitions
* Upgrade eslint-plugin-react to include that rule
  • Loading branch information
smith authored Jul 27, 2020
1 parent aab54c0 commit 74af87f
Show file tree
Hide file tree
Showing 140 changed files with 824 additions and 733 deletions.
21 changes: 12 additions & 9 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -775,19 +775,22 @@ module.exports = {
},

/**
* APM overrides
* APM and Observability overrides
*/
{
files: ['x-pack/plugins/apm/**/*.js'],
files: [
'x-pack/plugins/apm/**/*.{js,mjs,ts,tsx}',
'x-pack/plugins/observability/**/*.{js,mjs,ts,tsx}',
],
rules: {
'no-unused-vars': ['error', { ignoreRestSiblings: true }],
'no-console': ['warn', { allow: ['error'] }],
},
},
{
plugins: ['react-hooks'],
files: ['x-pack/plugins/apm/**/*.{ts,tsx}'],
rules: {
'react/function-component-definition': [
'warn',
{
namedComponents: 'function-declaration',
unnamedComponents: 'arrow-function',
},
],
'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks
'react-hooks/exhaustive-deps': ['error', { additionalHooks: '^useFetcher$' }],
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@
"eslint-plugin-node": "^11.0.0",
"eslint-plugin-prefer-object-spread": "^1.2.1",
"eslint-plugin-prettier": "^3.1.3",
"eslint-plugin-react": "^7.17.0",
"eslint-plugin-react": "^7.20.3",
"eslint-plugin-react-hooks": "^4.0.4",
"eslint-plugin-react-perf": "^3.2.3",
"exit-hook": "^2.2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ describe('Inspector Data View', () => {
});

it('should render loading state', () => {
const component = mountWithIntl(<DataView.component title="Test Data" adapters={adapters} />);
const component = mountWithIntl(<DataView.component title="Test Data" adapters={adapters} />); // eslint-disable-line react/jsx-pascal-case

expect(component).toMatchSnapshot();
});

it('should render empty state', async () => {
const component = mountWithIntl(<DataView.component title="Test Data" adapters={adapters} />);
const component = mountWithIntl(<DataView.component title="Test Data" adapters={adapters} />); // eslint-disable-line react/jsx-pascal-case
const tabularLoader = Promise.resolve(null);
adapters.data.setTabularLoader(() => tabularLoader);
await tabularLoader;
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/apm/e2e/cypress/plugins/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ module.exports = (on) => {

// readFileMaybe
on('task', {
// ESLint thinks this is a react component for some reason.
// eslint-disable-next-line react/function-component-definition
readFileMaybe(filename) {
if (fs.existsSync(filename)) {
return fs.readFileSync(filename, 'utf8');
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/apm/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const MainContainer = styled.div`
height: 100%;
`;

const App = () => {
function App() {
const [darkMode] = useUiSetting$<boolean>('theme:darkMode');

return (
Expand All @@ -59,9 +59,9 @@ const App = () => {
</MainContainer>
</ThemeProvider>
);
};
}

const ApmAppRoot = ({
function ApmAppRoot({
core,
deps,
routerHistory,
Expand All @@ -71,7 +71,7 @@ const ApmAppRoot = ({
deps: ApmPluginSetupDeps;
routerHistory: typeof history;
config: ConfigSchema;
}) => {
}) {
const i18nCore = core.i18n;
const plugins = deps;
const apmPluginContextValue = {
Expand Down Expand Up @@ -111,7 +111,7 @@ const ApmAppRoot = ({
</AlertsContextProvider>
</ApmPluginContext.Provider>
);
};
}

/**
* This module is rendered asynchronously in the Kibana platform.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ interface Props {
items: ErrorGroupListAPIResponse;
}

const ErrorGroupList: React.FC<Props> = (props) => {
function ErrorGroupList(props: Props) {
const { items } = props;
const { urlParams } = useUrlParams();
const { serviceName } = urlParams;
Expand Down Expand Up @@ -213,6 +213,6 @@ const ErrorGroupList: React.FC<Props> = (props) => {
sortItems={false}
/>
);
};
}

export { ErrorGroupList };
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { LocalUIFilters } from '../../shared/LocalUIFilters';
import { ErrorDistribution } from '../ErrorGroupDetails/Distribution';
import { ErrorGroupList } from './List';

const ErrorGroupOverview: React.FC = () => {
function ErrorGroupOverview() {
const { urlParams, uiFilters } = useUrlParams();

const { serviceName, start, end, sortField, sortDirection } = urlParams;
Expand Down Expand Up @@ -123,6 +123,6 @@ const ErrorGroupOverview: React.FC = () => {
</EuiFlexGroup>
</>
);
};
}

export { ErrorGroupOverview };
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ interface Props {
onBreakdownChange: (values: BreakdownItem[]) => void;
}

export const BreakdownFilter = ({
export function BreakdownFilter({
id,
selectedBreakdowns,
onBreakdownChange,
}: Props) => {
}: Props) {
const categories: BreakdownItem[] = [
{
name: 'Browser',
Expand Down Expand Up @@ -65,4 +65,4 @@ export const BreakdownFilter = ({
}}
/>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ export interface BreakdownGroupProps {
onChange: (values: BreakdownItem[]) => void;
}

export const BreakdownGroup = ({
export function BreakdownGroup({
id,
disabled,
onChange,
items,
}: BreakdownGroupProps) => {
}: BreakdownGroupProps) {
const [isOpen, setIsOpen] = useState<boolean>(false);

const [activeItems, setActiveItems] = useState<BreakdownItem[]>(items);
Expand Down Expand Up @@ -97,4 +97,4 @@ export const BreakdownGroup = ({
</EuiPopover>
</EuiFilterGroup>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, HTMLAttributes } from 'react';
import React, { HTMLAttributes, ReactNode } from 'react';
import {
EuiErrorBoundary,
EuiFlexGroup,
Expand All @@ -13,6 +13,7 @@ import {
} from '@elastic/eui';

interface Props {
children?: ReactNode;
/**
* Height for the chart
*/
Expand All @@ -27,12 +28,12 @@ interface Props {
'aria-label'?: string;
}

export const ChartWrapper: FC<Props> = ({
export function ChartWrapper({
loading = false,
height = '100%',
children,
...rest
}) => {
}: Props) {
const opacity = loading === true ? 0.3 : 1;

return (
Expand Down Expand Up @@ -60,4 +61,4 @@ export const ChartWrapper: FC<Props> = ({
)}
</EuiErrorBoundary>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function PageLoadDistChart({
onPercentileChange(minX, maxX);
};

// eslint-disable-next-line react/function-component-definition
const headerFormatter: TooltipValueFormatter = (tooltip: TooltipValue) => {
return (
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ interface Props {
}>;
}

export const VisitorBreakdownChart = ({ options }: Props) => {
export function VisitorBreakdownChart({ options }: Props) {
const [darkMode] = useUiSetting$<boolean>('theme:darkMode');

return (
Expand Down Expand Up @@ -93,4 +93,4 @@ export const VisitorBreakdownChart = ({ options }: Props) => {
</Chart>
</ChartWrapper>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, useEffect } from 'react';
import { CurveType, LineSeries, ScaleType } from '@elastic/charts';
import React, { useEffect } from 'react';
import { PercentileRange } from './index';
import { useBreakdowns } from './use_breakdowns';

Expand All @@ -16,12 +16,12 @@ interface Props {
onLoadingChange: (loading: boolean) => void;
}

export const BreakdownSeries: FC<Props> = ({
export function BreakdownSeries({
field,
value,
percentileRange,
onLoadingChange,
}) => {
}: Props) {
const { data, status } = useBreakdowns({
field,
value,
Expand All @@ -47,4 +47,4 @@ export const BreakdownSeries: FC<Props> = ({
))}
</>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const PercentileMarker = styled.span`
bottom: 205px;
`;

export const PercentileAnnotations = ({ percentiles }: Props) => {
export function PercentileAnnotations({ percentiles }: Props) {
const dataValues = generateAnnotationData(percentiles) ?? [];

const style: Partial<LineAnnotationStyle> = {
Expand All @@ -44,17 +44,17 @@ export const PercentileAnnotations = ({ percentiles }: Props) => {
},
};

const PercentileTooltip = ({
function PercentileTooltip({
annotation,
}: {
annotation: LineAnnotationDatum;
}) => {
}) {
return (
<span data-cy="percentileTooltipTitle">
{annotation.details}th Percentile
</span>
);
};
}

return (
<>
Expand Down Expand Up @@ -82,4 +82,4 @@ export const PercentileAnnotations = ({ percentiles }: Props) => {
))}
</>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface PercentileRange {
max?: number | null;
}

export const PageLoadDistribution = () => {
export function PageLoadDistribution() {
const { urlParams, uiFilters } = useUrlParams();

const { start, end, serviceName } = urlParams;
Expand Down Expand Up @@ -115,4 +115,4 @@ export const PageLoadDistribution = () => {
/>
</div>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { BreakdownFilter } from '../Breakdowns/BreakdownFilter';
import { PageViewsChart } from '../Charts/PageViewsChart';
import { BreakdownItem } from '../../../../../typings/ui_filters';

export const PageViewsTrend = () => {
export function PageViewsTrend() {
const { urlParams, uiFilters } = useUrlParams();

const { start, end, serviceName } = urlParams;
Expand Down Expand Up @@ -68,4 +68,4 @@ export const PageViewsTrend = () => {
<PageViewsChart data={data} loading={status !== 'success'} />
</div>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { PageLoadDistribution } from './PageLoadDistribution';
import { I18LABELS } from './translations';
import { VisitorBreakdown } from './VisitorBreakdown';

export const RumDashboard = () => {
export function RumDashboard() {
return (
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexItem>
Expand Down Expand Up @@ -54,4 +54,4 @@ export const RumDashboard = () => {
</EuiFlexItem>
</EuiFlexGroup>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@
*/

import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import React from 'react';
import React, { ReactNode } from 'react';
import { DatePicker } from '../../../shared/DatePicker';

export const RumHeader: React.FC = ({ children }) => (
<>
<EuiFlexGroup alignItems="center" gutterSize="s">
<EuiFlexItem>{children}</EuiFlexItem>
<EuiFlexItem grow={false}>
<DatePicker />
</EuiFlexItem>
</EuiFlexGroup>
</>
);
export function RumHeader({ children }: { children: ReactNode }) {
return (
<>
<EuiFlexGroup alignItems="center" gutterSize="s">
<EuiFlexItem>{children}</EuiFlexItem>
<EuiFlexItem grow={false}>
<DatePicker />
</EuiFlexItem>
</EuiFlexGroup>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { VisitorBreakdownLabel } from '../translations';
import { useFetcher } from '../../../../hooks/useFetcher';
import { useUrlParams } from '../../../../hooks/useUrlParams';

export const VisitorBreakdown = () => {
export function VisitorBreakdown() {
const { urlParams, uiFilters } = useUrlParams();

const { start, end, serviceName } = urlParams;
Expand Down Expand Up @@ -62,4 +62,4 @@ export const VisitorBreakdown = () => {
</EuiFlexGroup>
</>
);
};
}
Loading

0 comments on commit 74af87f

Please sign in to comment.