Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 12 additions & 3 deletions .oxlintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,18 @@
"react/jsx-no-useless-fragment": "warn",
"react/no-unescaped-entities": "error",
"react/exhaustive-deps": "error",
"react/react-compiler": "warn",
"react-doctor/no-fetch-in-effect": "warn",
"react-doctor/no-derived-state": "warn",
"react/unsupported-syntax": "error",
"react/no-deriving-state-in-effects": "error",
"react/invariant": "error",
"react/rule-suppression": "error",
"react/syntax": "error",
"react/todo": "error",
"react/capitalized-calls": "error",
"react/exhaustive-effect-dependencies": "error",
"react/hooks": "error",
"react/memo-dependencies": "error",
"react-doctor/no-fetch-in-effect": "error",
"react-doctor/no-derived-state": "error",
"react/jsx-curly-brace-presence": [
"error",
{ "props": "never", "children": "never", "propElementValues": "always" }
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ Then in [`perses`](https://github.com/perses/perses) repository:

Run `npm run lint` for the regular Oxlint checks, including the React Doctor rules configured in `.oxlintrc.json`. Run
`npm run doctor` for the full React Doctor project scan. Pull requests and pushes to `main` also run the scan in GitHub
Actions.
Actions. React Compiler rules and the configured React Doctor rules are enforced as errors; resolve their diagnostics
before merging.

### Working with Snapshots

Expand Down
2 changes: 1 addition & 1 deletion STYLEGUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ touch without expanding the task into unrelated cleanup.
for dynamic values passed across memoized boundaries. Do not add memoization blindly or omit dependencies.
- Memoize React context values when they contain objects or functions whose identity would otherwise change each render.
- Avoid array-index keys when a stable domain identifier exists.
- Treat `react/react-compiler` and `react-perf` diagnostics as design feedback. Fix new warnings rather than disabling
- Treat React Compiler and `react-perf` diagnostics as design feedback. Fix errors and new warnings rather than disabling
the rule or increasing the repository warning ceiling.

## Components, state, and accessibility
Expand Down
84 changes: 84 additions & 0 deletions alertmanager/src/components/ColumnsEditor.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright The Perses Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import { fireEvent, render, screen } from '@testing-library/react';
import type { ReactElement } from 'react';
import { useCallback, useState } from 'react';

import type { BaseColumnDefinition } from './ColumnsEditor';
import { ColumnsEditor } from './ColumnsEditor';

const SORT_MODES = { alphabetical: 'Alphabetical' };
const INITIAL_COLUMNS = [{ name: 'first' }, { name: 'second' }];
const getName = (column: BaseColumnDefinition): string => column.name;
const renderName = (column: BaseColumnDefinition): ReactElement => (
<input aria-label="Column name" defaultValue={column.name} />
);
const onUpdate = vi.fn();

function Editor(): ReactElement {
const [columns, setColumns] = useState(INITIAL_COLUMNS);
const onAdd = useCallback((): void => setColumns((previous) => [...previous, { name: 'added' }]), []);
const onRemove = useCallback(
(index: number): void => setColumns((previous) => previous.filter((_, i) => i !== index)),
[],
);
const move = useCallback((index: number, offset: number): void => {
setColumns((previous) => {
const next = [...previous];
const [column] = next.splice(index, 1);
if (column) next.splice(index + offset, 0, column);
return next;
});
}, []);
const onMoveUp = useCallback((index: number): void => move(index, -1), [move]);
const onMoveDown = useCallback((index: number): void => move(index, 1), [move]);

return (
<ColumnsEditor
columns={columns}
description="Columns"
sortModeLabels={SORT_MODES}
defaultSortMode="alphabetical"
getDisplayName={getName}
getHeaderPlaceholder={getName}
onAdd={onAdd}
onRemove={onRemove}
onUpdate={onUpdate}
onMoveUp={onMoveUp}
onMoveDown={onMoveDown}
renderNameField={renderName}
/>
);
}

describe('ColumnsEditor', () => {
it('keeps local edits with their column when moving, removing, and adding columns', () => {
render(<Editor />);
const firstInput = screen.getAllByRole('textbox', { name: 'Column name' })[0]!;
fireEvent.change(firstInput, { target: { value: 'draft' } });

fireEvent.click(screen.getAllByRole('button', { name: 'Move column down' })[0]!);
expect(screen.getAllByRole('textbox', { name: 'Column name' })[1]).toBe(firstInput);
expect(firstInput).toHaveValue('draft');

fireEvent.click(screen.getAllByRole('button', { name: 'Move column up' })[1]!);
expect(screen.getAllByRole('textbox', { name: 'Column name' })[0]).toBe(firstInput);

fireEvent.click(screen.getAllByRole('button', { name: 'Remove column' })[1]!);
fireEvent.click(screen.getByRole('button', { name: 'Add column' }));
expect(screen.getAllByRole('textbox', { name: 'Column name' })[0]).toBe(firstInput);
expect(firstInput).toHaveValue('draft');
expect(screen.getAllByRole('textbox', { name: 'Column name' })[1]).toHaveValue('added');
});
});
47 changes: 29 additions & 18 deletions alertmanager/src/components/ColumnsEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import ArrowUpIcon from 'mdi-material-ui/ArrowUp';
import DeleteIcon from 'mdi-material-ui/Delete';
import PlusIcon from 'mdi-material-ui/Plus';
import type { ReactElement } from 'react';
import { useCallback, useRef } from 'react';
import { useCallback, useState } from 'react';

export interface BaseColumnDefinition {
name: string;
Expand Down Expand Up @@ -202,22 +202,27 @@ export function ColumnsEditor<C extends BaseColumnDefinition>(props: ColumnsEdit
renderNameField,
} = props;

const idCounterRef = useRef(0);
const idsRef = useRef<number[]>([]);

while (idsRef.current.length < columns.length) {
idsRef.current.push(idCounterRef.current++);
const [columnIds, setColumnIds] = useState(() => ({
ids: columns.map((_, index) => index),
nextId: columns.length,
}));
if (columnIds.ids.length !== columns.length) {
const ids = columnIds.ids.slice(0, columns.length);
let nextId = columnIds.nextId;
while (ids.length < columns.length) {
ids.push(nextId++);
}
setColumnIds({ ids, nextId });
}
idsRef.current.length = columns.length;

const handleAdd = useCallback((): void => {
idsRef.current.push(idCounterRef.current++);
setColumnIds(({ ids, nextId }) => ({ ids: [...ids, nextId], nextId: nextId + 1 }));
onAdd();
}, [onAdd]);

const handleRemove = useCallback(
(index: number): void => {
idsRef.current.splice(index, 1);
setColumnIds((previous) => ({ ...previous, ids: previous.ids.filter((_, i) => i !== index) }));
onRemove(index);
},
[onRemove],
Expand All @@ -226,23 +231,29 @@ export function ColumnsEditor<C extends BaseColumnDefinition>(props: ColumnsEdit
const handleMoveUp = useCallback(
(index: number): void => {
if (index <= 0) return;
const ids = idsRef.current;
const id = ids.splice(index, 1)[0]!;
ids.splice(index - 1, 0, id);
setColumnIds((previous) => {
const ids = [...previous.ids];
const id = ids.splice(index, 1)[0]!;
ids.splice(index - 1, 0, id);
return { ...previous, ids };
});
onMoveUp(index);
},
[onMoveUp],
);

const handleMoveDown = useCallback(
(index: number): void => {
const ids = idsRef.current;
if (index >= ids.length - 1) return;
const id = ids.splice(index, 1)[0]!;
ids.splice(index + 1, 0, id);
if (index >= columns.length - 1) return;
setColumnIds((previous) => {
const ids = [...previous.ids];
const id = ids.splice(index, 1)[0]!;
ids.splice(index + 1, 0, id);
return { ...previous, ids };
});
onMoveDown(index);
},
[onMoveDown],
[onMoveDown, columns.length],
);

return (
Expand All @@ -252,7 +263,7 @@ export function ColumnsEditor<C extends BaseColumnDefinition>(props: ColumnsEdit
{description}
</Typography>
{columns.map((column, index) => (
<Box key={idsRef.current[index]}>
<Box key={columnIds.ids[index]}>
{index > 0 && <Divider sx={{ mb: 2 }} />}
<ColumnEntry
column={column}
Expand Down
42 changes: 42 additions & 0 deletions alertmanager/src/components/LazyTextField.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright The Perses Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import { fireEvent, render, screen } from '@testing-library/react';

import { LazyTextField } from './LazyTextField';

describe('LazyTextField', () => {
it('preserves an uncommitted draft across renders and commits it on blur', () => {
const onCommit = vi.fn();
const { rerender } = render(<LazyTextField label="Matcher" value="initial" onCommit={onCommit} />);
fireEvent.change(screen.getByRole('textbox'), { target: { value: 'draft' } });
rerender(<LazyTextField label="Matcher" value="initial" onCommit={onCommit} />);

expect(screen.getByRole('textbox')).toHaveValue('draft');
expect(onCommit).not.toHaveBeenCalled();
fireEvent.blur(screen.getByRole('textbox'));
expect(onCommit).toHaveBeenCalledWith('draft');
});

it('resets a draft when the committed value changes or is cleared', () => {
const onCommit = vi.fn();
const { rerender } = render(<LazyTextField label="Matcher" value="initial" onCommit={onCommit} />);
fireEvent.change(screen.getByRole('textbox'), { target: { value: 'draft' } });
rerender(<LazyTextField label="Matcher" value="updated" onCommit={onCommit} />);
expect(screen.getByRole('textbox')).toHaveValue('updated');

rerender(<LazyTextField label="Matcher" onCommit={onCommit} />);
expect(screen.getByRole('textbox')).toHaveValue('');
expect(onCommit).not.toHaveBeenCalled();
});
});
8 changes: 5 additions & 3 deletions alertmanager/src/components/LazyTextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import { TextField } from '@mui/material';
import type { ChangeEvent, ReactElement } from 'react';
import { useCallback, useEffect, useState } from 'react';
import { useCallback, useState } from 'react';

export interface LazyTextFieldProps {
label: string;
Expand All @@ -29,9 +29,11 @@ export function LazyTextField(props: LazyTextFieldProps): ReactElement {
const { value, onCommit, ...textFieldProps } = props;
const [draftValue, setDraftValue] = useState(value ?? '');

useEffect(() => {
const [previousValue, setPreviousValue] = useState(value);
if (value !== previousValue) {
setPreviousValue(value);
setDraftValue(value ?? '');
}, [value]);
}

const handleChange = useCallback((event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>): void => {
setDraftValue(event.target.value);
Expand Down
8 changes: 4 additions & 4 deletions alertmanager/src/explore/AlertManagerSilencesExplorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type { DatasourceSelector, QueryDefinition } from '@perses-dev/spec';
import { useQueryClient } from '@tanstack/react-query';
import BellOffIcon from 'mdi-material-ui/BellOff';
import type { ReactElement } from 'react';
import { useCallback, useMemo, useRef, useState } from 'react';
import { useCallback, useMemo, useState } from 'react';

import { SilenceForm } from '../components/SilenceForm';
import type { AlertManagerClient, PostableSilence } from '../model';
Expand Down Expand Up @@ -50,10 +50,10 @@ function CreateSilenceButton({ queries }: { queries: QueryDefinition[] }): React
const { successSnackbar, exceptionSnackbar } = useSnackbar();

const [open, setOpen] = useState(false);
const formKeyRef = useRef(0);
const [formKey, setFormKey] = useState(0);

const handleOpen = useCallback(() => {
formKeyRef.current++;
setFormKey((previous) => previous + 1);
setOpen(true);
}, []);

Expand All @@ -77,7 +77,7 @@ function CreateSilenceButton({ queries }: { queries: QueryDefinition[] }): React
<Button variant="contained" startIcon={<BellOffIcon />} onClick={handleOpen} size="small">
Create Silence
</Button>
<SilenceForm key={formKeyRef.current} open={open} onClose={() => setOpen(false)} onSubmit={handleSubmit} />
<SilenceForm key={formKey} open={open} onClose={() => setOpen(false)} onSubmit={handleSubmit} />
</>
);
}
Expand Down
25 changes: 13 additions & 12 deletions alertmanager/src/plugins/alert-table/AlertTablePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import MagnifyIcon from 'mdi-material-ui/Magnify';
import UnfoldLessHorizontalIcon from 'mdi-material-ui/UnfoldLessHorizontal';
import UnfoldMoreHorizontalIcon from 'mdi-material-ui/UnfoldMoreHorizontal';
import type { ReactElement } from 'react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useCallback, useMemo, useState } from 'react';

import { SilenceForm } from '../../components/SilenceForm';
import { StatusBadge } from '../../components/StatusBadge';
Expand Down Expand Up @@ -345,9 +345,9 @@ export function AlertTablePanel({ spec, queryResults, contentDimensions }: Alert
const queryClient = useQueryClient();

const [silenceTarget, setSilenceTarget] = useState<Alert | null>(null);
const silenceKeyRef = useRef(0);
const [silenceKey, setSilenceKey] = useState(0);
const handleSetSilenceTarget = useCallback((alert: Alert) => {
silenceKeyRef.current++;
setSilenceKey((previous) => previous + 1);
setSilenceTarget(alert);
}, []);
const [search, setSearch] = useState('');
Expand Down Expand Up @@ -414,9 +414,11 @@ export function AlertTablePanel({ spec, queryResults, contentDimensions }: Alert

const [groupBy, setGroupBy] = useState<string[]>(resolvedDefaultGroupBy);

useEffect(() => {
const [previousDefaultGroupBy, setPreviousDefaultGroupBy] = useState(resolvedDefaultGroupBy);
if (resolvedDefaultGroupBy !== previousDefaultGroupBy) {
setPreviousDefaultGroupBy(resolvedDefaultGroupBy);
setGroupBy(resolvedDefaultGroupBy);
}, [resolvedDefaultGroupBy]);
}

const effectiveActions = useMemo<AlertAction[]>(
() => spec.allowedActions ?? ALL_ALERT_ACTIONS,
Expand Down Expand Up @@ -517,18 +519,17 @@ export function AlertTablePanel({ spec, queryResults, contentDimensions }: Alert
return result;
}, [alerts, groupBy, allTrackedKeys, sortState]);

const prevGroupKeysRef = useRef<string>('');
useEffect(() => {
const currentKeys = groups.map((g) => g.key).join('\0');
if (currentKeys === prevGroupKeysRef.current) return;
prevGroupKeysRef.current = currentKeys;
const currentKeys = groups.map((g) => g.key).join('\0');
const [previousGroupKeys, setPreviousGroupKeys] = useState('');
if (currentKeys !== previousGroupKeys) {
setPreviousGroupKeys(currentKeys);

if (groups.length === 1) {
setExpandedGroups(new Set(groups.map((g) => g.key)));
} else {
setExpandedGroups(new Set());
}
}, [groups]);
}

const handleToggleGroup = useCallback((key: string) => {
setExpandedGroups((prev) => {
Expand Down Expand Up @@ -699,7 +700,7 @@ export function AlertTablePanel({ spec, queryResults, contentDimensions }: Alert
</Table>
</TableContainer>
<SilenceForm
key={silenceKeyRef.current}
key={silenceKey}
open={!!silenceTarget}
onClose={() => setSilenceTarget(null)}
onSubmit={handleSilenceSubmit}
Expand Down
Loading
Loading