Skip to content

OU-1384: Remove some of the set-state-in-effect overrides #1006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PeterYurkovich wants to merge 1 commit into
base: main
Choose a base branch
from
+66 −37
Open

OU-1384: Remove some of the set-state-in-effect overrides #1006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions web/src/components/Incidents/AlertsChart/AlertsChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ import { MonitoringState } from '../../../store/store';
import { isEmpty } from 'lodash-es';
import { DataTestIDs } from '../../data-test';

export const DEFAULT_CHART_CONTAINER_HEIGHT = 300;
export const DEFAULT_CHART_HEIGHT = 250;

const AlertsChart = ({ theme }: { theme: 'light' | 'dark' }) => {

@coderabbitai coderabbitai Bot Jun 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Both modified incident chart components violate the same TSX component contract (default export + non-FC declaration). Apply one consistent component pattern across both files: define a Props type, declare const Component: FC<Props>, and use named exports.

  • web/src/components/Incidents/AlertsChart/AlertsChart.tsx#L48-L48: replace inline props typing with AlertsChartProps + FC<AlertsChartProps>.
  • web/src/components/Incidents/AlertsChart/AlertsChart.tsx#L280-L280: replace default export with named export.
  • web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx#L59-L75: replace inline props typing with IncidentsChartProps + FC<IncidentsChartProps>.
  • web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx#L299-L299: replace export default memo(IncidentsChart) with named export form (e.g., named memo export).

As per coding guidelines, “Use functional components with explicit type annotations and FC type for React components” and “Always use named exports for React components instead of default exports.”

📍 Affects 2 files
  • web/src/components/Incidents/AlertsChart/AlertsChart.tsx#L48-L48 (this comment)
  • web/src/components/Incidents/AlertsChart/AlertsChart.tsx#L280-L280
  • web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx#L59-L75
  • web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx#L299-L299
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/components/Incidents/AlertsChart/AlertsChart.tsx` at line 48, Both
chart components violate the TSX component contract by using inline prop typing
and default exports. Fix both files to follow the consistent pattern: (1) In
web/src/components/Incidents/AlertsChart/AlertsChart.tsx at line 48, replace the
inline props destructuring { theme: 'light' | 'dark' } with a named
AlertsChartProps type and declare the component as const AlertsChart:
FC<AlertsChartProps>; then at line 280, replace the default export with a named
export. (2) In web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx at
lines 59-75, replace the inline props typing with a named IncidentsChartProps
type and declare the component as const IncidentsChart: FC<IncidentsChartProps>;
then at line 299, replace the default export (including the memo wrapper) with a
named export form using memo.

Source: Coding guidelines

const dispatch = useDispatch();
const [chartContainerHeight, setChartContainerHeight] = useState<number>();
const [chartHeight, setChartHeight] = useState<number>();
const alertsData = useSelector(
(state: MonitoringState) => state.plugins.mcp.incidentsData.alertsData,
);
Expand All @@ -71,16 +72,34 @@ const AlertsChart = ({ theme }: { theme: 'light' | 'dark' }) => {
return generateAlertsDateArray(alertsData, currentTime);
}, [alertsData, currentTime]);

const chartData: AlertsChartBar[][] = useMemo(() => {
if (!Array.isArray(alertsData) || alertsData.length === 0) return [];
return alertsData.map((alert) => createAlertsChartBars(alert));
}, [alertsData]);
const {
chartData,
chartContainerHeight,
chartHeight,
}: { chartData: AlertsChartBar[][]; chartContainerHeight: number; chartHeight: number } =
useMemo(() => {
if (!Array.isArray(alertsData) || alertsData.length === 0) {
return {
chartData: [],
chartContainerHeight: DEFAULT_CHART_CONTAINER_HEIGHT,
chartHeight: DEFAULT_CHART_HEIGHT,
};
}
const chartData = alertsData.map((alert) => createAlertsChartBars(alert));
if (chartData.length < 5) {
return {
chartData,
chartContainerHeight: DEFAULT_CHART_CONTAINER_HEIGHT,
chartHeight: DEFAULT_CHART_HEIGHT,
};
}

useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setChartContainerHeight(chartData?.length < 5 ? 300 : chartData?.length * 55);
setChartHeight(chartData?.length < 5 ? 250 : chartData?.length * 55);
}, [chartData]);
return {
chartData,
chartContainerHeight: chartData.length * 55,
chartHeight: chartData.length * 55,
};
}, [alertsData]);

const selectedIncidentIsVisible = useMemo(() => {
return filteredData.some(
Expand All @@ -100,8 +119,6 @@ const AlertsChart = ({ theme }: { theme: 'light' | 'dark' }) => {
}
}, []);
useEffect(() => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const observer = getResizeObserver(containerRef.current, handleResize);
handleResize();
return () => observer();
Expand Down
34 changes: 23 additions & 11 deletions web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
import { dateTimeFormatter, timeFormatter } from '../../console/utils/datetime';
import { useTranslation } from 'react-i18next';
import { DataTestIDs } from '../../data-test';
import { DEFAULT_CHART_CONTAINER_HEIGHT, DEFAULT_CHART_HEIGHT } from '../AlertsChart/AlertsChart';

/**
* Processes component list: moves "Others" to end and limits display to 3 components
Expand Down Expand Up @@ -73,17 +74,21 @@ const IncidentsChart = ({
lastRefreshTime: number | null;
}) => {
const [isLoading, setIsLoading] = useState(true);
const [chartContainerHeight, setChartContainerHeight] = useState<number>();
const [chartHeight, setChartHeight] = useState<number>();
const dateValues = useMemo(
() => generateDateArray(chartDays, currentTime),
[chartDays, currentTime],
);

const { t, i18n } = useTranslation(process.env.I18N_NAMESPACE);

const chartData = useMemo(() => {
if (!Array.isArray(incidentsData) || incidentsData.length === 0) return [];
const { chartData, chartContainerHeight, chartHeight } = useMemo(() => {
if (!Array.isArray(incidentsData) || incidentsData.length === 0) {
return {
chartData: [],
chartContainerHeight: DEFAULT_CHART_CONTAINER_HEIGHT,
chartHeight: DEFAULT_CHART_HEIGHT,
};
}

const filteredIncidents = selectedGroupId
? incidentsData.filter((incident) => incident.group_id === selectedGroupId)
Expand All @@ -96,20 +101,27 @@ const IncidentsChart = ({
chartBars.sort((a, b) => a[0].x - b[0].x);

// Reassign consecutive x values to eliminate gaps between bars
return chartBars.map((bars, index) => bars.map((bar) => ({ ...bar, x: index + 1 })));
const chartData = chartBars.map((bars, index) => bars.map((bar) => ({ ...bar, x: index + 1 })));
if (chartData.length < 5) {
return {
chartData,
chartContainerHeight: DEFAULT_CHART_CONTAINER_HEIGHT,
chartHeight: DEFAULT_CHART_HEIGHT,
};
}

return {
chartData,
chartContainerHeight: chartData.length * 60,
chartHeight: chartData.length * 55,
};
}, [incidentsData, dateValues, selectedGroupId]);

useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setIsLoading(false);
}, [incidentsData]);

useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setChartContainerHeight(chartData?.length < 5 ? 300 : chartData?.length * 60);
setChartHeight(chartData?.length < 5 ? 250 : chartData?.length * 55);
}, [chartData]);

const [width, setWidth] = useState(0);
const containerRef = useRef<HTMLDivElement>(null);

Expand Down
26 changes: 13 additions & 13 deletions web/src/components/console/public/components/autocomplete.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ComponentProps, FC, SetStateAction, Dispatch, FormEvent } from 'react';
import { useState, useEffect } from 'react';
import { useMemo } from 'react';
import { css } from '@patternfly/react-styles';
import * as _ from 'lodash-es';

Expand Down Expand Up @@ -74,7 +74,6 @@ type AutocompleteInputProps = {
};

const AutocompleteInput: FC<AutocompleteInputProps> = (props) => {
const [suggestions, setSuggestions] = useState<string[]>();
const { visible, setVisible, ref } = useDocumentListener<HTMLDivElement>(suggestionBoxKeyHandler);
const {
textValue,
Expand Down Expand Up @@ -110,18 +109,19 @@ const AutocompleteInput: FC<AutocompleteInputProps> = (props) => {
setTextValue(input);
};

useEffect(() => {
if (textValue && visible && showSuggestions) {
const processed = labelParser(data, labelPath);
// User input without whitespace
const processedText = textValue.trim().replace(/\s*=\s*/, '=');
const maxSuggestions = suggestionCount ?? MAX_SUGGESTIONS;
const filtered = [...processed]
.filter((item) => fuzzyCaseInsensitive(processedText, item))
.slice(0, maxSuggestions);
// eslint-disable-next-line react-hooks/set-state-in-effect
setSuggestions(filtered);
const suggestions = useMemo(() => {
if (!textValue || !visible || !showSuggestions) {
return [];
}

const processed = labelParser(data, labelPath);
// User input without whitespace
const processedText = textValue.trim().replace(/\s*=\s*/, '=');
const maxSuggestions = suggestionCount ?? MAX_SUGGESTIONS;
const filtered = [...processed]
.filter((item) => fuzzyCaseInsensitive(processedText, item))
.slice(0, maxSuggestions);
return filtered;
}, [visible, textValue, showSuggestions, data, labelPath, suggestionCount]);

return (
Expand Down