Skip to content

feat: add ECharts integration for advanced chart rendering - #21

Open
jordanking211 wants to merge 1 commit into
omdsh-dev:mainfrom
jordanking211:feat/echarts-support
Open

feat: add ECharts integration for advanced chart rendering#21
jordanking211 wants to merge 1 commit into
omdsh-dev:mainfrom
jordanking211:feat/echarts-support

Conversation

@jordanking211

Copy link
Copy Markdown

Add a new 'echart' node type that renders full ECharts charts with theme-aware colors, tooltips, and legends.

Two modes:

  • Preset shorthand: 'preset: bar|line|area|pie|scatter' + 'data'/'series' for quick upgrade from the 'chart' node
  • Full option: 'option' field for custom chart types (radar, combo, heatmap, dataZoom, visualMap, etc.)

Implementation:

  • EChartNode.tsx: React component with ResizeObserver for responsive charts
  • echarts-lazy.ts: on-demand engine loader (echarts ~1MB never enters main bundle)
  • asset-echarts.ts: standalone asset bundle registered on window.GenuiAssets
  • spec.ts: GenuiEChart type with preset/option/data/series fields
  • guard.ts: spec validation for echart nodes (option depth cap, preset whitelist)
  • render-node.tsx: wire echart into the component dispatcher
  • GenuiBlock.module.css: chart container, title, loading and fallback styles
  • tsdown.config.ts: add echarts asset build target
  • plugin/index.ts: register echarts asset route

Docs:

  • README.md / README.zh-CN.md: ECharts feature description, example, FAQ update
  • SKILL.md: echart node syntax documentation

The echarts engine loads lazily via the plugin's HTTP asset route only when an 'echart' node appears in a spec. Conversations without echart nodes never download it.

Add a new 'echart' node type that renders full ECharts charts with theme-aware colors, tooltips, and legends.

Two modes:
- Preset shorthand: 'preset: bar|line|area|pie|scatter' + 'data'/'series' for quick upgrade from the 'chart' node
- Full option: 'option' field for custom chart types (radar, combo, heatmap, dataZoom, visualMap, etc.)

Implementation:
- EChartNode.tsx: React component with ResizeObserver for responsive charts
- echarts-lazy.ts: on-demand engine loader (echarts ~1MB never enters main bundle)
- asset-echarts.ts: standalone asset bundle registered on window.__GenuiAssets__
- spec.ts: GenuiEChart type with preset/option/data/series fields
- guard.ts: spec validation for echart nodes (option depth cap, preset whitelist)
- render-node.tsx: wire echart into the component dispatcher
- GenuiBlock.module.css: chart container, title, loading and fallback styles
- tsdown.config.ts: add echarts asset build target
- plugin/index.ts: register echarts asset route

Docs:
- README.md / README.zh-CN.md: ECharts feature description, example, FAQ update
- SKILL.md: echart node syntax documentation

The echarts engine loads lazily via the plugin's HTTP asset route only when an 'echart' node appears in a spec. Conversations without echart nodes never download it.

@taekchef taekchef left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jordanking211 感谢这份 ECharts 集成——组件设计方向很好:preset 简写与 chart 节点同构(模型一行升级)、full option 逃生舱、按需懒加载 1MB 引擎不进主 bundle,这几个决策都符合本仓库的组件哲学。本地我完整跑了 pnpm run check(typecheck + 全量测试 + tsdown 构建),构建可以通过,但当前版本还不能合,以下按优先级列出:

阻塞项(必须改)

1. 提交进仓的 lib/client.js 与 src 不一致,CI 必挂

