Skip to content

Commit 7cbaebc

Browse files
authored
fix(chart) :: use typed Map instead of Object.prototype
Before loosly typed `Object.protoype` was used to map user inputs to built in properties. This is what a `Map` is supposed to do, so we use that with stong types.
1 parent e410f4c commit 7cbaebc

4 files changed

Lines changed: 34 additions & 21 deletions

File tree

‎frontend/src/apexcharts.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,23 +67,27 @@ const sqlpage_chart = (() => {
6767
["black", "#000000", "#000000"],
6868
["white", "#ffffff", "#f8f9fa"],
6969
];
70-
const colorNames = Object.fromEntries(
71-
tblrColors.flatMap(([name, dark, light]) => [
70+
const colorNames = new Map(
71+
tblrColors.flatMap(([name, dark, light]): [string, string][] => [
7272
[name, dark],
7373
[`${name}-lt`, light],
7474
]),
7575
);
7676
const isDarkTheme = document.body?.dataset?.bsTheme === "dark";
7777

7878
const STACKABLE_CHART_TYPES = ["line", "area", "bar"];
79-
const APEXCHARTS_TYPE_ALIASES = { column: "bar" };
79+
const STROKE_WIDTHS = new Map([
80+
["area", 3],
81+
["line", 2],
82+
]);
83+
const APEXCHARTS_TYPE_ALIASES = new Map([["column", "bar"]]);
8084

81-
const referenceColor = colorNames[isDarkTheme ? "gray-lt" : "gray"];
85+
const referenceColor = colorNames.get(isDarkTheme ? "gray-lt" : "gray");
8286

8387
type ReferenceLine = { [property: string]: string | number | null };
8488

8589
const named_color = (name: unknown): string | undefined =>
86-
typeof name === "string" ? colorNames[name] : undefined;
90+
typeof name === "string" ? colorNames.get(name) : undefined;
8791

8892
const reference_color = (name: string | number | null) =>
8993
named_color(name) || referenceColor;
@@ -128,7 +132,7 @@ const sqlpage_chart = (() => {
128132
chartContainer.innerHTML = "";
129133
const is_timeseries = !!data.time;
130134
const chart_type =
131-
APEXCHARTS_TYPE_ALIASES[data.type] || data.type || "line";
135+
APEXCHARTS_TYPE_ALIASES.get(data.type) || data.type || "line";
132136
const is_stacked =
133137
!!data.stacked && STACKABLE_CHART_TYPES.includes(chart_type);
134138
const points: DataPoint[] = data.points
@@ -277,11 +281,7 @@ const sqlpage_chart = (() => {
277281
type: chart_type === "area" ? "gradient" : "solid",
278282
},
279283
stroke: {
280-
width:
281-
{
282-
area: 3,
283-
line: 2,
284-
}[chart_type] || 0,
284+
width: STROKE_WIDTHS.get(chart_type) ?? 0,
285285
lineCap: "round",
286286
curve: "smooth",
287287
},

‎frontend/src/chart_series.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ export type Series = Map<string, ChartSeries>;
1111

1212
const NUMERIC_X_CHART_TYPES = ["line", "area", "bar", "scatter", "bubble"];
1313

14-
const Y_WHEN_A_SERIES_SKIPS_A_LABEL: Record<string, number | null> = {
15-
bar: 0,
16-
line: null,
17-
area: null,
18-
scatter: null,
19-
bubble: null,
20-
heatmap: null,
21-
};
14+
const Y_WHEN_A_SERIES_SKIPS_A_LABEL = new Map<string, number | null>([
15+
["bar", 0],
16+
["line", null],
17+
["area", null],
18+
["scatter", null],
19+
["bubble", null],
20+
["heatmap", null],
21+
]);
2222

2323
/** equal x values share a key */
2424
const x_key = (x: XValue): number | string =>
@@ -92,7 +92,8 @@ export function align_series_for(
9292
is_stacked: boolean,
9393
): ChartSeries[] {
9494
if (is_stacked) return align_series(series, 0);
95-
if (x_is_text(series) && chart_type in Y_WHEN_A_SERIES_SKIPS_A_LABEL)
96-
return align_series(series, Y_WHEN_A_SERIES_SKIPS_A_LABEL[chart_type]);
95+
const y_when_missing = Y_WHEN_A_SERIES_SKIPS_A_LABEL.get(chart_type);
96+
if (x_is_text(series) && y_when_missing !== undefined)
97+
return align_series(series, y_when_missing);
9798
return series;
9899
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
SELECT 'chart' AS component, 'test-chart' AS id, 'Chart test fixture' AS title, 'bar' AS type, 'toString' AS color, 4 AS marker;
2+
WITH points(series, x, y) AS (VALUES ('A', 'Q1', 1), ('A', 'Q2', 2)) SELECT * FROM points;

‎tests/end-to-end/fixtures/chart/test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,16 @@ test("keeps the default palette when the chart names a color SQLPage does not kn
547547
expect(fills(unknown)).toEqual(fills(plain));
548548
});
549549

550+
test("draws a chart whose color names a built-in JavaScript property", async ({
551+
page,
552+
}) => {
553+
const plain = await renderChart(page, "uncolored-bar");
554+
const chart = await renderChart(page, "builtin-chart-color");
555+
556+
expect(chart.failures).toEqual([]);
557+
expect(fills(chart)).toEqual(fills(plain));
558+
});
559+
550560
test("renders series named after built-in JavaScript properties", async ({
551561
page,
552562
}) => {

0 commit comments

Comments
 (0)