Skip to content

Commit

Permalink
[GEN-2199] UI types linter (#2163)
Browse files Browse the repository at this point in the history
This pull request includes several changes to improve type safety and
code clarity in the `frontend/webapp` directory. The most important
changes include adding type annotations, refactoring object
construction, and updating TypeScript configuration for stricter type
checking.

Improvements to type safety:

*
[`frontend/webapp/components/notification/toast-list.tsx`](diffhunk://#diff-c6ead7587d1e5c52295c921dde2a1a5b9b06977ab3900c7dbf475c47cc1c664cL39-R39):
Added type annotations to the `onClose` function parameter to ensure it
is an object with a boolean `asSeen` property.
*
[`frontend/webapp/containers/main/destinations/destination-drawer/build-card.ts`](diffhunk://#diff-ce1c4d3c06218dab699d84dac4690cc261b1209716055609b79e97ab930e6862L7-R7):
Updated the `filter` method to use a type-safe key lookup on
`ExportedSignals`.
*
[`frontend/webapp/containers/main/destinations/destination-modal/choose-destination-body/destinations-list/index.tsx`](diffhunk://#diff-0fc3893b30022c16fec91cb64d38fa36de0dc4e8eabbbbb7a3b94701b45a4badL58-R58):
Added type annotations for `SupportedSignals` in the `monitors` filter
method.

Code clarity improvements:

*
[`frontend/webapp/containers/main/destinations/destination-drawer/build-drawer-item.ts`](diffhunk://#diff-0a1f42815db27054a5330d48db69f3c86c12948854b1f4c2db7611dbc3088ffeL7-R12):
Refactored the construction of `fieldsStringified` by first building an
object and then converting it to a JSON string.

TypeScript configuration update:

*
[`frontend/webapp/tsconfig.json`](diffhunk://#diff-5414aabe0cae671212009d85c7bff3f80ae87557c3cfeb2ebf363e39a5dfb19fL31-R32):
Enabled the `noImplicitAny` compiler option to enforce explicit type
annotations, improving overall type safety.

---------

Co-authored-by: alonkeyval <alonbraymok007@gmail.com>
Co-authored-by: Ben Elferink <ben.elferink@icloud.com>
  • Loading branch information
3 people authored Jan 8, 2025
1 parent 26d1cbe commit 8e9e2a7
Show file tree
Hide file tree
Showing 30 changed files with 137 additions and 71 deletions.
2 changes: 1 addition & 1 deletion frontend/webapp/components/notification/toast-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const Toast: React.FC<Notification> = (props) => {
const { markAsDismissed, markAsSeen } = useNotificationStore();
const clickNotif = useClickNotif();

const onClose = ({ asSeen }) => {
const onClose = ({ asSeen }: { asSeen: boolean }) => {
markAsDismissed(id);
if (asSeen) markAsSeen(id);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { ActualDestination, DestinationDetailsResponse, ExportedSignals } f

const buildMonitorsList = (exportedSignals: ExportedSignals): string =>
Object.keys(exportedSignals)
.filter((key) => exportedSignals[key])
.filter((key) => exportedSignals[key as keyof ExportedSignals])
.join(', ');

const buildCard = (destination: ActualDestination, destinationTypeDetails?: DestinationDetailsResponse['destinationTypeDetails']) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ const buildDrawerItem = (id: string, formData: DestinationInput, drawerItem: Act
const { name, exportedSignals, fields } = formData;
const { destinationType, conditions } = drawerItem || {};

let fieldsStringified: string | Record<string, any> = {};
fields.forEach(({ key, value }) => (fieldsStringified[key] = value));
fieldsStringified = JSON.stringify(fieldsStringified);
const fieldsObject: Record<string, any> = {};
fields.forEach(({ key, value }) => {
fieldsObject[key] = value;
});

const fieldsStringified = JSON.stringify(fieldsObject);

return {
id,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from 'react';
import styled from 'styled-components';
import { DestinationTypeItem } from '@/types';
import { IDestinationListItem } from '@/hooks';
import { capitalizeFirstLetter } from '@/utils';
import { DataTab, NoDataFound, SectionTitle } from '@/reuseable-components';
import type { SupportedSignals, DestinationTypeItem } from '@/types';
import { PotentialDestinationsList } from './potential-destinations-list';
import { DataTab, NoDataFound, SectionTitle } from '@/reuseable-components';

const Container = styled.div`
display: flex;
Expand Down Expand Up @@ -55,7 +55,7 @@ export const DestinationsList: React.FC<DestinationsListProps> = ({ items, setSe
title={item.displayName}
iconSrc={item.imageUrl}
hoverText='Select'
monitors={Object.keys(item.supportedSignals).filter((signal) => item.supportedSignals[signal].supported)}
monitors={Object.keys(item.supportedSignals).filter((signal) => item.supportedSignals[signal as keyof SupportedSignals].supported)}
monitorsWithLabels
onClick={() => setSelectedItems(item)}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { OdigosLogo } from '@/assets';
import styled from 'styled-components';
import type { DestinationTypeItem } from '@/types';
import type { DestinationTypeItem, SupportedSignals } from '@/types';
import { usePotentialDestinations } from '@/hooks';
import { DataTab, SectionTitle, SkeletonLoader } from '@/reuseable-components';

Expand Down Expand Up @@ -38,7 +38,7 @@ export const PotentialDestinationsList: React.FC<Props> = ({ setSelectedItems })
title={item.displayName}
iconSrc={item.imageUrl}
hoverText='Select'
monitors={Object.keys(item.supportedSignals).filter((signal) => item.supportedSignals[signal].supported)}
monitors={Object.keys(item.supportedSignals).filter((signal: keyof SupportedSignals) => item.supportedSignals[signal].supported)}
monitorsWithLabels
onClick={() => setSelectedItems(item)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { SearchIcon } from '@/assets';
import styled from 'styled-components';
import { SignalUppercase } from '@/utils';
import { useDestinationTypes } from '@/hooks';
import type { DestinationTypeItem } from '@/types';
import { DestinationsList } from './destinations-list';
import type { DestinationTypeItem, SupportedSignals } from '@/types';
import { Divider, Dropdown, Input, MonitoringCheckboxes, SectionTitle } from '@/reuseable-components';

interface Props {
Expand Down Expand Up @@ -51,7 +51,7 @@ export const ChooseDestinationBody: React.FC<Props> = ({ onSelect, hidden }) =>
const filteredItems = category.items.filter((item) => {
const matchesSearch = !search || item.displayName.toLowerCase().includes(search.toLowerCase());
const matchesCategory = selectedCategory.id === 'all' || selectedCategory.id === category.name;
const matchesMonitor = selectedMonitors.some((monitor) => item.supportedSignals[monitor.toLowerCase()]?.supported);
const matchesMonitor = selectedMonitors.some((monitor) => item.supportedSignals[monitor.toLowerCase() as keyof SupportedSignals]?.supported);

return matchesSearch && matchesCategory && matchesMonitor;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { TrashIcon } from '@/assets';
import { useAppStore } from '@/store';
import styled from 'styled-components';
import { DeleteWarning } from '@/components';
import { OVERVIEW_ENTITY_TYPES } from '@/types';
import { K8sActualSource, OVERVIEW_ENTITY_TYPES } from '@/types';
import { useSourceCRUD, useTransition } from '@/hooks';
import { Badge, Button, Divider, Text } from '@/reuseable-components';

Expand Down Expand Up @@ -50,10 +50,10 @@ export const MultiSourceControl = () => {
};

const onDelete = () => {
const payload = {};
const payload: Record<string, K8sActualSource[]> = {};

Object.entries(configuredSources).forEach(([namespace, sources]) => {
payload[namespace] = sources.map(({ name, kind }) => ({ name, kind, selected: false }));
Object.entries(configuredSources).forEach(([namespace, sources]: [string, K8sActualSource[]]) => {
payload[namespace] = sources.map((source) => ({ ...source, selected: false }));
});

persistSources(payload, {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import theme from '@/styles/theme';
import styled from 'styled-components';
import { OVERVIEW_ENTITY_TYPES, WorkloadId } from '@/types';
import { AbsoluteContainer } from '../../styled';
import { getEntityIcon, getEntityLabel } from '@/utils';
import { getEntityIcon, getEntityItemId, getEntityLabel } from '@/utils';
import { buildSearchResults, type Category } from './builder';
import { Divider, SelectionButton, Text } from '@/reuseable-components';
import { useActionCRUD, useDestinationCRUD, useInstrumentationRuleCRUD, useNodeDataFlowHandlers, useSourceCRUD } from '@/hooks';
Expand Down Expand Up @@ -71,7 +71,7 @@ export const SearchResults = ({ searchText, onClose }: Props) => {
icon={getEntityIcon(category as OVERVIEW_ENTITY_TYPES)}
label={getEntityLabel(item, category as OVERVIEW_ENTITY_TYPES, { extended: true })}
onClick={() => {
const id = item.id || item.ruleId || { kind: item.kind, name: item.name, namespace: item.namespace };
const id = getEntityItemId(item);
// @ts-ignore
handleNodeClick(null, { data: { type: category, id } });
onClose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const SearchWrapper = styled.div`
`;

export const SourceControls: React.FC<Props> = ({ selectedSources, searchText, setSearchText, showSelectedOnly, setShowSelectedOnly }) => {
const selectedAppsCount = Object.values(selectedSources).reduce((prev, curr) => prev + curr.length, 0);
const selectedAppsCount = Object.values(selectedSources).reduce((prev, curr) => prev + curr.filter((s) => s.selected).length, 0);

return (
<>
Expand Down
14 changes: 10 additions & 4 deletions frontend/webapp/hooks/actions/useActionFormData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,33 @@ const INITIAL: ActionInput = {
details: '',
};

type Errors = {
type?: string;
signals?: string;
details?: string;
};

export function useActionFormData() {
const { addNotification } = useNotificationStore();
const { formData, formErrors, handleFormChange, handleErrorChange, resetFormData } = useGenericForm<ActionInput>(INITIAL);

const validateForm = (params?: { withAlert?: boolean; alertTitle?: string }) => {
const errors = {};
const errors: Errors = {};
let ok = true;

Object.entries(formData).forEach(([k, v]) => {
switch (k) {
case 'type':
case 'signals':
if (isEmpty(v)) errors[k] = FORM_ALERTS.FIELD_IS_REQUIRED;
if (isEmpty(v)) errors[k as keyof Errors] = FORM_ALERTS.FIELD_IS_REQUIRED;
break;

case 'details':
if (isEmpty(v)) errors[k] = FORM_ALERTS.FIELD_IS_REQUIRED;
if (isEmpty(v)) errors[k as keyof Errors] = FORM_ALERTS.FIELD_IS_REQUIRED;
if (formData.type === ActionsType.LATENCY_SAMPLER) {
(safeJsonParse(v as string, { endpoints_filters: [] }) as LatencySamplerSpec).endpoints_filters.forEach((endpoint) => {
if (endpoint.http_route.charAt(0) !== '/') {
errors[k] = FORM_ALERTS.LATENCY_HTTP_ROUTE;
errors[k as keyof Errors] = FORM_ALERTS.LATENCY_HTTP_ROUTE;
}
});
}
Expand Down
41 changes: 26 additions & 15 deletions frontend/webapp/hooks/common/useGenericForm.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useState } from 'react';

export const useGenericForm = <Form = Record<string, any>>(initialFormData: Form) => {
function copyInitial() {
export const useGenericForm = <Form extends Record<string, any>>(initialFormData: Form) => {
function copyInitial(): Form {
// this is to avoid reference issues with the initial form data,
// when an object has arrays or objects as part of it's values, a simple spread operator won't work, the children would act as references,
// so we use JSON.parse(JSON.stringify()) to create a deep copy of the object without affecting the original
Expand All @@ -11,27 +11,38 @@ export const useGenericForm = <Form = Record<string, any>>(initialFormData: Form
const [formData, setFormData] = useState<Form>(copyInitial());
const [formErrors, setFormErrors] = useState<Partial<Record<keyof Form, string>>>({});

const handleFormChange = (key?: keyof typeof formData, val?: any, obj?: typeof formData) => {
if (!!key) {
const handleFormChange = (key?: keyof Form | string, val?: any, obj?: Form) => {
if (key) {
// this is for cases where the form contains objects such as "exportedSignals",
// the object's child is targeted with a ".dot" for example: "exportedSignals.logs"

const [parentKey, childKey] = (key as string).split('.');

if (!!childKey) {
setFormData((prev) => ({ ...prev, [parentKey]: { ...prev[parentKey], [childKey]: val } }));
} else {
setFormData((prev) => ({ ...prev, [parentKey]: val }));
}
} else if (!!obj) {
const [parentKey, childKey] = key.toString().split('.');

setFormData((prev) => {
if (childKey) {
return {
...prev,
[parentKey]: {
...(prev[parentKey] as Record<string, any>),
[childKey]: val,
},
};
} else {
return {
...prev,
[parentKey]: val,
};
}
});
} else if (obj) {
setFormData({ ...obj });
}
};

const handleErrorChange = (key?: keyof typeof formErrors, val?: string, obj?: typeof formErrors) => {
if (!!key) {
const handleErrorChange = (key?: keyof Form | string, val?: string, obj?: Partial<Record<keyof Form, string>>) => {
if (key) {
setFormErrors((prev) => ({ ...prev, [key]: val }));
} else if (!!obj) {
} else if (obj) {
setFormErrors({ ...obj });
}
};
Expand Down
4 changes: 2 additions & 2 deletions frontend/webapp/hooks/compute-platform/useComputePlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useNotificationStore } from '@/store';
import { GET_COMPUTE_PLATFORM } from '@/graphql';
import { useFilterStore } from '@/store/useFilterStore';
import { ACTION, deriveTypeFromRule, safeJsonParse } from '@/utils';
import { NOTIFICATION_TYPE, type ActionItem, type ComputePlatform, type ComputePlatformMapped } from '@/types';
import { NOTIFICATION_TYPE, SupportedSignals, type ActionItem, type ComputePlatform, type ComputePlatformMapped } from '@/types';

type UseComputePlatformHook = {
data?: ComputePlatformMapped;
Expand Down Expand Up @@ -84,7 +84,7 @@ export const useComputePlatform = (): UseComputePlatformHook => {
let actions = [...mappedCP.computePlatform.actions];

if (!!filters.monitors.length) {
destinations = destinations.filter((destination) => !!filters.monitors.find((metric) => destination.exportedSignals[metric.id]));
destinations = destinations.filter((destination) => !!filters.monitors.find((metric) => destination.exportedSignals[metric.id as keyof SupportedSignals]));
actions = actions.filter((action) => !!filters.monitors.find((metric) => action.spec.signals.find((str) => str.toLowerCase() === metric.id)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export function useDestinationFormData(params?: { destinationType?: string; supp
}, [supportedSignals]);

const validateForm = (params?: { withAlert?: boolean; alertTitle?: string }) => {
const errors = {};
const errors: Record<DynamicField['name'], string> = {};
let ok = true;

dynamicFields.forEach(({ name, value, required }) => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/webapp/hooks/destinations/useDestinationTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function useDestinationTypes() {
setDestinations(
data.destinationTypes.categories.map((category) => ({
name: category.name,
description: CATEGORIES_DESCRIPTION[category.name],
description: CATEGORIES_DESCRIPTION[category.name as keyof typeof CATEGORIES_DESCRIPTION],
items: category.items,
})),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function useInstrumentationRuleFormData() {
const { formData, formErrors, handleFormChange, handleErrorChange, resetFormData } = useGenericForm<InstrumentationRuleInput>(INITIAL);

const validateForm = (params?: { withAlert?: boolean; alertTitle?: string }) => {
const errors = {};
const errors: Partial<Record<keyof InstrumentationRuleInput, string>> = {};
let ok = true;

Object.entries(formData).forEach(([k, v]) => {
Expand Down
7 changes: 5 additions & 2 deletions frontend/webapp/hooks/notification/useClickNotif.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useSourceCRUD } from '../sources';
import { useActionCRUD } from '../actions';
import { getIdFromSseTarget } from '@/utils';
import { useDestinationCRUD } from '../destinations';
import { type Notification, OVERVIEW_ENTITY_TYPES } from '@/types';
import { type Notification, OVERVIEW_ENTITY_TYPES, WorkloadId } from '@/types';
import { useInstrumentationRuleCRUD } from '../instrumentation-rules';
import { DrawerItem, useDrawerStore, useNotificationStore } from '@/store';

Expand Down Expand Up @@ -33,7 +33,10 @@ export const useClickNotif = () => {
case 'InstrumentationInstance':
drawerItem['type'] = OVERVIEW_ENTITY_TYPES.SOURCE;
drawerItem['id'] = getIdFromSseTarget(target, OVERVIEW_ENTITY_TYPES.SOURCE);
drawerItem['item'] = sources.find((item) => item.kind === drawerItem['id']?.['kind'] && item.name === drawerItem['id']?.['name'] && item.namespace === drawerItem['id']?.['namespace']);
drawerItem['item'] = sources.find(
(item) => item.kind === (drawerItem['id'] as WorkloadId).kind && item.name === (drawerItem['id'] as WorkloadId).name && item.namespace === (drawerItem['id'] as WorkloadId).namespace,
);

break;

case OVERVIEW_ENTITY_TYPES.ACTION:
Expand Down
43 changes: 30 additions & 13 deletions frontend/webapp/hooks/overview/useNodeDataFlowHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,36 @@ export const useNodeDataFlowHandlers = () => {
data: { id, type },
} = object;

const entities = {
sources,
actions,
destinations,
rules: instrumentationRules,
};

if (type === OVERVIEW_ENTITY_TYPES.SOURCE) {
const { kind, name, namespace } = id as WorkloadId;

const selectedDrawerItem = entities['sources'].find((item) => item.kind === kind && item.name === name && item.namespace === namespace);
const selectedDrawerItem = sources.find((item) => item.kind === kind && item.name === name && item.namespace === namespace);
if (!selectedDrawerItem) {
console.warn('Selected item not found', { id, sourcesCount: sources.length });
return;
}

setSelectedItem({
id,
type,
item: selectedDrawerItem,
});
} else if (type === OVERVIEW_ENTITY_TYPES.ACTION) {
const selectedDrawerItem = actions.find((item) => item.id === id);
if (!selectedDrawerItem) {
console.warn('Selected item not found', { id, actionsCount: actions.length });
return;
}

setSelectedItem({
id,
type,
item: selectedDrawerItem,
});
} else if (type === OVERVIEW_ENTITY_TYPES.DESTINATION) {
const selectedDrawerItem = destinations.find((item) => item.id === id);
if (!selectedDrawerItem) {
console.warn('Selected item not found', { id, [`${type}sCount`]: entities[`${type}s`].length });
console.warn('Selected item not found', { id, destinationsCount: destinations.length });
return;
}

Expand All @@ -52,16 +69,16 @@ export const useNodeDataFlowHandlers = () => {
type,
item: selectedDrawerItem,
});
} else if ([OVERVIEW_ENTITY_TYPES.RULE, OVERVIEW_ENTITY_TYPES.ACTION, OVERVIEW_ENTITY_TYPES.DESTINATION].includes(type as OVERVIEW_ENTITY_TYPES)) {
const selectedDrawerItem = entities[`${type}s`].find((item) => id && [item.id, item.ruleId].includes(id));
} else if (type === OVERVIEW_ENTITY_TYPES.RULE) {
const selectedDrawerItem = instrumentationRules.find((item) => item.ruleId === id);
if (!selectedDrawerItem) {
console.warn('Selected item not found', { id, [`${type}sCount`]: entities[`${type}s`].length });
console.warn('Selected item not found', { id, rulesCount: instrumentationRules.length });
return;
}

setSelectedItem({
id,
type: type as OVERVIEW_ENTITY_TYPES,
type,
item: selectedDrawerItem,
});
} else if (type === OVERVIEW_NODE_TYPES.ADD_RULE) {
Expand Down
2 changes: 1 addition & 1 deletion frontend/webapp/hooks/sources/useSourceCRUD.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const useSourceCRUD = (params?: Params) => {
const namespace = req?.variables?.namespace;
const count = req?.variables?.sources.length;

req?.variables?.sources.forEach(({ name, kind, selected }) => {
req?.variables?.sources.forEach(({ name, kind, selected }: { name: string; kind: string; selected: boolean }) => {
if (!selected) removeNotifications(getSseTargetFromId({ namespace, name, kind }, OVERVIEW_ENTITY_TYPES.SOURCE));
});

Expand Down
Loading

0 comments on commit 8e9e2a7

Please sign in to comment.