chore(deps): migrate recharts to v3 across all three workspaces - #9878
Merged
Conversation
The issue named apps/loopover-ui, but THREE workspaces depended on recharts -- apps/loopover-ui, apps/loopover-miner-ui, and the published @loopover/ui-kit. Bumping only one would have left two majors resolved in the same tree, which is a worse problem than the deprecation warning: two copies bundled, and ui-kit's charts drifting from the app's. Three real v3 breaks, each fixed by reading the library's own types rather than re-stating them locally: - Line's `dot` render prop: v3 widened `key` from string to React's Key, which the hand-written param annotation silently disagreed with. The props are derived from Line's own signature now. - Tooltip/Legend content: v3 moved `payload`/`label` off the components' props (they are read from context) and publishes TooltipContentProps / LegendPayload for the render-prop shape. - A React key taken from `item.dataKey`, which v3 lets be a FUNCTION accessor. The correct string was already computed two lines above. The sparkline had no test at all, which is a poor place to be for a chart-library major: a type-clean upgrade that renders nothing looks exactly like a passing build. It has one now, asserting a line is actually drawn -- verified by mutation, which kills 3 of the 4 cases.
Contributor
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-29 18:37:59 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 3af8979 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 06:16 PM |
Bundle ReportChanges will decrease total bundle size by 93.56kB (-1.18%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9878 +/- ##
=======================================
Coverage 91.75% 91.75%
=======================================
Files 919 919
Lines 112987 112987
Branches 27193 27193
=======================================
Hits 103667 103667
Misses 8034 8034
Partials 1286 1286
Flags with carried forward coverage won't be shown. Click here to find out more. |
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #8610.
Scope correction
The issue described recharts as "a direct UI dependency (
apps/loopover-ui/package.json)". It is actually declared in three workspaces:apps/loopover-ui^2.15.4^3.10.1apps/loopover-miner-ui^2.15.4^3.10.1packages/loopover-ui-kit(published)^2.15.4^3.10.1Bumping only the one named would have left two majors resolved in the same tree — a worse problem than the warning it fixes: two copies bundled, and the published ui-kit's chart primitives drifting from the app's. The lockfile now resolves exactly one version.
Three real v3 breaks
Each fixed by reading the library's own exported types rather than restating them locally, so the next such change is a compile error instead of silent drift:
Line'sdotrender prop (sparkline.tsx) — v3 widenedkeyfromstringto React'sKey, which the hand-written parameter annotation silently disagreed with. The props are derived fromLine's own signature now.Tooltip/Legendcontent (ui-kit/chart.tsx) — v3 movedpayloadandlabeloff the components' props (they are read from context) and publishesTooltipContentProps/LegendPayloadfor the render-prop shape.item.dataKey— v3 letsdataKeybe a function accessor, which is not a valid key. The correct string was already computed two lines above and is used instead.The sparkline had no test
Which is a poor place to be for a chart-library major bump: a type-clean upgrade that renders nothing looks exactly like a passing build. It has one now, asserting a line path is actually drawn, that an all-null series renders nothing rather than a flat line at zero, and that a partial-null series still renders (the
connectNulls={false}gap is the "insufficient data" signal).ResponsiveContainermeasures its parent and jsdom reports 0×0 for everything, so the component's own fixed pixel box is stubbed in the harness — without that recharts correctly draws nothing and every assertion would pass vacuously. Verified by mutation: forcing the early return kills 3 of the 4 cases.Validation
ui:typecheckclean,ui:lint0 errors,ui:buildand the miner-UI build clean.test:ui-kit-packclean (161 files) — the published package still packs.npm audit --audit-level=moderate: 0 vulnerabilities.npm ci. The two@esbuild-kit/*warnings that remain are Trace and replace the @esbuild-kit/* transitive dependency (merged into tsx upstream) #8609's, which is blocked upstream — analysis posted there.No visual evidence table: this is a library migration with no intended visual change, and the sparkline is a 64×28px axis-less glyph. The new tests assert the render output directly instead.