本仓库 CI 有硬门:pnpm run checkgit diff --exit-code -- lib/。我在你的分支上跑完 check 后,lib/client.js 相对你提交的版本有改动(新构建产物与提交版不同)。请 pnpm run build 后把全部 lib 变更一起提交;另请确认 lib/types/** 也已同步。

2. full option 直传存在 XSS 面(安全红线)

本仓库的安全姿态是「模型只能产出围栏文本,不能执行任意代码/DOM」。但 sanitizeEChartOption 保留了任意字符串,而 ECharts 默认 tooltip.renderMode:'html' 会把 formatter/数据名按 innerHTML 写入 tooltip DOM(我核对了 echarts/lib/component/tooltip/TooltipHTMLContent.jsel.innerHTML = content)。模型(或被 prompt 注入的模型)输出 {"type":"echart","option":{"tooltip":{"formatter":"<img src=x onerror=...>"},...}} 即可在用户浏览器执行脚本。建议二选一:

  • 对 full option 强制覆盖 tooltip.renderMode: 'richText'(该分支走文本渲染,不落 innerHTML);或
  • sanitizeEChartOption 里对所有字符串做 HTML 危险内容过滤(<scripton[a-z]+=javascript:<img 等),并给 data 名等字段统一入口。
    preset 路径自己构造 option,不受影响,但要保证用户无法通过 data[].label 把 HTML 带进 tooltip(label 字符串会进 tooltip 模板——richText 或过滤同样适用)。

3. 懒加载期间 spec 更新会渲染旧数据

EChartNode 第一个 effect(空依赖)在挂载时用当次的 node.option 发起 lazyCreateChart;流式渲染里模型会边写边更新 spec。如果引擎还在加载时 node 变了,then 回调 setOption 用的是旧 option,而第三个 update effect 在 status !== 'ready' 时直接 return,之后 status 变为 ready 也不会重跑(deps 只有 [node])→ 图表永远停在旧数据。修法:update effect 依赖加 status,或 ready 后用 ref 取最新 node 再 setOption 一次。

4. 没有任何测试

本仓库每个组件 PR 都带回归测试(chart/plot/mermaid/scene3d 均有 guard + jsdom 渲染测试)。请至少补:

  • tests/genui-echart-guard.spec.ts:preset 白名单、height 100–800、option 深度上限、函数/url() 过滤、非法节点拒绝、大数组预算;
  • tests/genui-echart.spec.tsx:preset 五种形态渲染出 data-genui-echart 容器、error fallback、option 优先于 preset、标题与高度。

5. 版本号 + CHANGELOG

仓库惯例:每个要发布的 PR 都动版本并写 changelog。新增组件建议 0.9.0,CHANGELOG 顶部加 [0.9.0] 条目(新增 echart 组件、preset/option 双模式、懒加载资产、guard 预算)。

需要修复的正确性/健壮性问题

6. preset: 'scatter' 数据映射错误

data.map(d => [d.label, d.value]) 把字符串 label 放进 type:'value' 的 xAxis——非数字 label(如「一月」)在 value 轴上画不出来。要么 xAxis 用 category + 按索引配对,要么改为 [index, value] 并把 label 显示交给 tooltip 的 axis 维度;请顺手补一个带中文 label 的 scatter 测试钉住。

7. full option 绕过了节点/数组预算

sanitizeEChartOption 只限深度和单字符串长度,不限制数组长度与总条目数;模型可以给 series.data 塞几十万点卡住渲染。请加数组长度上限 + 总节点预算(与 GENUI_LIMITS 的 200 节点/深度 8 精神一致,比如数组 ≤ 500、总遍历条目有界)。

8. tsdown.config.tsassetConfig 签名没放宽

assetConfig(name: 'mermaid' | 'three', …) 的联合类型没有加 'echarts'(该文件不进 tsc -p tsconfig.json 所以现在没报错,但这是一个等着咬人的坑)。顺带把 asset-loader.ts@param name 注释同步成三资产。

9. scripts/verify-pack.mjs 的必查列表缺 echarts

required 里只列了 lib/assets/mermaid.jslib/assets/three.js。新增的 lib/assets/echarts.js 若不进列表,打包检查会漏掉「echarts 资产缺失」的发布事故(issue #15 刚立下的规矩)。

非阻塞小观察

  • 画布无 role="img" / aria-label(plot 组件是有的),无障碍语义缺失。
  • preset 路径忽略了 series[].colorchart 节点是尊重的),建议对齐或文档说明。
  • optiontitle 同时给时,组件外层标题和 ECharts 内部 option.title 会重复——文档里说明一句即可。
  • error fallback 的文案「ECharts 渲染失败」没有可诊断信息;asset 404 / 引擎注册失败 / option 异常三类失败建议分别给一次 console.warn(本仓库「静默失败必须可观测」的约定)。

以上 1–9 补齐后我再跑一遍全量,随时可以合。方向没问题,感谢贡献 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants