From f8614b124f0be3aba84c5dfb454f4efcf8d0a284 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Thu, 8 Oct 2026 21:50:13 +0000 Subject: [PATCH] refactor(frontend): enforce exact optional property contracts --- CONTRIBUTING.md | 4 +- frontend/src/apexcharts.ts | 118 ++++++++++++++------------ frontend/src/tomselect.ts | 3 +- tests/end-to-end/playwright.config.ts | 2 +- tsconfig.json | 3 +- 5 files changed, 71 insertions(+), 59 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 252c86e9d..9badf8dd2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,7 +64,9 @@ cargo clippy --all-targets --all-features -- -D warnings ### Frontend We use Biome for linting and formatting of the frontend code, and TypeScript -to typecheck it. +to typecheck it. TypeScript checks optional properties exactly: omit an unset option so library defaults +remain intact; use an explicit `| undefined` only when the property contract supports a present, +undefined value. ```bash npm install # once diff --git a/frontend/src/apexcharts.ts b/frontend/src/apexcharts.ts index 8a2e5d377..2ac226cc8 100644 --- a/frontend/src/apexcharts.ts +++ b/frontend/src/apexcharts.ts @@ -1,4 +1,10 @@ -import type { ApexChart, ApexOptions } from "apexcharts"; +import type { + ApexChart, + ApexOptions, + ApexTooltip, + ApexXAxis, + ApexYAxis, +} from "apexcharts"; import ApexCharts from "apexcharts"; import { type ReferenceLine, read_chart_data } from "./chart_data.ts"; import { @@ -225,12 +231,13 @@ const sqlpage_chart = (() => { y = y.map((value) => new Date(value ?? 0).getTime()); else x = new Date(x ?? 0); } + const fillColor = named_color(color); point_series.data.push({ x, y, - z, - link, - fillColor: named_color(color), + ...(z === undefined ? {} : { z }), + ...(link === undefined ? {} : { link }), + ...(fillColor === undefined ? {} : { fillColor }), }); } @@ -297,6 +304,53 @@ const sqlpage_chart = (() => { ? ({ seriesIndex }: PointIndex) => points[seriesIndex]?.link : ({ seriesIndex, dataPointIndex }: PointIndex) => aligned_series[seriesIndex]?.data[dataPointIndex]?.link; + // Unlike absent options, these two explicit undefined values disable + // ApexCharts defaults. Its types do not describe that runtime contract. + const xaxis: Omit & { type: ApexXAxis["type"] } = { + type: xaxis_type, + tooltip: { enabled: false }, + labels: { datetimeUTC: false }, + }; + if (data.xmin !== undefined) xaxis.min = data.xmin; + if (data.xmax !== undefined) xaxis.max = data.xmax; + if (axis_titles.x !== undefined) xaxis.title = { text: axis_titles.x }; + // Numeric axes count intervals; category and time axes use tickAmount + // as a target for label density. Zero leaves the library's default. + if (data.xticks) xaxis.tickAmount = data.xticks; + + const yaxis: ApexYAxis = { logarithmic: data.logarithmic }; + if (data.ymin !== undefined) yaxis.min = data.ymin; + if (data.ymax !== undefined) yaxis.max = data.ymax; + if (data.ystep !== undefined) yaxis.stepSize = data.ystep; + if (data.yticks !== undefined) yaxis.tickAmount = data.yticks; + if (axis_titles.y !== undefined) yaxis.title = { text: axis_titles.y }; + + const tooltip: Omit & { + custom: ApexTooltip["custom"]; + } = { + fillSeriesColor: false, + interactive: has_point_links, + // A rangeBar's default custom tooltip would replace our linked title. + custom: + chart_type === "bubble" || chart_type === "scatter" + ? axisTooltip(aligned_series, axis_titles, formatValue) + : undefined, + ...(has_point_links && text_x_values + ? { + x: { + formatter: (value, args) => + linkTooltipValue(value, args && pointLink(args)), + }, + } + : {}), + y: { + formatter: (value, args) => + linkTooltipValue( + formatValue(value), + args?.seriesIndex !== undefined ? pointLink(args) : undefined, + ), + }, + }; const options: ApexOptions = { annotations: { [`${value_axis}axis`]: reference_lines( @@ -315,7 +369,7 @@ const sqlpage_chart = (() => { chart: { // The query may name any type ApexCharts draws, not only the ones // the component documents. - type: chart_type as ApexChart["type"], + type: chart_type as NonNullable, fontFamily: "inherit", background: "transparent", parentHeightOffset: 0, @@ -364,33 +418,8 @@ const sqlpage_chart = (() => { lineCap: "round", curve: "smooth", }, - xaxis: { - tooltip: { - enabled: false, - }, - min: data.xmin, - max: data.xmax, - title: { - text: axis_titles.x, - }, - type: xaxis_type, - labels: { - datetimeUTC: false, - }, - // Numeric axes count intervals; category and time axes use tickAmount - // as a target for label density. - tickAmount: data.xticks || undefined, - }, - yaxis: { - logarithmic: data.logarithmic, - min: data.ymin, - max: data.ymax, - stepSize: data.ystep, - tickAmount: data.yticks, - title: { - text: axis_titles.y, - }, - }, + xaxis: xaxis as ApexXAxis, + yaxis, markers: { size: data.marker ?? 0, strokeWidth: 0, @@ -398,28 +427,7 @@ const sqlpage_chart = (() => { sizeOffset: 5, }, }, - tooltip: { - fillSeriesColor: false, - interactive: has_point_links, - custom: - chart_type === "bubble" || chart_type === "scatter" - ? axisTooltip(aligned_series, axis_titles, formatValue) - : undefined, - x: { - formatter: - has_point_links && text_x_values - ? (value, args) => - linkTooltipValue(value, args && pointLink(args)) - : undefined, - }, - y: { - formatter: (value, args) => - linkTooltipValue( - formatValue(value), - args?.seriesIndex !== undefined ? pointLink(args) : undefined, - ), - }, - }, + tooltip: tooltip as ApexTooltip, plotOptions: { bar: { horizontal: data.horizontal || chart_type === "rangeBar", @@ -429,7 +437,7 @@ const sqlpage_chart = (() => { }, colors, // ApexCharts draws a numeric series name but declares only a string. - series: series as ApexOptions["series"], + series: series as NonNullable, }; if (is_pie) options.labels = pie_labels; const chart = new ApexCharts(chartContainer, options) as RenderedChart; diff --git a/frontend/src/tomselect.ts b/frontend/src/tomselect.ts index 21b01bc16..b082eff5d 100644 --- a/frontend/src/tomselect.ts +++ b/frontend/src/tomselect.ts @@ -22,8 +22,9 @@ function sqlpage_select_dropdown_individual(s: HTMLSelectElement) { // This is a workaround to fix that const is_focused = s === document.activeElement; + const load = sqlpage_load_options_source(s.dataset.options_source); const tom = new TomSelect(s, { - load: sqlpage_load_options_source(s.dataset.options_source), + ...(load ? { load } : {}), valueField: "value", labelField: "label", searchField: ["label"], diff --git a/tests/end-to-end/playwright.config.ts b/tests/end-to-end/playwright.config.ts index f8c9b4075..46f11176b 100644 --- a/tests/end-to-end/playwright.config.ts +++ b/tests/end-to-end/playwright.config.ts @@ -26,7 +26,7 @@ export default defineConfig({ fullyParallel: true, forbidOnly: !!process.env.CI, retries: process.env.CI ? 2 : 0, - workers: process.env.CI ? 1 : undefined, + ...(process.env.CI ? { workers: 1 } : {}), reporter: "html", use: { trace: "on-first-retry", diff --git a/tsconfig.json b/tsconfig.json index aa8793a19..a5016c45a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -17,7 +17,8 @@ "allowUnreachableCode": false, "allowUnusedLabels": false, "types": [], - "noImplicitReturns": true + "noImplicitReturns": true, + "exactOptionalPropertyTypes": true }, "include": ["frontend/src"] }