Skip to content

Commit 5cef0b4

Browse files
committed
Fix for ChartConfigPanel dependency issue
1 parent 2ca9d15 commit 5cef0b4

File tree

1 file changed

+26
-17
lines changed

1 file changed

+26
-17
lines changed

apps/webapp/app/components/code/ChartConfigPanel.tsx

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { OutputColumnMetadata } from "@internal/clickhouse";
22
import { BarChart, LineChart } from "lucide-react";
3-
import { useCallback, useEffect, useMemo } from "react";
3+
import { useCallback, useEffect, useMemo, useRef } from "react";
44
import { cn } from "~/utils/cn";
55
import { Header3 } from "../primitives/Headers";
66
import { Paragraph } from "../primitives/Paragraph";
@@ -101,50 +101,59 @@ export function ChartConfigPanel({ columns, config, onChange, className }: Chart
101101
};
102102
}, [columns]);
103103

104+
// Create a stable key from column names and types to detect actual changes
105+
const columnsKey = useMemo(() => columns.map((c) => `${c.name}:${c.type}`).join(","), [columns]);
106+
107+
// Use refs to access current config/onChange without adding them as dependencies
108+
const configRef = useRef(config);
109+
const onChangeRef = useRef(onChange);
110+
useEffect(() => {
111+
configRef.current = config;
112+
onChangeRef.current = onChange;
113+
});
114+
104115
// Auto-select defaults when columns change
105116
useEffect(() => {
106117
if (columns.length === 0) return;
107118

119+
const currentConfig = configRef.current;
108120
let needsUpdate = false;
109121
const updates: Partial<ChartConfiguration> = {};
110122

111123
// Auto-select X-axis (prefer datetime, then first categorical)
112-
if (!config.xAxisColumn) {
124+
if (!currentConfig.xAxisColumn) {
113125
const defaultX = dateTimeColumns[0] ?? categoricalColumns[0] ?? columns[0];
114126
if (defaultX) {
115127
updates.xAxisColumn = defaultX.name;
116128
needsUpdate = true;
117-
118-
// Auto-set sort to x-axis ASC if it's a datetime column
119-
if (isDateTimeType(defaultX.type) && !config.sortByColumn) {
120-
updates.sortByColumn = defaultX.name;
121-
updates.sortDirection = "asc";
122-
}
123129
}
124130
}
125131

126132
// Auto-select Y-axis (first numeric column)
127-
if (config.yAxisColumns.length === 0 && numericColumns.length > 0) {
133+
if (currentConfig.yAxisColumns.length === 0 && numericColumns.length > 0) {
128134
updates.yAxisColumns = [numericColumns[0].name];
129135
needsUpdate = true;
130136
}
131137

132-
// Auto-set sort by x-axis ASC if x-axis is datetime and no sort is configured
138+
// Determine the effective x-axis column (either existing or newly selected)
139+
const effectiveXAxis = updates.xAxisColumn ?? currentConfig.xAxisColumn;
140+
141+
// Auto-set sort to x-axis ASC if it's a datetime column and no sort is configured
133142
if (
134-
config.xAxisColumn &&
135-
!config.sortByColumn &&
136-
!updates.sortByColumn &&
137-
dateTimeColumns.some((col) => col.name === config.xAxisColumn)
143+
effectiveXAxis &&
144+
!currentConfig.sortByColumn &&
145+
dateTimeColumns.some((col) => col.name === effectiveXAxis)
138146
) {
139-
updates.sortByColumn = config.xAxisColumn;
147+
updates.sortByColumn = effectiveXAxis;
140148
updates.sortDirection = "asc";
141149
needsUpdate = true;
142150
}
143151

144152
if (needsUpdate) {
145-
onChange({ ...config, ...updates });
153+
onChangeRef.current({ ...currentConfig, ...updates });
146154
}
147-
}, [columns, config, onChange, dateTimeColumns, categoricalColumns, numericColumns]);
155+
// Only re-run when the actual column structure changes, not on every config change
156+
}, [columnsKey, columns, dateTimeColumns, categoricalColumns, numericColumns]);
148157

149158
const updateConfig = useCallback(
150159
(updates: Partial<ChartConfiguration>) => {

0 commit comments

Comments
 (0)