LabelList uses React context instead of reading DOM by PavelVanecek · Pull Request #6246 · recharts/recharts

PavelVanecek · 2025-08-24T03:44:43Z

Description

Switched LabelList from renderCallByParent (which was reading DOM nodes and parsing props from them) into React context.

LabelList can now be nested arbitrarily, no need to be direct child
Few bugfixes in the animation flow, and in passing props

Some visual diff in Storybook:

Funnel labels moved a few pixels - looks alright to me? Should I dig into it?
The other Funnel story had explicit fill=black in it, the labels were grey, now they are black, so I consider that a bugfix.

Related Issue

Fixes #6215
#5768

Motivation and Context

Removed one more usage of findChildrenByType - now only Cell is remaining, and then we can delete react-is dependency completely.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

PavelVanecek · 2025-08-24T03:45:52Z

...shots__/tests/LabelList.spec-vr.tsx-snapshots/Funnel-explicit-LabelList-1-chromium-linux.png

Now uses fill color instead of stroke, to be consistent with other chart behaviour. We can change this back if we want.

PavelVanecek · 2025-08-24T03:46:54Z

...apshots__/tests/LabelList.spec-vr.tsx-snapshots/Line-explicit-LabelList-1-chromium-linux.png

Order changed, now labels are below the dot. We can change this with Portals but perhaps this is fine - I assume everyone will choose different positions to prevent overlapping labels anyway?

PavelVanecek · 2025-08-24T03:47:04Z

...napshots__/tests/LabelList.spec-vr.tsx-snapshots/Pie-explicit-LabelList-1-chromium-linux.png

#6215 is fixed!

PavelVanecek · 2025-08-24T03:48:27Z

...napshots__/tests/LabelList.spec-vr.tsx-snapshots/Pie-implicit-LabelList-1-chromium-linux.png

Pie label has special behaviour, with the little label line on each label. This is the same in this PR (and recorded in multiple storybook snapshots).

If a label however has explicit position set then the LabelList takes over which is what we are seeing here.

Some labels render inside but because they inherit the same fill color as the Pie we can't see them.

PavelVanecek · 2025-08-24T03:48:40Z

test-vr/tests/LabelListComponents.tsx

        dataKey="uv"
        fill="none"
        stroke="green"
-        // @ts-expect-error typescript type says that valueAccessor is not a valid prop, but it does work


Fixed type bug

PavelVanecek · 2025-08-24T03:49:01Z

