fix(cli): add missing chart types to plugin list command - #666
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThe ChangesBuilt-in Chart Registry Sync
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/src/commands/plugin.ts (1)
137-159: ⚡ Quick winAvoid duplicating the built-in chart list here.
Adding the missing chart types fixes today’s gap, but
builtInChartsis still a second source of truth. The next chart addition can drift again unless the CLI reads from the canonicalapp/src/plugins/chart-types.tslist directly.♻️ Suggested change
- // Keep in sync with app/src/plugins/chart-types.ts - const builtInCharts = [ - "bar", - "line", - "pie", - "table", - "single-value", - "graph", - "map", - "json", - "parameter-select", - "form", - "markdown", - "iframe", - "gauge", - "sankey", - "sunburst", - "radar", - "treemap", - "gantt", - "circle-packing", - "choropleth", - ]; + import { CHART_TYPES } from "../../app/src/plugins/chart-types.js"; + const builtInCharts = [...CHART_TYPES];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/plugin.ts` around lines 137 - 159, Replace the duplicated builtInCharts array with a single import of the canonical chart types export from the chart-types module and use that exported list wherever builtInCharts is referenced; specifically remove the local builtInCharts constant and import the exported chart-types value (e.g., exported const chartTypes) and assign or re-export it so functions that reference builtInCharts continue to work, and add a small runtime check (throw or log) if the imported list is missing or not an array to fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cli/src/commands/plugin.ts`:
- Around line 137-159: Replace the duplicated builtInCharts array with a single
import of the canonical chart types export from the chart-types module and use
that exported list wherever builtInCharts is referenced; specifically remove the
local builtInCharts constant and import the exported chart-types value (e.g.,
exported const chartTypes) and assign or re-export it so functions that
reference builtInCharts continue to work, and add a small runtime check (throw
or log) if the imported list is missing or not an array to fail fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 30221794-36cd-4c55-b260-f11f415ecca5
📒 Files selected for processing (1)
cli/src/commands/plugin.ts
|



Closes #662
Summary
circle-packingandchoroplethto the hardcodedbuiltInChartsarray incli/src/commands/plugin.tsapp/src/plugins/chart-types.tsTest plan
npx tsc --noEmit -p cli/tsconfig.jsonpasses🤖 Generated with Claude Code
Summary by CodeRabbit