From 90e71937eaedecabf7f45d7a5a998d44bff231ba Mon Sep 17 00:00:00 2001 From: 81reap Date: Wed, 30 Sep 2026 00:13:40 -0400 Subject: [PATCH] refactor(frontend) :: enable `noImplicitAny` `noImplicitAny` is now enabled. Most of the remaining implicit workarounds rooted from `JSON.parse(data_element.textContent)`. This found 3 bugs due to `as ChartPoint` :: 1. `ChartPoint.x` excluded null, `z` was a number, and `y` excluded the `string[]` two `value` columns return. 2. `ChartSeries.name` was a string, and the test pinning a numeric series name has passed all along. 3. `merged_x_values` compares x values that may be null, which JavaScript reads as zero. Most remaining work arounds we should step back and think big about if we need to upstreaming patches along with the fixes. --- frontend/src/apexcharts.ts | 322 +++++++++--------- frontend/src/chart_data.ts | 109 ++++++ frontend/src/chart_series.ts | 21 +- frontend/src/globals.d.ts | 10 +- tests/end-to-end/fixture.ts | 2 +- .../end-to-end/fixtures/chart/labeled-pie.sql | 2 + .../fixtures/chart/labeled-range-bar.sql | 3 + tests/end-to-end/fixtures/chart/test.ts | 81 ++--- tests/end-to-end/fixtures/map/test.ts | 4 +- tests/end-to-end/globals.d.ts | 7 - tests/end-to-end/tsconfig.json | 2 +- tests/js/chart_series.spec.ts | 18 +- tsconfig.json | 1 - 13 files changed, 344 insertions(+), 238 deletions(-) create mode 100644 frontend/src/chart_data.ts create mode 100644 tests/end-to-end/fixtures/chart/labeled-pie.sql create mode 100644 tests/end-to-end/fixtures/chart/labeled-range-bar.sql delete mode 100644 tests/end-to-end/globals.d.ts diff --git a/frontend/src/apexcharts.ts b/frontend/src/apexcharts.ts index 916861416..5e0469f44 100644 --- a/frontend/src/apexcharts.ts +++ b/frontend/src/apexcharts.ts @@ -1,30 +1,34 @@ -import type { ApexOptions } from "apexcharts"; +import type { ApexChart, ApexOptions } from "apexcharts"; import ApexCharts from "apexcharts"; +import { type ReferenceLine, read_chart_data } from "./chart_data.ts"; import { align_series_for, - type ChartPoint, type ChartSeries, + type PlotValue, type Series, + type XValue, xaxis_type_for, } from "./chart_series.ts"; import { add_init_fn } from "./init.ts"; -type DataPoint = { - name: string; - x: string | number | null; - y: string | number | number[] | null; - color?: string | null; - z?: string | number | null; - link?: string; -}; - type AxisTitles = Record<"x" | "y" | "z", string | undefined>; -type TooltipArgs = { - seriesIndex: number; - dataPointIndex: number; - // biome-ignore lint/suspicious/noExplicitAny: ApexCharts leaves its tooltip context untyped - w: any; +type PointIndex = { seriesIndex: number; dataPointIndex: number }; + +/** + * ApexCharts leaves the options a chart rendered, on `chart.w`, undescribed. + * These are the ones every chart built here sets. + */ +export type RenderedChart = ApexCharts & { + w: { + config: { + chart: { type: string; stacked: boolean }; + xaxis: { type?: string; tickAmount?: number }; + series: ChartSeries[] | number[]; + tooltip: { custom?: (point: PointIndex) => string }; + }; + globals: { labels: (string | number)[]; isDataXYZ: boolean }; + }; }; function formatTooltipX( @@ -39,17 +43,73 @@ function formatTooltipX( return anchor.outerHTML; } -const rangeBarLabel = (_value: string | number, args?: TooltipArgs) => - args ? args.w.config.series[args.seriesIndex].name : ""; +const rangeBarLabel = + (names: (string | number)[]) => + (_value: string | number, point?: PointIndex) => + point ? String(names[point.seriesIndex] ?? "") : ""; -const pieLabel = (value: string | number, args?: TooltipArgs) => - args - ? `${args.w.config.labels[args.seriesIndex]}: ${Number(value).toFixed()}%` - : ""; +const pieLabel = + (labels: string[]) => (value: string | number, point?: PointIndex) => + point ? `${labels[point.seriesIndex]}: ${Number(value).toFixed()}%` : ""; const numberLabel = (value: string | number) => value == null ? "" : value.toLocaleString?.() || String(value); +const dateLabel = (value: number) => { + const date = new Date(value); + if (date.getHours() === 0 && date.getMinutes() === 0) + return date.toLocaleDateString(); + return date.toLocaleString(); +}; + +/** + * Bubble and scatter points carry an x, a y and a z that each need naming, and + * the tooltip ApexCharts draws names none of them. + */ +const axisTooltip = + ( + series: ChartSeries[], + titles: AxisTitles, + formatValue: (value: number | null) => string, + ) => + ({ seriesIndex, dataPointIndex }: PointIndex) => { + const plotted = series[seriesIndex]; + const point = plotted?.data[dataPointIndex]; + + const tooltip = document.createElement("div"); + tooltip.className = "apexcharts-tooltip-text"; + tooltip.style.fontFamily = "inherit"; + + const seriesName = document.createElement("div"); + seriesName.className = "apexcharts-tooltip-y-group"; + seriesName.style.fontWeight = "bold"; + seriesName.innerText = String(plotted?.name ?? ""); + tooltip.appendChild(seriesName); + + for (const axis of ["x", "y", "z"] as const) { + const measured = point?.[axis]; + if (measured == null) continue; + const format = (value: XValue) => + axis === "y" && typeof value === "number" + ? formatValue(value) + : String(value ?? ""); + const axisValue = document.createElement("div"); + axisValue.className = "apexcharts-tooltip-y-group"; + const labelSpan = document.createElement("span"); + labelSpan.className = "apexcharts-tooltip-text-y-label"; + labelSpan.innerText = `${titles[axis] || axis}: `; + axisValue.appendChild(labelSpan); + const valueSpan = document.createElement("span"); + valueSpan.className = "apexcharts-tooltip-text-y-value"; + valueSpan.innerText = Array.isArray(measured) + ? measured.map(format).join(" - ") + : format(measured); + axisValue.appendChild(valueSpan); + tooltip.appendChild(axisValue); + } + return tooltip.outerHTML; + }; + const sqlpage_chart = (() => { function sqlpage_chart() { const charts = document.querySelectorAll( @@ -98,24 +158,19 @@ const sqlpage_chart = (() => { const referenceColor = colorNames.get(isDarkTheme ? "gray-lt" : "gray"); - type ReferenceLine = Record< - "xline" | "xline_end" | "yline" | "yline_end" | "label" | "color", - string | number | null - >; - const named_color = (name: unknown): string | undefined => typeof name === "string" ? colorNames.get(name) : undefined; - const reference_color = (name: string | number | null) => + const reference_color = (name: PlotValue) => named_color(name) || referenceColor; function reference_lines( rows: ReferenceLine[], column: "x" | "y", axis: "x" | "y", - to_axis_value: (value: string | number) => unknown, + to_axis_value: (value: PlotValue) => unknown, ): object[] { - const on_axis = (value: string | number | null) => { + const on_axis = (value: PlotValue) => { if (value == null) return null; const placed = to_axis_value(value); return Number.isNaN(placed) ? null : placed; @@ -144,27 +199,16 @@ const sqlpage_chart = (() => { function build_sqlpage_chart(c: HTMLElement) { const [data_element] = c.getElementsByTagName("data"); - const data = JSON.parse(data_element.textContent); + const data = read_chart_data(data_element.textContent); const chartContainer = c.querySelector(".chart") as HTMLElement; chartContainer.innerHTML = ""; - const is_timeseries = !!data.time; + const is_timeseries = data.time; const chart_type = APEXCHARTS_TYPE_ALIASES.get(data.type) || data.type || "line"; + const is_pie = chart_type === "pie"; const is_stacked = - !!data.stacked && STACKABLE_CHART_TYPES.includes(chart_type); - const points: DataPoint[] = data.points - .filter(Array.isArray) - .map(([name, x, y, color, z, link]) => ({ - name, - x, - y, - color, - z, - link: link ?? undefined, - })); - const reference_rows: ReferenceLine[] = data.points.filter( - (row: unknown) => !Array.isArray(row), - ); + data.stacked && STACKABLE_CHART_TYPES.includes(chart_type); + const { points } = data; const series_map: Series = new Map(); for (const { name, x: old_x, y: old_y, color, z, link } of points) { const point_series: ChartSeries = series_map.get(name) ?? { @@ -172,13 +216,13 @@ const sqlpage_chart = (() => { data: [], }; series_map.set(name, point_series); - let x: string | number | Date | null = old_x; - let y = old_y; + let x: XValue = old_x; + let y: PlotValue | PlotValue[] = old_y; if (is_timeseries) { if (typeof x === "number") x = new Date(x * 1000); else if (chart_type === "rangeBar" && Array.isArray(y)) - y = y.map((y) => new Date(y).getTime()); - else x = new Date((x ?? 0) as string | number); + y = y.map((value) => new Date(value ?? 0).getTime()); + else x = new Date(x ?? 0); } point_series.data.push({ x, @@ -186,86 +230,98 @@ const sqlpage_chart = (() => { z, link, fillColor: named_color(color), - } as ChartPoint); + }); } - if (data.xmin == null) data.xmin = undefined; - if (data.xmax == null) data.xmax = undefined; - if (data.ymin == null) data.ymin = undefined; - if (data.ymax == null) data.ymax = undefined; const palette = [ - ...data.colors.map(named_color).filter((c) => c !== undefined), + ...data.colors.map(named_color).filter((color) => color !== undefined), ...tblrColors.map(([_, dark, light]) => (isDarkTheme ? dark : light)), ...tblrColors.map(([_, dark, light]) => (isDarkTheme ? light : dark)), ]; - let colors = palette; const chart_series = [...series_map.values()]; + const aligned_series = + chart_series.length > 1 + ? align_series_for(chart_series, chart_type, is_stacked) + : chart_series; const xaxis_type = xaxis_type_for( chart_series, chart_type, is_timeseries, - !!data.horizontal, + data.horizontal, ); - const labels = - chart_type === "pie" - ? points.map(({ name, x }) => String(x || name)) - : undefined; - const series = - chart_type === "pie" - ? points.map(({ y }) => Number.parseFloat(String(y))) - : chart_series.length > 1 - ? align_series_for(chart_series, chart_type, is_stacked) - : chart_series; - if (chart_type === "pie") - colors = points.map( - ({ color }, i) => named_color(color) || palette[i % palette.length], - ); + const pie_labels = is_pie + ? points.map(({ name, x }) => String(x || name)) + : []; + const series = is_pie + ? points.map(({ y }) => Number.parseFloat(String(y))) + : aligned_series; + const colors = is_pie + ? points.map( + ({ color }, i) => named_color(color) || palette[i % palette.length], + ) + : palette; - const to_timestamp = (v) => - (typeof v === "number" ? new Date(v * 1000) : new Date(v)).getTime(); + const to_timestamp = (value: PlotValue) => + (typeof value === "number" + ? new Date(value * 1000) + : new Date(value ?? 0) + ).getTime(); const dates_are_values = is_timeseries && chart_type === "rangeBar"; const to_value = dates_are_values ? to_timestamp : Number; const to_category = - is_timeseries && !dates_are_values ? to_timestamp : (v) => v; + is_timeseries && !dates_are_values + ? to_timestamp + : (value: PlotValue) => value; const inverted = - chart_type === "rangeBar" || (chart_type === "bar" && !!data.horizontal); + chart_type === "rangeBar" || (chart_type === "bar" && data.horizontal); const value_axis = inverted ? "x" : "y"; const category_axis = inverted ? "y" : "x"; const axis_titles: AxisTitles = { - x: data.xtitle || undefined, - y: data.ytitle || undefined, - z: data.ztitle || undefined, + x: data.xtitle, + y: data.ytitle, + z: data.ztitle, }; const has_point_links = points.some((point) => point.link); const text_x_values = chart_series.every(({ data }) => data.every(({ x }) => x == null || typeof x === "string"), ); + const formatValue = (value: number | null) => { + if (value == null) return ""; + if (dates_are_values) return dateLabel(value); + return value.toLocaleString(undefined, { maximumFractionDigits: 2 }); + }; + const pointLink = is_pie + ? ({ seriesIndex }: PointIndex) => points[seriesIndex]?.link + : ({ seriesIndex, dataPointIndex }: PointIndex) => + aligned_series[seriesIndex]?.data[dataPointIndex]?.link; const options: ApexOptions = { annotations: { [`${value_axis}axis`]: reference_lines( - reference_rows, + data.reference_lines, "y", value_axis, to_value, ), [`${category_axis}axis`]: reference_lines( - reference_rows, + data.reference_lines, "x", category_axis, to_category, ), }, chart: { - type: chart_type, + // The query may name any type ApexCharts draws, not only the ones + // the component documents. + type: chart_type as ApexChart["type"], fontFamily: "inherit", background: "transparent", parentHeightOffset: 0, height: chartContainer.style.height, stacked: is_stacked, toolbar: { - show: !!data.toolbar, + show: data.toolbar, }, animations: { enabled: false, @@ -275,7 +331,7 @@ const sqlpage_chart = (() => { }, events: { dataPointSelection: (_event, _chart, args) => { - const link = args && pointLink(args, points); + const link = args && pointLink(args); if (link) window.location.assign(link); }, }, @@ -285,20 +341,19 @@ const sqlpage_chart = (() => { palette: "palette4", }, legend: { - show: data.show_legend === null || !!data.show_legend, + show: data.show_legend, }, dataLabels: { - enabled: !!data.labels, + enabled: data.labels, dropShadow: { enabled: true, color: "var(--tblr-primary-bg-subtle)", }, - formatter: - chart_type === "rangeBar" - ? rangeBarLabel - : chart_type === "pie" - ? pieLabel - : numberLabel, + formatter: is_pie + ? pieLabel(pie_labels) + : chart_type === "rangeBar" + ? rangeBarLabel(aligned_series.map(({ name }) => name)) + : numberLabel, }, fill: { type: chart_type === "area" ? "gradient" : "solid", @@ -326,7 +381,7 @@ const sqlpage_chart = (() => { tickAmount: data.xticks || undefined, }, yaxis: { - logarithmic: !!data.logarithmic, + logarithmic: data.logarithmic, min: data.ymin, max: data.ymax, stepSize: data.ystep, @@ -336,7 +391,7 @@ const sqlpage_chart = (() => { }, }, markers: { - size: data.marker || 0, + size: data.marker ?? 0, strokeWidth: 0, hover: { sizeOffset: 5, @@ -347,100 +402,37 @@ const sqlpage_chart = (() => { interactive: has_point_links, custom: chart_type === "bubble" || chart_type === "scatter" - ? (args: TooltipArgs) => chartTooltip(args, axis_titles) + ? axisTooltip(aligned_series, axis_titles, formatValue) : undefined, x: { formatter: has_point_links && text_x_values - ? (value, args) => - formatTooltipX(value, args?.w && pointLink(args, points)) + ? (value, args) => formatTooltipX(value, args && pointLink(args)) : undefined, }, y: { - formatter: (value) => { - if (value == null) return ""; - if (is_timeseries && chart_type === "rangeBar") { - const d = new Date(value); - if (d.getHours() === 0 && d.getMinutes() === 0) - return d.toLocaleDateString(); - return d.toLocaleString(); - } - return value.toLocaleString(undefined, { - maximumFractionDigits: 2, - }); - }, + formatter: formatValue, }, }, plotOptions: { bar: { - horizontal: !!data.horizontal || chart_type === "rangeBar", + horizontal: data.horizontal || chart_type === "rangeBar", borderRadius: 5, }, bubble: { minBubbleRadius: 5 }, }, colors, - series, + // ApexCharts draws a numeric series name but declares only a string. + series: series as ApexOptions["series"], }; - if (labels) options.labels = labels; - const chart = new ApexCharts(chartContainer, options); + if (is_pie) options.labels = pie_labels; + const chart = new ApexCharts(chartContainer, options) as RenderedChart; chart.render().catch(console.error); if (window.charts) window.charts.push(chart); else window.charts = [chart]; c.removeAttribute("data-pre-init"); } - function chartTooltip( - { seriesIndex, dataPointIndex, w }: TooltipArgs, - titles: AxisTitles, - ) { - const series = w.config.series[seriesIndex]; - const name = series?.name || ""; - const point = series?.data[dataPointIndex]; - - const tooltip = document.createElement("div"); - tooltip.className = "apexcharts-tooltip-text"; - tooltip.style.fontFamily = "inherit"; - - const seriesName = document.createElement("div"); - seriesName.className = "apexcharts-tooltip-y-group"; - seriesName.style.fontWeight = "bold"; - seriesName.innerText = name; - tooltip.appendChild(seriesName); - - for (const axis of ["x", "y", "z"] as const) { - const value = point[axis]; - if (value == null) continue; - const axisValue = document.createElement("div"); - axisValue.className = "apexcharts-tooltip-y-group"; - const title = titles[axis] || axis; - const labelSpan = document.createElement("span"); - labelSpan.className = "apexcharts-tooltip-text-y-label"; - labelSpan.innerText = `${title}: `; - axisValue.appendChild(labelSpan); - const valueSpan = document.createElement("span"); - valueSpan.className = "apexcharts-tooltip-text-y-value"; - const formatter = axis === "y" && w.config.tooltip.y.formatter; - const format = (v) => - formatter ? formatter(v, { seriesIndex, dataPointIndex, w }) : v; - valueSpan.innerText = Array.isArray(value) - ? value.map(format).join(" - ") - : format(value); - axisValue.appendChild(valueSpan); - tooltip.appendChild(axisValue); - } - return tooltip.outerHTML; - } - - function pointLink( - { seriesIndex, dataPointIndex, w }: TooltipArgs, - points: DataPoint[], - ) { - const series = w.config.series[seriesIndex]; - return Array.isArray(series?.data) - ? series.data[dataPointIndex]?.link - : points[seriesIndex]?.link; - } - return sqlpage_chart; })(); diff --git a/frontend/src/chart_data.ts b/frontend/src/chart_data.ts new file mode 100644 index 000000000..fe8d9d6ab --- /dev/null +++ b/frontend/src/chart_data.ts @@ -0,0 +1,109 @@ +import type { PlotValue } from "./chart_series.ts"; + +export type DataPoint = { + name: string | number; + x: PlotValue; + y: PlotValue | PlotValue[]; + color: PlotValue; + z: PlotValue | undefined; + link: string | undefined; +}; + +/** A row that draws a line across the chart instead of plotting a point. */ +export type ReferenceLine = Record< + "xline" | "xline_end" | "yline" | "yline_end" | "label" | "color", + PlotValue +>; + +/** One chart's properties, as `chart.handlebars` encodes them. */ +export type ChartData = { + type: string; + time: boolean; + labels: boolean; + marker: number | undefined; + xtitle: string | undefined; + ytitle: string | undefined; + ztitle: string | undefined; + xticks: number | undefined; + yticks: number | undefined; + ystep: number | undefined; + xmin: number | undefined; + ymin: number | undefined; + xmax: number | undefined; + ymax: number | undefined; + toolbar: boolean; + show_legend: boolean; + logarithmic: boolean; + horizontal: boolean; + stacked: boolean; + colors: PlotValue[]; + points: DataPoint[]; + reference_lines: ReferenceLine[]; +}; + +const plot_value = (value: unknown): PlotValue => { + if (value == null) return null; + if (typeof value === "number" || typeof value === "string") return value; + if (typeof value === "boolean") return Number(value); + return String(value); +}; + +const text_value = (value: unknown) => + String(plot_value(value) ?? "") || undefined; + +const number_value = (value: unknown) => { + const text = plot_value(value); + const number = text == null || text === "" ? Number.NaN : Number(text); + return Number.isFinite(number) ? number : undefined; +}; + +const data_point = ([name, x, y, color, z, link]: unknown[]): DataPoint => ({ + name: plot_value(name) ?? "", + x: plot_value(x), + y: Array.isArray(y) ? y.map(plot_value) : plot_value(y), + color: plot_value(color), + // ApexCharts reads a point as three-dimensional as soon as it carries a z, + // so a row that never mentioned one must not carry a null. + z: z === undefined ? undefined : plot_value(z), + link: text_value(link), +}); + +const reference_line = (row: Record): ReferenceLine => ({ + xline: plot_value(row.xline), + xline_end: plot_value(row.xline_end), + yline: plot_value(row.yline), + yline_end: plot_value(row.yline_end), + label: plot_value(row.label), + color: plot_value(row.color), +}); + +export function read_chart_data(json: string | null): ChartData { + const data = JSON.parse(json ?? ""); + const rows = Array.isArray(data.points) ? data.points : []; + return { + type: text_value(data.type) ?? "", + time: !!data.time, + labels: !!data.labels, + marker: number_value(data.marker), + xtitle: text_value(data.xtitle), + ytitle: text_value(data.ytitle), + ztitle: text_value(data.ztitle), + xticks: number_value(data.xticks), + yticks: number_value(data.yticks), + ystep: number_value(data.ystep), + xmin: number_value(data.xmin), + ymin: number_value(data.ymin), + xmax: number_value(data.xmax), + ymax: number_value(data.ymax), + toolbar: !!data.toolbar, + show_legend: data.show_legend == null || !!data.show_legend, + logarithmic: !!data.logarithmic, + horizontal: !!data.horizontal, + stacked: !!data.stacked, + colors: Array.isArray(data.colors) ? data.colors.map(plot_value) : [], + points: rows.filter(Array.isArray).map(data_point), + reference_lines: rows + .filter((row: unknown) => !Array.isArray(row)) + .map(reference_line), + }; +} diff --git a/frontend/src/chart_series.ts b/frontend/src/chart_series.ts index 45e26ba6b..52e6d7fbb 100644 --- a/frontend/src/chart_series.ts +++ b/frontend/src/chart_series.ts @@ -1,13 +1,14 @@ -export type XValue = number | string | Date; +export type PlotValue = string | number | null; +export type XValue = PlotValue | Date; export type ChartPoint = { x: XValue; - y: number | string | number[] | null; - z?: number; + y: PlotValue | PlotValue[]; + z?: PlotValue; fillColor?: string; link?: string; }; -export type ChartSeries = { name: string; data: ChartPoint[] }; -export type Series = Map; +export type ChartSeries = { name: string | number; data: ChartPoint[] }; +export type Series = Map; const NUMERIC_X_CHART_TYPES = ["line", "area", "bar", "scatter", "bubble"]; @@ -21,8 +22,10 @@ const Y_WHEN_A_SERIES_SKIPS_A_LABEL = new Map([ ]); /** equal x values share a key */ -const x_key = (x: XValue): number | string => - x instanceof Date ? x.getTime() : x; +const x_key = (x: XValue): PlotValue => (x instanceof Date ? x.getTime() : x); + +/** A missing x sorts as zero, which is how JavaScript compares it. */ +const is_lower = (x: XValue, than: XValue) => (x ?? 0) < (than ?? 0); const x_is_text = (series: ChartSeries[]) => typeof series[0]?.data?.[0]?.x === "string"; @@ -50,11 +53,11 @@ export function xaxis_type_for( */ export function merged_x_values(series: ChartSeries[]): XValue[] { const unread = series.map(({ data }) => data.map(({ x }) => x)); - const merged = new Map(); + const merged = new Map(); while (unread.some((xs) => xs.length > 0)) { const with_lowest_x = unread .filter((xs) => xs.length > 0) - .reduce((a, b) => (b[0] < a[0] ? b : a)); + .reduce((a, b) => (is_lower(b[0], a[0]) ? b : a)); const x = with_lowest_x.shift() as XValue; merged.set(x_key(x), x); } diff --git a/frontend/src/globals.d.ts b/frontend/src/globals.d.ts index d4af324cc..274fb0414 100644 --- a/frontend/src/globals.d.ts +++ b/frontend/src/globals.d.ts @@ -1,8 +1,14 @@ -// Names the browser bundle relies on at runtime rather than through an import. +// What the bundle installs on the page at runtime rather than through an +// import: the page's own scripts and the browser tests read these back. interface Window { /** Every chart rendered on the page, in the order they were built. */ - charts?: unknown[]; + charts?: import("./apexcharts.ts").RenderedChart[]; /** A Bootstrap a page loaded for itself, preferred over the bundled copy. */ bootstrap?: typeof import("@tabler/core").bootstrap; } + +interface HTMLElement { + /** Attached by sqlpage_select_dropdown to every select it takes over. */ + tomselect?: import("tom-select/popular").default; +} diff --git a/tests/end-to-end/fixture.ts b/tests/end-to-end/fixture.ts index c6addb1c0..3f19cbe25 100644 --- a/tests/end-to-end/fixture.ts +++ b/tests/end-to-end/fixture.ts @@ -30,5 +30,5 @@ export const test = base.extend({ }, }); -export type { Page } from "@playwright/test"; +export type { ConsoleMessage, Page } from "@playwright/test"; export { expect }; diff --git a/tests/end-to-end/fixtures/chart/labeled-pie.sql b/tests/end-to-end/fixtures/chart/labeled-pie.sql new file mode 100644 index 000000000..d0cc23d10 --- /dev/null +++ b/tests/end-to-end/fixtures/chart/labeled-pie.sql @@ -0,0 +1,2 @@ +SELECT 'chart' AS component, 'test-chart' AS id, 'Chart test fixture' AS title, 'pie' AS type, TRUE AS labels; +WITH points(label, value) AS (VALUES ('Yes', 65), ('No', 35)) SELECT * FROM points; diff --git a/tests/end-to-end/fixtures/chart/labeled-range-bar.sql b/tests/end-to-end/fixtures/chart/labeled-range-bar.sql new file mode 100644 index 000000000..c90ba5a95 --- /dev/null +++ b/tests/end-to-end/fixtures/chart/labeled-range-bar.sql @@ -0,0 +1,3 @@ +SELECT 'chart' AS component, 'test-chart' AS id, 'Chart test fixture' AS title, 'rangeBar' AS type, TRUE AS time, TRUE AS labels; +SELECT 'Design' AS series, 'Alice' AS label, '2024-03-01' AS value, '2024-03-05' AS value; +SELECT 'Build' AS series, 'Bob' AS label, '2024-03-04' AS value, '2024-03-09' AS value; diff --git a/tests/end-to-end/fixtures/chart/test.ts b/tests/end-to-end/fixtures/chart/test.ts index 022dfcdcc..ced24d71e 100644 --- a/tests/end-to-end/fixtures/chart/test.ts +++ b/tests/end-to-end/fixtures/chart/test.ts @@ -1,28 +1,4 @@ -import { expect, type Page, test } from "../../fixture.ts"; - -type ChartPoint = { x: string | number | Date; y: number | null }; - -declare global { - interface Window { - charts?: { - w: { - config: { - chart: { type: string; stacked: boolean }; - xaxis: { type?: string; tickAmount?: number }; - series: { name: string | number; data?: ChartPoint[] }[]; - tooltip: { - custom?: (args: { - seriesIndex: number; - dataPointIndex: number; - w: unknown; - }) => string; - }; - }; - globals: { labels: (string | number)[] }; - }; - }[]; - } -} +import { type ConsoleMessage, expect, type Page, test } from "../../fixture.ts"; const MARKS = ".apexcharts-bar-area, .apexcharts-rangebar-area, .apexcharts-treemap-rect, .apexcharts-pie-area, .apexcharts-heatmap-rect, .apexcharts-series .apexcharts-marker"; @@ -32,7 +8,7 @@ const ORANGE = "#f76707"; const GREEN = "#37b24d"; async function renderChart(page: Page, fixture: string) { const failures: string[] = []; - const recordError = (message: { type(): string; text(): string }) => { + const recordError = (message: ConsoleMessage) => { if (message.type() === "error") failures.push(message.text()); }; page.on("console", recordError); @@ -46,13 +22,15 @@ async function renderChart(page: Page, fixture: string) { const container = document.getElementById("test-chart"); if (!container) throw new Error("Chart fixture did not render"); const rendered = window.charts?.[0]; - const series = (rendered?.w.config.series ?? []).map((s) => ({ - name: s.name, - points: (s.data ?? []).map((p) => [ - p.x instanceof Date ? p.x.toISOString() : p.x, - p.y, - ]), - })); + const series = (rendered?.w.config.series ?? []) + .filter((s) => typeof s !== "number") + .map((s) => ({ + name: s.name, + points: s.data.map((p) => [ + p.x instanceof Date ? p.x.toISOString() : p.x, + p.y, + ]), + })); const drawnPerSeries = series.map(({ name }) => { const markers = [ ...container.querySelectorAll( @@ -122,9 +100,12 @@ async function renderChart(page: Page, fixture: string) { tickAmount: rendered?.w.config.xaxis.tickAmount ?? null, }, generatedLabels: rendered?.w.globals.labels ?? [], + threeDimensional: rendered?.w.globals.isDataXYZ ?? null, axisLabels, dataLabels: [ - ...container.querySelectorAll(".apexcharts-datalabel"), + ...container.querySelectorAll( + ".apexcharts-datalabel, .apexcharts-pie-label", + ), ].map((label) => label.textContent), barGroups, series, @@ -407,6 +388,16 @@ for (const type of ["area", "scatter", "heatmap"]) { }); } +test("counts a third dimension only where the rows carried one", async ({ + page, +}) => { + const flat = await renderChart(page, "index"); + const bubbles = await renderChart(page, "bubble-categories"); + + expect(flat.threeDimensional).toBe(false); + expect(bubbles.threeDimensional).toBe(true); +}); + test("keeps the bubble size of the points it lined up", async ({ page }) => { const chart = await renderChart(page, "bubble-categories"); @@ -455,6 +446,22 @@ test("gives the tooltip title the color of the tooltip around it", async ({ expect(colors.title).toBe(colors.tooltip); }); +test("names the series of every bar of a range bar chart", async ({ page }) => { + const chart = await renderChart(page, "labeled-range-bar"); + + expect(chart.failures).toEqual([]); + expect(chart.dataLabels).toEqual(["Design", "Build"]); +}); + +test("gives every slice of a pie chart its label and its share", async ({ + page, +}) => { + const chart = await renderChart(page, "labeled-pie"); + + expect(chart.failures).toEqual([]); + expect(chart.dataLabels).toEqual(["Yes: 65%", "No: 35%"]); +}); + test("draws a reference line that carries no label", async ({ page }) => { const chart = await renderChart(page, "unlabeled-reference-lines"); @@ -601,11 +608,7 @@ test("labels each axis of a bubble tooltip with its own title", async ({ const custom = chart.w.config.tooltip.custom; if (!custom) throw new Error("A bubble chart needs the custom tooltip"); const holder = document.createElement("div"); - holder.innerHTML = custom({ - seriesIndex: 0, - dataPointIndex: 1, - w: chart.w, - }); + holder.innerHTML = custom({ seriesIndex: 0, dataPointIndex: 1 }); const values = holder.querySelectorAll(".apexcharts-tooltip-text-y-value"); return [...holder.querySelectorAll(".apexcharts-tooltip-text-y-label")].map( (label, i): [string, string] => [ diff --git a/tests/end-to-end/fixtures/map/test.ts b/tests/end-to-end/fixtures/map/test.ts index a65650768..3df8cd612 100644 --- a/tests/end-to-end/fixtures/map/test.ts +++ b/tests/end-to-end/fixtures/map/test.ts @@ -1,4 +1,4 @@ -import { expect, type Page, test } from "../../fixture.ts"; +import { type ConsoleMessage, expect, type Page, test } from "../../fixture.ts"; const PARIS_WITHOUT_ITS_LONGITUDE = "48.85,"; const NOT_COORDINATES = "somewhere nice"; @@ -7,7 +7,7 @@ async function renderMap(page: Page, fixture: string, markerCount = 0) { const errors: string[] = []; const logged: string[] = []; const recordPageError = (error: Error) => errors.push(error.message); - const recordConsoleError = (message: { type(): string; text(): string }) => { + const recordConsoleError = (message: ConsoleMessage) => { if (message.type() === "error") logged.push(message.text()); }; page.on("pageerror", recordPageError); diff --git a/tests/end-to-end/globals.d.ts b/tests/end-to-end/globals.d.ts deleted file mode 100644 index ce7f66036..000000000 --- a/tests/end-to-end/globals.d.ts +++ /dev/null @@ -1,7 +0,0 @@ -// What the browser tests reach for on the page: widgets that the scripts under -// test attach to elements at runtime. - -interface HTMLElement { - /** Attached by sqlpage_select_dropdown to every select it takes over. */ - tomselect?: import("tom-select/popular").default; -} diff --git a/tests/end-to-end/tsconfig.json b/tests/end-to-end/tsconfig.json index f6bccf8fe..6567c5634 100644 --- a/tests/end-to-end/tsconfig.json +++ b/tests/end-to-end/tsconfig.json @@ -5,5 +5,5 @@ "moduleResolution": "nodenext", "types": ["node"] }, - "include": ["**/*.ts"] + "include": ["**/*.ts", "../../frontend/src/globals.d.ts"] } diff --git a/tests/js/chart_series.spec.ts b/tests/js/chart_series.spec.ts index 3c52b6166..ed56fd223 100644 --- a/tests/js/chart_series.spec.ts +++ b/tests/js/chart_series.spec.ts @@ -3,6 +3,8 @@ import test from "node:test"; import { align_series, align_series_for, + type ChartPoint, + type ChartSeries, merged_x_values, xaxis_type_for, } from "../../frontend/src/chart_series.ts"; @@ -12,17 +14,11 @@ const LEAVES_A_GAP = null; const STACKED = true; const UNSTACKED = false; -type XValue = number | string | Date; -type Point = { - x: XValue; - y: number | string | null | number[]; - z?: number; - fillColor?: string; -}; -type Series = { name: string; data: Point[] }; - -const series = (name: string, ...data: Point[]): Series => ({ name, data }); -const xs = (s: Series) => s.data.map((p) => p.x); +const series = (name: string, ...data: ChartPoint[]): ChartSeries => ({ + name, + data, +}); +const xs = (s: ChartSeries) => s.data.map((p) => p.x); test("uses a continuous axis for numeric Cartesian x values", () => { const numeric = [series("a", { x: 1, y: 1 }, { x: 12, y: 12 })]; diff --git a/tsconfig.json b/tsconfig.json index 7d017ff49..aa8793a19 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -16,7 +16,6 @@ "noUnusedParameters": true, "allowUnreachableCode": false, "allowUnusedLabels": false, - "noImplicitAny": false, "types": [], "noImplicitReturns": true },