test/cartesian/Area.animation.spec.tsx


 const expectedUvLabels: ReadonlyArray<ExpectedLabel> = [
  {
-    height: null,


width and height don't do anything in text SVG element, we should remove them.

PavelVanecek · 2025-08-24T03:49:32Z

test/cartesian/Area.animation.spec.tsx

    it('should hide labels during the animation', async () => {
      const { container, animationManager } = renderTestCase();

-      return expectLabelsRemainVisible(container, animationManager);


This test was wrong - expects labels to hide, but tests that they are visible? Fixed.

PavelVanecek · 2025-08-24T03:49:42Z

test/cartesian/Area.spec.tsx

      };

-      render(
+      const { debug } = render(


I should remove this.

PavelVanecek · 2025-08-24T03:49:55Z

test/cartesian/Line.animation.spec.tsx

        /*
         * The labels should still be hidden! But all labels now appear again.
         * Why? Because the animation is not started yet, but they swap to new label values immediately.
-         * Unfortunately the labels still show at the old position so we have a flash of new labels at the old position.


Fixed a bug

codecov · 2025-08-24T03:52:19Z

Codecov Report

❌ Patch coverage is 94.70014% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.61%. Comparing base (71521bc) to head (ccd59b5).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/cartesian/Area.tsx	89.18%	12 Missing ⚠️
src/state/errorBarSlice.ts	16.66%	10 Missing ⚠️
src/cartesian/Line.tsx	91.00%	9 Missing ⚠️
src/context/ErrorBarContext.tsx	82.35%	3 Missing ⚠️
src/cartesian/Bar.tsx	97.56%	2 Missing ⚠️
src/polar/Pie.tsx	98.27%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6246      +/-   ##
==========================================
- Coverage   96.63%   96.61%   -0.03%     
==========================================
  Files         221      221              
  Lines       19898    20125     +227     
  Branches     4093     4110      +17     
==========================================
+ Hits        19229    19443     +214     
- Misses        663      675      +12     
- Partials        6        7       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov · 2025-08-24T03:52:40Z

Bundle Report

Changes will increase total bundle size by 18.66kB (0.78%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
recharts/bundle-cjs	1.04MB	8.47kB (0.82%) ⬆️
recharts/bundle-es6	892.77kB	8.59kB (0.97%) ⬆️
recharts/bundle-umd	485.54kB	1.6kB (0.33%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`cartesian/Area.js`	742 bytes	25.7kB	2.97%
`cartesian/Line.js`	1.07kB	23.35kB	4.79%
`cartesian/Bar.js`	782 bytes	22.43kB	3.61%
`polar/Pie.js`	1.55kB	22.25kB	7.49% ⚠️
`cartesian/Scatter.js`	1.59kB	20.49kB	8.43% ⚠️
`polar/RadialBar.js`	836 bytes	18.1kB	4.84%
`cartesian/Funnel.js`	1.04kB	17.17kB	6.45% ⚠️
`polar/Radar.js`	1.49kB	16.19kB	10.16% ⚠️
`component/Label.js`	-1.1kB	15.84kB	-6.52%
`component/LabelList.js`	-222 bytes	5.06kB	-4.2%
`context/ErrorBarContext.js`	502 bytes	2.44kB	25.96% ⚠️
`state/errorBarSlice.js`	305 bytes	1.35kB	29.16% ⚠️

view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`cartesian/Area.js`	705 bytes	27.44kB	2.64%
`cartesian/Line.js`	1.03kB	24.81kB	4.35%
`polar/Pie.js`	1.54kB	23.9kB	6.89% ⚠️
`cartesian/Bar.js`	745 bytes	23.89kB	3.22%
`cartesian/Scatter.js`	1.6kB	21.98kB	7.86% ⚠️
`polar/RadialBar.js`	803 bytes	19.38kB	4.32%
`cartesian/Funnel.js`	1.02kB	18.66kB	5.76% ⚠️
`polar/Radar.js`	1.46kB	17.6kB	9.03% ⚠️
`component/Label.js`	-1.32kB	17.48kB	-7.01%
`component/LabelList.js`	-5 bytes	6.26kB	-0.08%
`context/ErrorBarContext.js`	521 bytes	3.43kB	17.9% ⚠️
`state/errorBarSlice.js`	374 bytes	1.68kB	28.68% ⚠️

view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`Recharts.js`	1.6kB	485.54kB	0.33%

ckifer

nice

ckifer · 2025-08-25T05:04:58Z

src/cartesian/Line.tsx

+        payload: point.payload,
+        viewBox,
+        /*
+         * Line is not passing parentViewBox to the LabelList so the labels can escape - looks like a bug, should we pass parentViewBox?


Does anything bad happen if we do?

If we add parent view box then that should push labels back into the viewbox if they are peeking out. I haven't tested it though.

ckifer · 2025-08-25T05:07:08Z

src/cartesian/Scatter.tsx

  );
 }

+function ScatterLabelListProvider({


I'm not sure I like that labels are now below points for scatter. Feel like words should always render on top even if both are bad. People like to render large z scatter items and then put text in those

Also why is it below for scatter but not for other charts?

Because they are rendered together with error bars and error bars are better looking under points.

Proper fix would be portals, then we could have error bars below and labels on top.

Different default for label positions would help too.

Ah yeah. Another thing I suppose

ckifer · 2025-08-25T05:10:43Z

src/polar/Radar.tsx

+   * Radar provides a Cartesian label list context. Do we want to also provide a polar label list context?
+   * That way, users can choose to use polar positions for the Radar labels.
+   */
+  // const labelListEntries: ReadonlyArray<PolarLabelListEntry> = points.map(


I think that Radar being a polar chart should also support polar label positions, so I propose to pass both viewboxes and then use one or the other depending on the position? I didn't do this here because the PR is already large enough.

Sounds good

ckifer · 2025-08-25T05:11:33Z

src/state/errorBarSlice.ts

      }
      state[itemId].push(errorBar);
    },
+    replaceErrorBar: (


did adding this do something for labels or fix something else?

Fixed infinite rerender loop. Error bars and label lists are in the same {children}

PavelVanecek added 8 commits August 24, 2025 12:40

LabelList uses context instead of cloning

f8b6917

Remove LabelList.renderCallByParent and Label.parseViewBox

8e37545

Fix types

42ff9c3

Fix Funnel label width calculation

dce2d34

Move children so that error bars render below graphical items, not above

ab14d5f

Fix: LabelList inherits fill color from entries, if available

3f9f2d3

Update VR snapshots

fc25d06

Remove debug useWhyDidYouRender hook from ErrorBarContext

ccd59b5

Copilot AI review requested due to automatic review settings August 24, 2025 03:44

PavelVanecek commented Aug 24, 2025

View reviewed changes

ckifer approved these changes Aug 25, 2025

View reviewed changes

ckifer merged commit 0daea65 into main Aug 25, 2025
23 of 27 checks passed

PavelVanecek deleted the labellist-context branch August 25, 2025 09:30

Uh oh!

Conversation

PavelVanecek commented Aug 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

Types of changes

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov bot commented Aug 24, 2025

Bundle Report

Affected Assets, Files, and Routes:

Assets Changed:

Assets Changed:

Assets Changed:

Uh oh!

ckifer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PavelVanecek Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PavelVanecek commented Aug 24, 2025 •

edited

Loading

codecov bot commented Aug 24, 2025 •

edited

Loading

PavelVanecek Aug 25, 2025 •

edited

Loading