tbonelee commented on code in PR #5257:
URL: https://github.com/apache/zeppelin/pull/5257#discussion_r3298212746
##########
zeppelin-web-angular/projects/zeppelin-react/src/components/visualizations/TableVisualization.tsx:
##########
@@ -75,72 +74,132 @@ export const TableVisualization = ({ result, config }:
TableVisualizationProps)
y: parseFloat(row[1] || '0') || 0
}));
- let chart: Column | Line | Pie | Scatter | null = null;
+ chartRef.current.innerHTML = '';
+
+ let chart: any = null;
Review Comment:
**praise:** Lint is clean now. `Chart | null` / `ChartConfiguration | null`
resolves both `no-explicit-any` errors, and the union still narrows per-case at
the `{ type: 'bar', ... }` literal assignment, so each case's dataset/options
shape stays type-checked. Verified locally with the project ruleset: errors 4 →
0 on this file.
**thought (non-blocking):** If you ever want tighter per-case typing,
`Chart<'bar'>` / `ChartConfiguration<'bar'>` etc. drop the union entirely and
narrow without relying on the assignment site. Current shape is good;
completely optional follow-up.
**nitpick (non-blocking):** Now that `container` is captured at effect
start, the `!container` check inside `import('chart.js/auto').then(...)` and
the `if (container)` guard in the cleanup are unreachable. `container` was
already non-null past the early return, and closure capture pins it for the
effect's lifetime. Safe to simplify to `if (cancelled) return;` and
`container.innerHTML = '';`. Pure tidy-up, no behavior change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
