feat: support SVG renderer in ECharts charts - #38
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughECharts now supports Canvas and SVG renderers through shared configuration. Eight chart components accept a ChangesECharts SVG renderer support
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🟡 Moderate · up to The PR is not merge-ready until the reported lint errors in the changed chart components are fixed or explicitly accepted; otherwise the renderer update carries a concrete validation and maintenance risk. The remaining radar documentation typo is minor and non-blocking. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2770f1ac-184c-43a3-bc8c-4ebec9e6e657
📒 Files selected for processing (34)
AGENTS.mdregistry.jsonsrc/content/docs/echarts/area-chart/static.mdxsrc/content/docs/echarts/bar-chart/static.mdxsrc/content/docs/echarts/composed-chart/static.mdxsrc/content/docs/echarts/installation.mdxsrc/content/docs/echarts/line-chart/static.mdxsrc/content/docs/echarts/pie-chart/static.mdxsrc/content/docs/echarts/radar-chart/static.mdxsrc/content/docs/echarts/radial-chart/static.mdxsrc/content/docs/echarts/sankey-chart/static.mdxsrc/content/docs/echarts/ui/brush.mdxsrc/content/docs/index.mdxsrc/globals/constants/providers.tssrc/registry/__index__.tsxsrc/registry/charts/echarts-area-chart.tsxsrc/registry/charts/echarts-bar-chart.tsxsrc/registry/charts/echarts-composed-chart.tsxsrc/registry/charts/echarts-line-chart.tsxsrc/registry/charts/echarts-pie-chart.tsxsrc/registry/charts/echarts-radar-chart.tsxsrc/registry/charts/echarts-radial-chart.tsxsrc/registry/charts/echarts-sankey-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-area-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-bar-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-composed-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-line-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-pie-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-radar-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-radial-chart.tsxsrc/registry/examples/echarts/ex-svg-renderer-echarts-sankey-chart.tsxsrc/registry/registry-example.tssrc/registry/registry-ui.tssrc/registry/ui/echarts-chart.tsx
| ## API Reference | ||
|
|
||
| The radar chart is a root container plus composible parts. On canvas each part is declarative config the root compiles, but the API mirrors the Recharts twin. Each is documented below. | ||
| The radar chart is a root container plus composible parts. Regardless of renderer, each part is declarative config the root compiles, but the API mirrors the Recharts twin. Each is documented below. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the API overview spelling.
Line 219 uses composible. Replace it with composable.
| raf = requestAnimationFrame(tick); | ||
| return () => cancelAnimationFrame(raf); | ||
| }, [live, isLoading, loadingPoints, loadingData]); | ||
| }, [renderer, live, isLoading, loadingPoints, loadingData]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
src/registry/charts/echarts-area-chart.tsx \
src/registry/charts/echarts-bar-chart.tsx \
src/registry/charts/echarts-composed-chart.tsx \
src/registry/charts/echarts-line-chart.tsx \
src/registry/charts/echarts-radar-chart.tsx
do
echo "===== $f ====="
rg -n -C 14 'const live|live\s*=|useRef|useEffect|loadingPoints|loadingBars|loadingLineData|loadingData|renderer' "$f" | tail -n 220
doneRepository: legions-developer/evilcharts
Length of output: 45913
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
src/registry/charts/echarts-area-chart.tsx \
src/registry/charts/echarts-bar-chart.tsx \
src/registry/charts/echarts-composed-chart.tsx \
src/registry/charts/echarts-line-chart.tsx \
src/registry/charts/echarts-radar-chart.tsx
do
echo "===== $f: ref declarations and nearby dependencies ====="
rg -n -C 8 'useRef<|useRef\(|\.current\)|const live|let live|react-hooks/refs|exhaustive-deps' "$f" | head -n 180
done
echo "===== lint configuration and React Hooks versions ====="
git ls-files | rg '(^|/)(package\.json|eslint[^/]*|pnpm-lock\.yaml|yarn\.lock|package-lock\.json|bun\.lockb?)$' | sort
rg -n -C 3 'eslint-plugin-react-hooks|react-hooks|eslintConfig|flatConfig|recommended' \
package.json .eslintrc* eslint.config.* 2>/dev/null || trueRepository: legions-developer/evilcharts
Length of output: 10508
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
src/registry/charts/echarts-area-chart.tsx \
src/registry/charts/echarts-bar-chart.tsx \
src/registry/charts/echarts-composed-chart.tsx \
src/registry/charts/echarts-line-chart.tsx \
src/registry/charts/echarts-radar-chart.tsx
do
echo "===== $f: complete live initialization ====="
start=$(rg -n 'const live = useRef<LiveState>' "$f" | cut -d: -f1)
end=$((start + 145))
sed -n "${start},${end}p" "$f" | nl -ba -v "$start"
done
echo "===== package and lint configuration ====="
cat package.json
cat eslint.config.mjsRepository: legions-developer/evilcharts
Length of output: 292
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
src/registry/charts/echarts-area-chart.tsx \
src/registry/charts/echarts-bar-chart.tsx \
src/registry/charts/echarts-composed-chart.tsx \
src/registry/charts/echarts-line-chart.tsx \
src/registry/charts/echarts-radar-chart.tsx
do
echo "===== $f: complete live initialization ====="
start=$(rg -n 'const live = useRef<LiveState>' "$f" | cut -d: -f1)
end=$((start + 145))
sed -n "${start},${end}p" "$f"
done
echo "===== package and lint configuration ====="
cat package.json
cat eslint.config.mjsRepository: legions-developer/evilcharts
Length of output: 28569
🌐 Web query:
React eslint-plugin-react-hooks react-hooks/refs rule useRef current during render dependency array official documentation
💡 Result:
According to official React documentation and the eslint-plugin-react-hooks rules, you should not include ref.current in a dependency array [1][2]. There are several authoritative technical reasons for this, centered on how React manages state and effects [3][2]. 1. Why you should not use ref.current as a dependency Mutable values like ref.current are not "reactive" [2]. In React, dependency arrays are intended to track values that, when changed, should trigger a re-render or a re-synchronization of an effect [4][2]. Because changing ref.current does not trigger a re-render of your component, React has no way of knowing when the value has changed [3][2]. Consequently, even if you include it in a dependency array, the effect will not reliably re-run when the ref is mutated [1][2]. 2. Reading or writing refs during render Official React documentation explicitly warns against reading or writing ref.current during the rendering phase [5][3]. Doing so makes your component's behavior unpredictable because React does not track changes to refs [3]. The only common exception is using a ref for lazy initialization during the first render (e.g., if (!ref.current) ref.current = new Thing) [5][3]. Generally, you should only access refs inside event handlers or Effects, which run after the commit phase [5][6]. 3. Proper alternatives If you need an effect to run in response to a value change, that value must be part of React's state management system (e.g., useState or useReducer) [1][7]. Using state ensures that updates trigger a re-render, making the data reactive and correctly observable by your effects [1][2]. If your goal is to be notified when a DOM node is attached or detached, you should use a callback ref instead of useRef [8][7]. In summary, the exhaustive-deps linting rule correctly flags ref.current because it is a mutable property that does not participate in the React data flow, and using it as a dependency creates a false expectation that your effect will respond to its changes [1][8][2].
Citations:
- 1: https://www.epicreact.dev/why-you-shouldnt-put-refs-in-a-dependency-array
- 2: https://react.dev/learn/lifecycle-of-reactive-effects
- 3: https://react.dev/learn/referencing-values-with-refs
- 4: https://react.dev/reference/react/useEffect
- 5: https://react.dev/reference/react/useRef
- 6: https://react.dev/learn/manipulating-the-dom-with-refs
- 7: useEffect will unpredictable when depends on ref (useRef) react/react#16121
- 8: eslint-plugin-react-hooks: exhaustive-deps nagging behaviour react/react#23026
🏁 Script executed:
#!/bin/bash
set -eu
echo "===== lockfile entries for React Hooks lint packages ====="
rg -n -C 2 'eslint-plugin-react-hooks|react-hooks' bun.lock || true
echo "===== all render-time live accesses in the five files ====="
for f in \
src/registry/charts/echarts-area-chart.tsx \
src/registry/charts/echarts-bar-chart.tsx \
src/registry/charts/echarts-composed-chart.tsx \
src/registry/charts/echarts-line-chart.tsx \
src/registry/charts/echarts-radar-chart.tsx
do
echo "--- $f ---"
rg -n '(^|[^[:alnum:]_])live(\.|[,)])|useRef<LiveState>|\.current' "$f"
doneRepository: legions-developer/evilcharts
Length of output: 24571
🌐 Web query:
site:github.com/facebook/react eslint-plugin-react-hooks "Cannot access refs during render" "react-hooks/refs"
💡 Result:
The ESLint error "Cannot access refs during render" (associated with the rule react-hooks/refs in eslint-plugin-react-hooks) occurs because reading or writing the .current property of a ref directly inside the body of a React component can lead to stale UI states and unpredictable behavior [1][2][3]. React refs are intended to hold mutable values that persist across renders without triggering re-renders [1][4][3]. Because the render function must be pure and predictable, accessing a ref value during render violates these principles, as the value of ref.current can change independently of React's state management [1][3]. Key points regarding this rule: 1. When it is acceptable: Refs are safe to access outside of the render phase, specifically within event handlers (e.g., onClick, onChange) and effects (useEffect, useLayoutEffect) [1][3]. 2. Why it happens: The rule identifies when you are reading or writing to ref.current during the component execution phase [2][5]. This is enforced to ensure that your component's render output remains consistent and does not depend on mutable values that React cannot track [1][3]. 3. Troubleshooting false positives: If you are seeing this error in scenarios where you believe ref access is safe (e.g., passing a ref object through to another component, or using refs inside asynchronous browser callbacks like IntersectionObserver), this may be a known issue with the plugin's analysis logic [2][6][7][8]. - Ref Forwarding: Recent updates to the compiler and linting rules have addressed false positives when simply passing a ref prop down to a child component, provided you are not reading the .current value itself during render [8]. - Async Callbacks: If you use refs inside callbacks that are executed by the browser asynchronously (not during the render phase), and the linter incorrectly flags them, ensure you are using the latest version of eslint-plugin-react-hooks, as ongoing fixes (such as those for IntersectionObserver) are regularly released to improve detection accuracy [7]. If you are encountering a legitimate error, the standard fix is to move the logic that reads or writes the ref value into a useEffect hook or an event handler [1][3]. If you believe the error is a false positive, you can check the React repository issues for similar patterns or temporarily suppress the rule for specific lines using // eslint-disable-next-line react-hooks/refs after verifying that your usage does not actually occur during render [1][6].
Citations:
- 1: Bug: eslint-react-hooks false positives on refs rule react/react#34775
- 2: Bug: react-hooks/refs false positive using IntersectionObserver react/react#35982
- 3: [Compiler Bug]: Compiler doesn't bail out when reading or writing
ref.currentduring a render react/react#29161 - 4: ESLint exhaustive-deps rule should be more consistent about 'Ref' values react/react#17404
- 5: [Compiler]: Ref values (the
currentproperty) may not be accessed during render - showing error message for custom hooks react/react#30745 - 6: [Compiler Bug]: react-hooks/refs false positive on ref forwarding react/react#34954
- 7: Fix: react-hooks/refs false positive with IntersectionObserver in useMemo react/react#36125
- 8: [compiler] Fix refs validation for forwarded ref props react/react#36208
🌐 Web query:
site:github.com/facebook/react packages/eslint-plugin-react-hooks refs rule "Cannot access ref value during render"
💡 Result:
The eslint-plugin-react-hooks rule react-hooks/refs is a validation check designed to enforce the architectural principle that React refs (the .current property) should not be read or written to during the render phase [1][2]. The core reason for this rule is that the render function should be pure and predictable [3][4]. Refs are mutable, and accessing or modifying them during render can cause side effects or lead to inconsistent component behavior, as the render phase may be interrupted, restarted, or run multiple times by React [1][3]. Key points regarding this rule: - Purpose: The rule specifically targets the current property of a ref [1][5]. According to React's design, refs are intended for values that do not affect the output of a component (e.g., storing a DOM element reference or a stable ID that persists across renders) [2]. - Exceptions: The only widely accepted use case for accessing a ref during render is the lazy initialization pattern, where a ref is used to lazily initialize and store an expensive object that is only set once [3][4]. - False Positives: As of recent versions (specifically since v7.0.0 of the plugin), many users have reported false positives [2][6][7]. These occur when the analysis tool incorrectly flags safe code—such as passing a ref to a function or storing refs within objects—as an "unsafe access" [2][6][7]. Development on the React compiler and the associated ESLint rules is ongoing, and these issues are actively tracked in the React repository [5][7]. If you are encountering this error and believe your code is safe, it may be due to a known false positive in the current version of the plugin [2][7]. Developers often choose to either suppress the warning if they are certain of the pattern's safety or refactor code to ensure refs are only accessed within useEffect or event handlers, which are the idiomatic locations for ref interaction [8][2].
Citations:
- 1: https://github.com/facebook/react/blob/e71a6393/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts
- 2: Bug: eslint-react-hooks false positives on refs rule react/react#34775
- 3: useRef: Warn about reading or writing mutable values during render react/react#18545
- 4: react/react@c59c3dfe5
- 5: [Compiler]: Ref values (the
currentproperty) may not be accessed during render - showing error message for custom hooks react/react#30745 - 6: Bug: react-hooks/refs false positive using IntersectionObserver react/react#35982
- 7: [eslint-plugin-react-hooks] Bug:
react-hooks/refsrule reporting false positive react/react#35813 - 8: [Compiler]: Ref values (the
currentproperty) may not be accessed during render. (eslint) react/react#31330
Refactor live into a ref object before merge.
All five files read useRef(...).current during render. The react-hooks/refs rule flags this initializer, so removing live from the loading-effect dependencies alone will not fix the lint errors. Keep the ref object stable, access .current only in effects and callbacks, and move render-time mutations such as live.handlers = ... out of render.
🧰 Tools
🪛 ESLint
[error] 2287-2287: Error: Cannot access refs during render
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
/home/jailuser/git/src/registry/charts/echarts-area-chart.tsx:2287:6
2285 | raf = requestAnimationFrame(tick);
2286 | return () => cancelAnimationFrame(raf);
2287 | }, [renderer, live, isLoading, loadingPoints, loadingData]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Cannot access ref value during render
2288 |
2289 | // ── Legend overlay position ──────────────────────────────────────────────────
2290 | // Insets match the Recharts legend's breathing room inside the plot frame.
(react-hooks/refs)
📍 Affects 5 files
src/registry/charts/echarts-area-chart.tsx#L2287-L2287(this comment)src/registry/charts/echarts-bar-chart.tsx#L2139-L2139src/registry/charts/echarts-composed-chart.tsx#L1973-L1973src/registry/charts/echarts-line-chart.tsx#L2040-L2040src/registry/charts/echarts-radar-chart.tsx#L1184-L1184
Source: Linters/SAST tools
| raf = requestAnimationFrame(tick); | ||
| return () => cancelAnimationFrame(raf); | ||
| }, [live, isLoading, loadingData]); | ||
| }, [live, isLoading, loadingData, renderer]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate file outline ---'
ast-grep outline src/registry/charts/echarts-radial-chart.tsx
printf '%s\n' '--- relevant source ---'
sed -n '980,1210p' src/registry/charts/echarts-radial-chart.tsx
printf '%s\n' '--- live declarations and effect references ---'
rg -n -C 4 '\blive\b|useEffect\(' src/registry/charts/echarts-radial-chart.tsx
printf '%s\n' '--- lint configuration and scripts ---'
rg -n -C 3 'eslint|react-hooks|Cannot access refs during render|lint' package.json .eslintrc* eslint.config.* 2>/dev/null || trueRepository: legions-developer/evilcharts
Length of output: 23841
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- live state declaration and hooks ---'
sed -n '850,1040p' src/registry/charts/echarts-radial-chart.tsx
printf '%s\n' '--- imports and React type versions ---'
sed -n '1,75p' src/registry/charts/echarts-radial-chart.tsx
rg -n '"react"|"eslint-plugin-react-hooks"|eslint-config-next' package.json bun.lockb bun.lock package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
printf '%s\n' '--- all useRef declarations and dependency arrays ---'
rg -n -C 2 'useRef|^\s*\],?$|^\s*\}, \[' src/registry/charts/echarts-radial-chart.tsx
printf '%s\n' '--- installed lint tooling, if present (no project code execution) ---'
if [ -x node_modules/.bin/eslint ]; then
node_modules/.bin/eslint --version
node_modules/.bin/eslint --print-config src/registry/charts/echarts-radial-chart.tsx \
| rg -n -C 2 'react-hooks/(exhaustive-deps|refs)|Cannot access refs'
else
echo 'node_modules/.bin/eslint is unavailable'
fiRepository: legions-developer/evilcharts
Length of output: 49447
🏁 Script executed:
#!/bin/bash
set -u
node_modules/.bin/eslint src/registry/charts/echarts-radial-chart.tsx
status=$?
printf '\nESLint exit status: %s\n' "$status"
exit "$status"Repository: legions-developer/evilcharts
Length of output: 9735
Remove live from both effect dependency arrays and avoid reading .current during render.
live is a stable useRef(...).current value, not a reactive dependency. The react-hooks/refs rule reports the sync effect at line 1137 and the shimmer effect at line 1192. It also reports the .current access at line 915, so removing only the dependency at line 1192 does not resolve lint failures.
🧰 Tools
🪛 ESLint
[error] 1192-1192: Error: Cannot access refs during render
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
/home/jailuser/git/src/registry/charts/echarts-radial-chart.tsx:1192:6
1190 | raf = requestAnimationFrame(tick);
1191 | return () => cancelAnimationFrame(raf);
1192 | }, [live, isLoading, loadingData, renderer]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Cannot access ref value during render
1193 |
1194 | // ── Legend overlay (HTML) ─────────────────────────────────────────────────────
1195 | // One entry per ring. Unlike the area twin's absolutely-positioned legend, the
(react-hooks/refs)
Source: Linters/SAST tools
1 participant
Summary
renderer="svg"line in every example's code previewValidation
bunx eslinton the touched TypeScript and TSX filesbunx tsc --noEmit --incremental falsebun run registry:fresh(279 registry items)bun run build(89 static pages)