From 5fc637617fbd3c5305057cec3c6825a464a8575f Mon Sep 17 00:00:00 2001
From: 81reap <prayag.bhakar@gmail.com>
Date: Mon, 28 Sep 2026 23:28:20 -0400
Subject: [PATCH] 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.
---
 frontend/src/apexcharts.ts                    | 22 +++++++++----------
 frontend/src/chart_series.ts                  | 21 +++++++++---------
 .../fixtures/chart/builtin-chart-color.sql    |  2 ++
 tests/end-to-end/fixtures/chart/test.ts       | 10 +++++++++
 4 files changed, 34 insertions(+), 21 deletions(-)
 create mode 100644 tests/end-to-end/fixtures/chart/builtin-chart-color.sql

diff --git a/frontend/src/apexcharts.ts b/frontend/src/apexcharts.ts
index fb09d984..8d15b324 100644
--- a/frontend/src/apexcharts.ts
+++ b/frontend/src/apexcharts.ts
@@ -39,8 +39,8 @@ const sqlpage_chart = (() => {
     ["black", "#000000", "#000000"],
     ["white", "#ffffff", "#f8f9fa"],
   ];
-  const colorNames = Object.fromEntries(
-    tblrColors.flatMap(([name, dark, light]) => [
+  const colorNames = new Map(
+    tblrColors.flatMap(([name, dark, light]): [string, string][] => [
       [name, dark],
       [`${name}-lt`, light],
     ]),
@@ -48,14 +48,18 @@ const sqlpage_chart = (() => {
   const isDarkTheme = document.body?.dataset?.bsTheme === "dark";
 
   const STACKABLE_CHART_TYPES = ["line", "area", "bar"];
-  const APEXCHARTS_TYPE_ALIASES = { column: "bar" };
+  const STROKE_WIDTHS = new Map([
+    ["area", 3],
+    ["line", 2],
+  ]);
+  const APEXCHARTS_TYPE_ALIASES = new Map([["column", "bar"]]);
 
-  const referenceColor = colorNames[isDarkTheme ? "gray-lt" : "gray"];
+  const referenceColor = colorNames.get(isDarkTheme ? "gray-lt" : "gray");
 
   type ReferenceLine = { [property: string]: string | number | null };
 
   const named_color = (name: unknown): string | undefined =>
-    typeof name === "string" ? colorNames[name] : undefined;
+    typeof name === "string" ? colorNames.get(name) : undefined;
 
   const reference_color = (name: string | number | null) =>
     named_color(name) || referenceColor;
@@ -100,7 +104,7 @@ const sqlpage_chart = (() => {
     chartContainer.innerHTML = "";
     const is_timeseries = !!data.time;
     const chart_type =
-      APEXCHARTS_TYPE_ALIASES[data.type] || data.type || "line";
+      APEXCHARTS_TYPE_ALIASES.get(data.type) || data.type || "line";
     const is_stacked =
       !!data.stacked && STACKABLE_CHART_TYPES.includes(chart_type);
     const points = data.points.filter(Array.isArray);
@@ -219,11 +223,7 @@ const sqlpage_chart = (() => {
         type: chart_type === "area" ? "gradient" : "solid",
       },
       stroke: {
-        width:
-          {
-            area: 3,
-            line: 2,
-          }[chart_type] || 0,
+        width: STROKE_WIDTHS.get(chart_type) ?? 0,
         lineCap: "round",
         curve: "smooth",
       },
diff --git a/frontend/src/chart_series.ts b/frontend/src/chart_series.ts
index bd6b48a1..1c63c137 100644
--- a/frontend/src/chart_series.ts
+++ b/frontend/src/chart_series.ts
@@ -10,14 +10,14 @@ export type Series = Map<string, ChartSeries>;
 
 const NUMERIC_X_CHART_TYPES = ["line", "area", "bar", "scatter", "bubble"];
 
-const Y_WHEN_A_SERIES_SKIPS_A_LABEL: Record<string, number | null> = {
-  bar: 0,
-  line: null,
-  area: null,
-  scatter: null,
-  bubble: null,
-  heatmap: null,
-};
+const Y_WHEN_A_SERIES_SKIPS_A_LABEL = new Map<string, number | null>([
+  ["bar", 0],
+  ["line", null],
+  ["area", null],
+  ["scatter", null],
+  ["bubble", null],
+  ["heatmap", null],
+]);
 
 /** equal x values share a key */
 const x_key = (x: XValue): number | string =>
@@ -91,7 +91,8 @@ export function align_series_for(
   is_stacked: boolean,
 ): ChartSeries[] {
   if (is_stacked) return align_series(series, 0);
-  if (x_is_text(series) && chart_type in Y_WHEN_A_SERIES_SKIPS_A_LABEL)
-    return align_series(series, Y_WHEN_A_SERIES_SKIPS_A_LABEL[chart_type]);
+  const y_when_missing = Y_WHEN_A_SERIES_SKIPS_A_LABEL.get(chart_type);
+  if (x_is_text(series) && y_when_missing !== undefined)
+    return align_series(series, y_when_missing);
   return series;
 }
diff --git a/tests/end-to-end/fixtures/chart/builtin-chart-color.sql b/tests/end-to-end/fixtures/chart/builtin-chart-color.sql
new file mode 100644
index 00000000..3b93b699
--- /dev/null
+++ b/tests/end-to-end/fixtures/chart/builtin-chart-color.sql
@@ -0,0 +1,2 @@
+SELECT 'chart' AS component, 'test-chart' AS id, 'Chart test fixture' AS title, 'bar' AS type, 'toString' AS color, 4 AS marker;
+WITH points(series, x, y) AS (VALUES ('A', 'Q1', 1), ('A', 'Q2', 2)) SELECT * FROM points;
diff --git a/tests/end-to-end/fixtures/chart/test.ts b/tests/end-to-end/fixtures/chart/test.ts
index 1844d39a..d073081d 100644
--- a/tests/end-to-end/fixtures/chart/test.ts
+++ b/tests/end-to-end/fixtures/chart/test.ts
@@ -497,6 +497,16 @@ test("keeps the default palette when the chart names a color SQLPage does not kn
   expect(fills(unknown)).toEqual(fills(plain));
 });
 
+test("draws a chart whose color names a built-in JavaScript property", async ({
+  page,
+}) => {
+  const plain = await renderChart(page, "uncolored-bar");
+  const chart = await renderChart(page, "builtin-chart-color");
+
+  expect(chart.failures).toEqual([]);
+  expect(fills(chart)).toEqual(fills(plain));
+});
+
 test("renders series named after built-in JavaScript properties", async ({
   page,
 }) => {