Skip to content

Commit

Permalink
Consolidate react-hooks/exhaustive-deps lint rules for O11y (#184865)
Browse files Browse the repository at this point in the history
Use one react-hooks/exhaustive-deps across our Obs plugins, for
consistency reasons.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 8, 2024
1 parent 229b3e8 commit 5381dd7
Show file tree
Hide file tree
Showing 52 changed files with 238 additions and 167 deletions.
27 changes: 12 additions & 15 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -908,10 +908,6 @@ module.exports = {
},
],
'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks
'react-hooks/exhaustive-deps': [
'error',
{ additionalHooks: '^(useFetcher|useProgressiveFetcher|useBreadcrumb)$' },
],
},
},
{
Expand All @@ -931,6 +927,18 @@ module.exports = {
],
},
},
{
files: ['x-pack/plugins/observability_solution/**/*.{ts,tsx}'],
rules: {
'react-hooks/exhaustive-deps': [
'error',
{
additionalHooks:
'^(useAbortableAsync|useMemoWithAbortSignal|useFetcher|useProgressiveFetcher|useBreadcrumb|useAsync|useTimeRangeAsync|useAutoAbortedHttpClient)$',
},
],
},
},
{
files: [
'x-pack/plugins/aiops/**/*.tsx',
Expand Down Expand Up @@ -964,17 +972,6 @@ module.exports = {
],
},
},
// Profiling
{
files: ['x-pack/plugins/observability_solution/profiling/**/*.{js,mjs,ts,tsx}'],
rules: {
'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks
'react-hooks/exhaustive-deps': [
'error',
{ additionalHooks: '^(useAsync|useTimeRangeAsync|useAutoAbortedHttpClient)$' },
],
},
},
{
// disable imports from legacy uptime plugin
files: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,22 @@ import { useDatePickerContext } from '../hooks/use_date_picker';
import { extractRangeFromChartFilterEvent } from './chart_utils';
import { useLoadingStateContext } from '../hooks/use_loading_state';

export type ChartProps = LensConfig &
Pick<LensChartProps, 'overrides'> & {
id: string;
queryField: string;
dateRange: TimeRange;
assetId: string;
};
export type ChartProps = Pick<LensChartProps, 'overrides'> & {
id: string;
queryField: string;
dateRange: TimeRange;
assetId: string;
lensAttributes: LensConfig;
};

export const Chart = ({ id, queryField, overrides, dateRange, assetId, ...props }: ChartProps) => {
export const Chart = ({
id,
queryField,
overrides,
dateRange,
assetId,
lensAttributes,
}: ChartProps) => {
const { setDateRange } = useDatePickerContext();
const { searchSessionId } = useLoadingStateContext();
const {
Expand All @@ -34,7 +41,7 @@ export const Chart = ({ id, queryField, overrides, dateRange, assetId, ...props

const { value: filters = [] } = useAsync(async () => {
const resolvedDataView = await resolveDataView({
dataViewId: (props.dataset as LensDataviewDataset)?.index,
dataViewId: (lensAttributes.dataset as LensDataviewDataset)?.index,
dataViewsService: dataViews,
});

Expand All @@ -45,7 +52,7 @@ export const Chart = ({ id, queryField, overrides, dateRange, assetId, ...props
dataView: resolvedDataView.dataViewReference,
}),
];
}, [assetId, dataViews, props.dataset, queryField]);
}, [assetId, dataViews, lensAttributes.dataset, queryField]);

const handleBrushEnd = useCallback(
({ range, preventDefault }: BrushEndArgs) => {
Expand Down Expand Up @@ -75,13 +82,13 @@ export const Chart = ({ id, queryField, overrides, dateRange, assetId, ...props

return (
<LensChart
{...props}
id={`infraAssetDetailsMetricChart${id}`}
borderRadius="m"
dateRange={dateRange}
height={METRIC_CHART_HEIGHT}
searchSessionId={searchSessionId}
filters={filters}
lensAttributes={lensAttributes}
overrides={overrides}
onBrushEnd={handleBrushEnd}
onFilter={handleFilter}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ export const DockerCharts = React.forwardRef<HTMLDivElement, Props>(
<ChartsGrid columns={2}>
{charts.map((chart) => (
<Chart
id={chart.id}
key={chart.id}
{...chart}
lensAttributes={chart}
assetId={assetId}
dateRange={dateRange}
queryField={findInventoryFields('container').id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const HostCharts = React.forwardRef<HTMLDivElement, Props>(
const { charts } = useHostCharts({
metric,
dataViewId: dataView?.id,
options: { overview },
overview,
});

return (
Expand Down Expand Up @@ -91,10 +91,11 @@ export const HostCharts = React.forwardRef<HTMLDivElement, Props>(
<ChartsGrid columns={2}>
{charts.map((chart) => (
<Chart
id={chart.id}
key={chart.id}
{...chart}
assetId={assetId}
dateRange={dateRange}
lensAttributes={chart}
queryField={findInventoryFields('host').id}
/>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const KubernetesNodeCharts = React.forwardRef<HTMLDivElement, MetricsChar
({ assetId, dataView, dateRange, onShowAll, overview }, ref) => {
const { charts } = useKubernetesCharts({
dataViewId: dataView?.id,
options: { overview },
overview,
});

const hasIntegration = useIntegrationCheck({ dependsOn: INTEGRATIONS.kubernetesNode });
Expand Down Expand Up @@ -63,10 +63,11 @@ export const KubernetesNodeCharts = React.forwardRef<HTMLDivElement, MetricsChar
<ChartsGrid columns={2}>
{charts.map((chart) => (
<Chart
id={chart.id}
key={chart.id}
{...chart}
assetId={assetId}
dateRange={dateRange}
lensAttributes={chart}
queryField={findInventoryFields('host').id}
/>
))}
Expand Down Expand Up @@ -127,8 +128,9 @@ export const KubernetesContainerCharts = React.forwardRef<
<ChartsGrid columns={2}>
{charts.map((chart) => (
<Chart
id={chart.id}
key={chart.id}
{...chart}
lensAttributes={chart}
assetId={assetId}
dateRange={dateRange}
queryField={findInventoryFields('container').id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const ContainerKpiCharts = ({
dateRange,
dataView,
filters,
options,
query,
searchSessionId,
loading = false,
Expand All @@ -50,7 +49,6 @@ export const ContainerKpiCharts = ({
dateRange={dateRange}
dataView={dataView}
filters={filters}
options={options}
query={query}
searchSessionId={searchSessionId}
loading={loading}
Expand All @@ -60,7 +58,6 @@ export const ContainerKpiCharts = ({
dateRange={dateRange}
dataView={dataView}
filters={filters}
options={options}
query={query}
searchSessionId={searchSessionId}
loading={loading}
Expand All @@ -72,18 +69,14 @@ const DockerKpiCharts = ({
dateRange,
dataView,
filters,
options,
query,
searchSessionId,
loading = false,
}: ContainerKpiChartsProps) => {
const { euiTheme } = useEuiTheme();
const charts = useDockerContainerKpiCharts({
dataViewId: dataView?.id,
options: {
getSubtitle: options?.getSubtitle,
seriesColor: euiTheme.colors.lightestShade,
},
seriesColor: euiTheme.colors.lightestShade,
});

return (
Expand Down Expand Up @@ -116,10 +109,7 @@ const KubernetesKpiCharts = ({
const { euiTheme } = useEuiTheme();
const charts = useK8sContainerKpiCharts({
dataViewId: dataView?.id,
options: {
getSubtitle: options?.getSubtitle,
seriesColor: euiTheme.colors.lightestShade,
},
seriesColor: euiTheme.colors.lightestShade,
});

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,24 @@ export interface HostKpiChartsProps {
query?: Query;
filters?: Filter[];
searchSessionId?: string;
options?: {
getSubtitle?: (formulaValue: string) => string;
};
getSubtitle?: (formulaValue: string) => string;
loading?: boolean;
}

export const HostKpiCharts = ({
dateRange,
dataView,
filters,
options,
getSubtitle,
query,
searchSessionId,
loading = false,
}: HostKpiChartsProps) => {
const { euiTheme } = useEuiTheme();
const charts = useHostKpiCharts({
dataViewId: dataView?.id,
options: {
getSubtitle: options?.getSubtitle,
seriesColor: euiTheme.colors.lightestShade,
},
getSubtitle,
seriesColor: euiTheme.colors.lightestShade,
});

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ export const Kpi = ({

return (
<LensChart
{...chartProps}
id={`infraAssetDetailsKPI${id}`}
dateRange={dateRange}
height={KPI_CHART_HEIGHT}
filters={filters}
lensAttributes={chartProps}
query={query}
loading={loading}
toolTip={tooltipContent}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const useDockerContainerPageViewMetricsCharts = ({
}),
};
});
}, [metricsDataViewId]);
}, [metricsDataViewId, metric]);

return { charts, error };
};
Expand Down Expand Up @@ -80,7 +80,7 @@ export const useK8sContainerPageViewMetricsCharts = ({
}),
};
});
}, [metricsDataViewId]);
}, [metricsDataViewId, metric]);

return { charts, error };
};
Expand All @@ -101,10 +101,10 @@ const getK8sContainerCharts = async (metric: ContainerMetricTypes) => {

export const useDockerContainerKpiCharts = ({
dataViewId,
options,
seriesColor,
}: {
dataViewId?: string;
options?: { seriesColor: string; getSubtitle?: (formulaValue: string) => string };
seriesColor?: string;
}) => {
const { value: charts = [] } = useAsync(async () => {
const model = findInventoryModel('container');
Expand All @@ -113,27 +113,26 @@ export const useDockerContainerKpiCharts = ({
return [cpu.metric.dockerContainerCpuUsage, memory.metric.dockerContainerMemoryUsage].map(
(chart) => ({
...chart,
seriesColor: options?.seriesColor,
seriesColor,
decimals: 1,
subtitle: getSubtitle(options, chart),
...(dataViewId && {
dataset: {
index: dataViewId,
},
}),
})
);
}, [dataViewId, options?.seriesColor, options?.getSubtitle]);
}, [dataViewId, seriesColor]);

return charts;
};

export const useK8sContainerKpiCharts = ({
dataViewId,
options,
seriesColor,
}: {
dataViewId?: string;
options?: { seriesColor: string; getSubtitle?: (formulaValue: string) => string };
seriesColor?: string;
}) => {
const { value: charts = [] } = useAsync(async () => {
const model = findInventoryModel('container');
Expand All @@ -142,26 +141,21 @@ export const useK8sContainerKpiCharts = ({
return [cpu.metric.k8sContainerCpuUsage, memory.metric.k8sContainerMemoryUsage].map(
(chart) => ({
...chart,
seriesColor: options?.seriesColor,
seriesColor,
decimals: 1,
subtitle: getSubtitle(options, chart),
subtitle: getSubtitle(chart),
...(dataViewId && {
dataset: {
index: dataViewId,
},
}),
})
);
}, [dataViewId, options?.seriesColor, options?.getSubtitle]);
}, [dataViewId, seriesColor]);

return charts;
};

function getSubtitle(
options: { getSubtitle?: ((formulaValue: string) => string) | undefined } | undefined,
chart: { value: string }
) {
return options?.getSubtitle
? options?.getSubtitle(chart.value)
: getSubtitleFromFormula(chart.value);
function getSubtitle(chart: { value: string }) {
return getSubtitleFromFormula(chart.value);
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const useDataViews = () => {

const { value: logsDataView, loading: logsDataViewLoading } = useAsync(
() => getLogsDataView(logViewReference),
[logViewReference]
[logViewReference, getLogsDataView]
);

return {
Expand Down
Loading

0 comments on commit 5381dd7

Please sign in to comment.