diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 0e043c1..a9c8029 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,30 +1,3 @@ -### TASK-STATE-02 — `withRefresh` in `useJailList` Creates New Function References Every Render - -**Where found** -`frontend/src/hooks/useJailList.ts`. The `withRefresh` helper is defined inside the hook body without `useCallback`. It wraps an operation and calls `refresh()` after it completes. Because it is recreated every render, `startJail`, `stopJail`, `setIdle`, and `reloadJail` — which are all produced by `withRefresh(...)` — are also new references every render. Any child component receiving these as props will re-render even when nothing has changed. - -**Goal** -Wrap `withRefresh` in `useCallback` with `[refresh]` as dependency, or inline `useCallback` directly for each operation: -```ts -const startJail = useCallback(async (name: string) => { - await apiStartJail(name); - refresh(); -}, [refresh]); -``` -This ensures the function references are stable between renders. - -**Possible traps and issues** -- If `withRefresh` is used to produce many operations, it may be cleaner to keep `withRefresh` as a `useCallback` that captures `refresh`, rather than wrapping each call individually. -- After stabilising these references, `JailOverviewSection` or any child wrapped in `React.memo` will correctly skip re-renders when only unrelated state changes. - -**Docs changes needed** -None required. - -**Why this is needed** -Unstable function references passed as props defeat `React.memo` optimisations and cause unnecessary child re-renders. On the Jails page, which renders a list of potentially many jails, this compounds with every parent state change. - ---- - ### TASK-STATE-03 — `DashboardFilterBar` Has Dual State Source **Where found** diff --git a/frontend/src/components/BanTrendChart.tsx b/frontend/src/components/BanTrendChart.tsx index 5135cb2..cd467ef 100644 --- a/frontend/src/components/BanTrendChart.tsx +++ b/frontend/src/components/BanTrendChart.tsx @@ -5,7 +5,6 @@ */ import { memo, useMemo } from "react"; -import { useDashboardFilters } from "../providers/DashboardFilterProvider"; import { Area, AreaChart, @@ -53,11 +52,11 @@ const TICK_INTERVAL: Record = { /** Props for {@link BanTrendChart}. */ interface BanTrendChartProps { /** Time-range preset controlling the query window. */ - timeRange?: TimeRange; + timeRange: TimeRange; /** Origin filter controlling which bans are included. */ - origin?: BanOriginFilter; + origin: BanOriginFilter; /** Data source used for the chart. */ - source?: "fail2ban" | "archive"; + source: "fail2ban" | "archive"; } /** Internal chart data point shape. */ @@ -182,19 +181,13 @@ function TrendTooltip(props: TrendTooltipProps): React.JSX.Element | null { // Component // --------------------------------------------------------------------------- -/** - * Area chart showing ban counts over time. - * - * Fetches data via `useBanTrend` and handles loading, error, and empty states - * inline so the parent only needs to pass filter props. - * /** * Area chart showing ban counts over time. * * Fetches data via `useBanTrend` and delegates loading, error, and empty states * to `ChartStateWrapper`. * - * @param props - `timeRange` and `origin` filter props. + * @param props - `timeRange`, `origin`, and `source` filter props. */ export const BanTrendChart = memo(function BanTrendChart({ timeRange, @@ -202,15 +195,11 @@ export const BanTrendChart = memo(function BanTrendChart({ source, }: BanTrendChartProps): React.JSX.Element { const styles = useStyles(); - const context = useDashboardFilters(); - const effectiveTimeRange = timeRange ?? context.timeRange; - const effectiveOrigin = origin ?? context.originFilter; - const effectiveSource = source ?? context.source; const { colorMode } = useThemeMode(); - const { buckets, loading, error, reload } = useBanTrend(effectiveTimeRange, effectiveOrigin, effectiveSource); + const { buckets, loading, error, reload } = useBanTrend(timeRange, origin, source); const isEmpty = buckets.every((b) => b.count === 0); - const entries = buildEntries(buckets, effectiveTimeRange); + const entries = buildEntries(buckets, timeRange); const { primaryColour, axisColour, gridColour } = useMemo( () => { void colorMode; @@ -222,7 +211,7 @@ export const BanTrendChart = memo(function BanTrendChart({ }, [colorMode], ); - const tickInterval = TICK_INTERVAL[effectiveTimeRange]; + const tickInterval = TICK_INTERVAL[timeRange]; const tooltipContent = useMemo( () => { diff --git a/frontend/src/components/DashboardFilterBar.tsx b/frontend/src/components/DashboardFilterBar.tsx index 0fff5a8..7e13657 100644 --- a/frontend/src/components/DashboardFilterBar.tsx +++ b/frontend/src/components/DashboardFilterBar.tsx @@ -16,7 +16,6 @@ import { tokens, } from "@fluentui/react-components"; import { useCardStyles } from "../components/commonStyles"; -import { useDashboardFilters } from "../providers/DashboardFilterProvider"; import type { BanOriginFilter, TimeRange } from "../types/ban"; import { BAN_ORIGIN_FILTER_LABELS, @@ -30,13 +29,13 @@ import { /** Props for {@link DashboardFilterBar}. */ export interface DashboardFilterBarProps { /** Currently selected time-range preset. */ - timeRange?: TimeRange; + timeRange: TimeRange; /** Called when the user selects a different time-range preset. */ - onTimeRangeChange?: (value: TimeRange) => void; + onTimeRangeChange: (value: TimeRange) => void; /** Currently selected origin filter. */ - originFilter?: BanOriginFilter; + originFilter: BanOriginFilter; /** Called when the user selects a different origin filter. */ - onOriginFilterChange?: (value: BanOriginFilter) => void; + onOriginFilterChange: (value: BanOriginFilter) => void; /** Jail filter value (optional). */ jail?: string; /** Called when the jail filter text changes (optional). */ @@ -93,7 +92,8 @@ const useStyles = makeStyles({ /** * Renders a global filter bar with time-range and origin-filter toggle buttons. * - * The bar is fully controlled — it does not maintain its own state. + * The bar is fully controlled — all state is passed in via props and changes + * are communicated through callbacks. The component does not read from context. * * @param props - See {@link DashboardFilterBarProps}. */ @@ -107,15 +107,9 @@ export function DashboardFilterBar({ ip, onIpChange, }: DashboardFilterBarProps): React.JSX.Element { - const context = useDashboardFilters(); const styles = useStyles(); const cardStyles = useCardStyles(); - const currentTimeRange = timeRange ?? context.timeRange; - const handleTimeRangeChange = onTimeRangeChange ?? context.setTimeRange; - const currentOriginFilter = originFilter ?? context.originFilter; - const handleOriginFilterChange = onOriginFilterChange ?? context.setOriginFilter; - return (
{/* Time-range group */} @@ -128,10 +122,10 @@ export function DashboardFilterBar({ { - handleTimeRangeChange(r); + onTimeRangeChange(r); }} > {TIME_RANGE_LABELS[r]} @@ -155,10 +149,10 @@ export function DashboardFilterBar({ { - handleOriginFilterChange(f); + onOriginFilterChange(f); }} > {BAN_ORIGIN_FILTER_LABELS[f]} diff --git a/frontend/src/components/JailDistributionChart.tsx b/frontend/src/components/JailDistributionChart.tsx index 3892e39..77c542e 100644 --- a/frontend/src/components/JailDistributionChart.tsx +++ b/frontend/src/components/JailDistributionChart.tsx @@ -24,7 +24,6 @@ import { CHART_PALETTE, resolveFluentToken, } from "../utils/chartTheme"; -import { useDashboardFilters } from "../providers/DashboardFilterProvider"; import { useThemeMode } from "../providers/ThemeProvider"; import { ChartStateWrapper } from "./ChartStateWrapper"; import { ChartTooltip } from "./ChartTooltip"; @@ -51,9 +50,9 @@ const MIN_CHART_HEIGHT = 180; /** Props for {@link JailDistributionChart}. */ interface JailDistributionChartProps { /** Time-range preset controlling the query window. */ - timeRange?: TimeRange; + timeRange: TimeRange; /** Origin filter controlling which bans are included. */ - origin?: BanOriginFilter; + origin: BanOriginFilter; } /** Internal chart data point shape. */ @@ -136,11 +135,8 @@ export const JailDistributionChart = memo(function JailDistributionChart({ origin, }: JailDistributionChartProps): React.JSX.Element { const styles = useStyles(); - const context = useDashboardFilters(); - const effectiveTimeRange = timeRange ?? context.timeRange; - const effectiveOrigin = origin ?? context.originFilter; const { colorMode } = useThemeMode(); - const { jails, loading, error, reload } = useJailDistribution(effectiveTimeRange, effectiveOrigin); + const { jails, loading, error, reload } = useJailDistribution(timeRange, origin); const entries = buildEntries(jails); const chartHeight = Math.max(entries.length * BAR_HEIGHT_PX, MIN_CHART_HEIGHT); diff --git a/frontend/src/components/__tests__/BanTrendChart.test.tsx b/frontend/src/components/__tests__/BanTrendChart.test.tsx index 1a4bd95..09b0d68 100644 --- a/frontend/src/components/__tests__/BanTrendChart.test.tsx +++ b/frontend/src/components/__tests__/BanTrendChart.test.tsx @@ -55,7 +55,7 @@ beforeEach(() => { describe("BanTrendChart", () => { it("shows a spinner while loading", () => { mockHook({ loading: true }); - wrap(); + wrap(); expect(screen.getByRole("progressbar")).toBeInTheDocument(); }); @@ -63,7 +63,7 @@ describe("BanTrendChart", () => { const reload = vi.fn(); mockHook({ error: "Failed to fetch trend", reload }); const user = userEvent.setup(); - wrap(); + wrap(); expect(screen.getByText("Failed to fetch trend")).toBeInTheDocument(); await user.click(screen.getByRole("button", { name: /retry/i })); expect(reload).toHaveBeenCalledOnce(); @@ -75,7 +75,7 @@ describe("BanTrendChart", () => { { timestamp: "2024-01-01T01:00:00Z", count: 0 }, ]; mockHook({ buckets: emptyBuckets }); - wrap(); + wrap(); expect(screen.getByText("No bans in this time range.")).toBeInTheDocument(); }); @@ -85,13 +85,13 @@ describe("BanTrendChart", () => { { timestamp: "2024-01-01T01:00:00Z", count: 12 }, ]; mockHook({ buckets }); - wrap(); + wrap(); expect(screen.getByTestId("area-chart")).toBeInTheDocument(); }); it("memoizes token resolution across rerenders", () => { const spy = vi.spyOn(chartTheme, "resolveFluentToken"); - const props = { timeRange: "24h" as const, origin: "all" as const }; + const props = { timeRange: "24h" as const, origin: "all" as const, source: "fail2ban" as const }; mockHook({ buckets: [{ timestamp: "2024-01-01T00:00:00Z", count: 5 }] }); const { rerender } = wrap(); diff --git a/frontend/src/pages/DashboardPage.tsx b/frontend/src/pages/DashboardPage.tsx index 6b46fac..6fbe4f9 100644 --- a/frontend/src/pages/DashboardPage.tsx +++ b/frontend/src/pages/DashboardPage.tsx @@ -73,7 +73,7 @@ const useStyles = makeStyles({ */ function DashboardPageContent(): React.JSX.Element { const styles = useStyles(); - const { timeRange, originFilter, source } = useDashboardFilters(); + const { timeRange, originFilter, source, setTimeRange, setOriginFilter } = useDashboardFilters(); const { countries, countryNames, loading: countryLoading, error: countryError, reload: reloadCountry } = useDashboardCountryData(timeRange, originFilter, source); @@ -91,7 +91,12 @@ function DashboardPageContent(): React.JSX.Element { {/* Global filter bar */} {/* ------------------------------------------------------------------ */}
- +
{/* ------------------------------------------------------------------ */} @@ -104,7 +109,11 @@ function DashboardPageContent(): React.JSX.Element {
- +
@@ -155,7 +164,11 @@ function DashboardPageContent(): React.JSX.Element { {/* Ban table */}
- +
diff --git a/frontend/src/providers/__tests__/DashboardFilterProvider.test.tsx b/frontend/src/providers/__tests__/DashboardFilterProvider.test.tsx index e6ddc1c..66ce509 100644 --- a/frontend/src/providers/__tests__/DashboardFilterProvider.test.tsx +++ b/frontend/src/providers/__tests__/DashboardFilterProvider.test.tsx @@ -4,9 +4,19 @@ import userEvent from "@testing-library/user-event"; import { DashboardFilterBar } from "../../components/DashboardFilterBar"; import { DashboardFilterProvider, useDashboardFilters } from "../DashboardFilterProvider"; -function DashboardFilterSummary(): React.JSX.Element { - const { timeRange, originFilter } = useDashboardFilters(); - return
{`${timeRange}-${originFilter}`}
; +function DashboardFilterContent(): React.JSX.Element { + const { timeRange, originFilter, setTimeRange, setOriginFilter } = useDashboardFilters(); + return ( + <> + +
{`${timeRange}-${originFilter}`}
+ + ); } describe("DashboardFilterProvider", () => { @@ -15,8 +25,7 @@ describe("DashboardFilterProvider", () => { render( - - + , );