[charts] Remove react spring #17123

bernardobelchior · 2025-03-25T07:38:52Z

Remove react-spring and replace it with CSS transitions or our useAnimate hook.

Changelog

Removes react-spring as a dependency of @mui/x-charts. A consequence of this change is that the props of some slots have been changed because the SpringValue wrapper has been removed. The affected slots and props are:

the type of the x, y, width and height props of the bar slot are now number;
the type of startAngle, endAngle, innerRadius, outerRadius, arcLabelRadius, cornerRadius and paddingAngle props of pieArcLabel slot are now number.

bernardobelchior · 2025-03-25T07:39:51Z

packages/x-charts/src/internals/animation/Transition.tsx

+    const time = now() - this.elapsed;
+
+    this.timer = timer((elapsed) => this.timerCallback(elapsed), 0, time);
+    timerFlush();


Without timerFlush():

Screen.Recording.2025-03-25.at.08.37.47.mov

With timerFlush():

Screen.Recording.2025-03-25.at.08.38.02.mov

mui-bot · 2025-03-25T07:43:11Z

Deploy preview: https://deploy-preview-17123--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against 2007381

github-actions · 2025-03-26T16:09:52Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

bernardobelchior · 2025-03-26T17:16:25Z

packages/x-charts/src/BarChart/BarPlot.tsx

+            xOrigin: xScale(0) ?? 0,
+            yOrigin: yScale(0) ?? 0,
            height: verticalLayout ? maxValueCoord - minValueCoord : barWidth,


Not sure how to handle this. Technically, defaulting this to 0 is wrong, but it's also wrong to assert as non-undefined because it can be.

To have proper types, I suppose we'd need to change the return value depending on the layout.

Open to suggestions.

const xOrigin = xScale(0) if (!xOrigin) throw 💣

🤷

I think defaulting to 0 is reasonable enough, even though it could create issues, it shouldn't happen

If we throw our tests will start failing. The thing is that we don't use the value in the cases where it doesn't make sense, so there's no issue. If we throw, we'll break our tests.

bernardobelchior · 2025-03-27T10:51:06Z

docs/data/charts/components/JSAnimationCustomization.tsx

+    yOrigin,
+    y,
+    width,
+    height: -8,


This feels a bit hacky because it requires knowledge of the implementation details of useAnimateBarLabel.

I wonder if we should expose useAnimate after all (or maybe a higher-level version of it?)

That's weird. Is it just to specify the initial value that you need this height: -8

If yes it might be easier to let user specify their own initial values.

Wy not having also the default initial height adapting to the layout like you do for x and y.

const initialProps = { ..., height: props.layout === 'vertical' ? 0 : props.height

And if the -8 is here to hide the element, maybe the opacity could be an extra label to animate

Is it just to specify the initial value that you need this height: -8

It's not about the initial value, it's about the end value actually. I've since updated this code, but it's still a bit weird.

By default we render the bar labels in the middle of the bar. In this demo I'm moving them to the top of the bar and keeping the animation, so that's why I need this hack.

If yes it might be easier to let user specify their own initial values.

That's another use case, but yeah, I'm considering exporting a slightly higher level version of useAnimate.

I'll try to get a grasp of how something like that would look like.

I got confused because the y-axis goes from top to bottom 🙈

Maybe the solution for now is to not encourage users to create custom animation with those hook.

User want to create a custom component because they need to add extra element: They can reuse the hook

User want to customize the animation: They pick their own animation library

Created a higher-level useAnimateRef hook here.

We might want to expose that one so that users can customize the animations using our "framework".

We might want to expose that one so that users can customize the animations using our "framework".

I agree. My point was more about the timeline. This PR could have a docs with a simpler example. But I struggle to find an interesting one. I thought about a bar label with a rectangle behind it but it does seems as hacky as the current one because the useBarLabelAnimation will only animate the x and y.

It seems that without hack, those hook are only to reproduce the exact same behavior as default components.

So maybe in this PR just documenting that user can use those hook to create their own version. And in another PR we will discuss the DX and

So maybe in this PR just documenting that user can use those hook to create their own version. And in another PR we will discuss the DX and

Sure. So you're saying that we should ship this as is and then in a separate PR export a higher-level hook to be customizable by users, right?

codspeed-hq · 2025-03-27T12:12:59Z

CodSpeed Performance Report

Merging #17123 will improve performances by 58.76%

_{Comparing bernardobelchior:remove-react-spring (2007381) with master (347f7d6)}

Summary

⚡ 3 improvements
✅ 5 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`BarChart with big data amount`	502.8 ms	354 ms	+42.05%
⚡	`BarChartPro with big data amount`	751.1 ms	524.1 ms	+43.31%
⚡	`PieChart with big data amount`	131.8 ms	83 ms	+58.76%

alexfauquette

Looks good. All the comments are more thoughts I got while reading than requests for changes

alexfauquette · 2025-03-28T08:27:55Z

packages/x-charts/src/BarChart/BarLabel/BarLabelPlot.tsx

+            y={y}
+            width={width}
+            height={height}
+            skipAnimation={skipAnimation ?? false}


Suggested change

skipAnimation={skipAnimation ?? false}

skipAnimation={skipAnimation}

Usually false has the same behavior as undefined

Yeah, but the problem is that we're now exposing skipAnimation through slots, so it might not be clear to users what undefined means. IMO, we should always return either false or true.

Would it make sense to update useSkipAnimation to reflect that? I think so. If you agree, I'll do that in a separate PR.

Please check, but IIRC we used undefined|true because that was the ReactSpring api, now that we got rid of it, we can have undefined | boolean

Sorry, I didn't get that. What do you mean by it was the React Spring API? It seems we are the ones setting the type to true | undefined in line 17 of AnimationProvider.tsx.

Nevertheless, I wanted to know if we could just return boolean from useSkipAnimation, instead of boolean | undefined. Basically, I need to know: what's the difference between undefined and false?

Hum, then I'm not sure why. It could have been different behaviours between false vs undefined.

Returning boolean should be ok

alexfauquette · 2025-03-28T08:39:26Z

docs/data/charts/components/JSAnimationCustomization.tsx

+    yOrigin,
+    y,
+    width,
+    height: -8,


I got confused because the y-axis goes from top to bottom 🙈

Maybe the solution for now is to not encourage users to create custom animation with those hook.

User want to create a custom component because they need to add extra element: They can reuse the hook

User want to customize the animation: They pick their own animation library

docs/data/charts/components/components.md

alexfauquette · 2025-03-28T08:47:59Z

packages/x-charts/src/BarChart/BarElement.tsx

+    x: number;
+    xOrigin: number;
+    y: number;
+    yOrigin: number;
+    width: number;
+    height: number;
+    layout: 'horizontal' | 'vertical';
+    skipAnimation: boolean;


Maybe a BarCommonProps could help to define whihc porps are needed to draw an element (bar, barLabel, barClipPath)

Initially this looked like a good idea to me, but the comments on each property are slightly different. BarClipPath and BarElement aren't exported, so we don't need to document their properties.

BarProps and BarLabelProps are exported, so we should document those. However, the comments aren't the same for properties with the same name:

BarProps:

/** * The position of the bar in the x-axis. */ x: number; /** * The position of the bar in the y-axis. */ y: number;

BarLabelProps:

/** * Position in the x-axis of the bar this label belongs to. */ x: number; /** * Position in the y-axis of the bar this label belongs to. */

So, I suppose it makes sense that they're duplicated? What do you think?

I did not thougth about JSDocs. Then I'm ok to keep it as it is
👍

alexfauquette

Looks good. All the comments are more thoughts I got while reading than requests for changes

Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Signed-off-by: Bernardo Belchior <Bernardo.belchior1@gmail.com>

Co-authored-by: Jose C Quintas Jr <juniorquintas@gmail.com> Signed-off-by: Bernardo Belchior <Bernardo.belchior1@gmail.com>

JCQuintas · 2025-03-31T16:13:32Z

I thought it had something to do with react updates. Maybe merge master in again? The changes in this PR shouldn't affect the axes, so it could be something outside of this PR

It still happens but I just accepted the changes, the benefits are bigger than small text changes. It might be difference in render since we were probably using react:18 on docs prior to this pr.

bernardobelchior commented Mar 25, 2025

View reviewed changes

bernardobelchior force-pushed the remove-react-spring branch 7 times, most recently from 4960d5c to e95e201 Compare March 26, 2025 15:58

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 26, 2025

bernardobelchior force-pushed the remove-react-spring branch 2 times, most recently from ba4ee01 to b8c3085 Compare March 26, 2025 16:45

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 26, 2025

bernardobelchior commented Mar 26, 2025

View reviewed changes

bernardobelchior commented Mar 27, 2025

View reviewed changes

bernardobelchior changed the title ~~WIP: Remove react spring~~ Remove react spring Mar 27, 2025

bernardobelchior marked this pull request as ready for review March 27, 2025 12:03

bernardobelchior added enhancement This is not a bug, nor a new feature component: charts This is the name of the generic UI component, not the React module! labels Mar 27, 2025

bernardobelchior added this to the 8.0.0 milestone Mar 27, 2025

bernardobelchior requested review from alexfauquette and JCQuintas March 27, 2025 14:56

bernardobelchior changed the title ~~Remove react spring~~ [charts] Remove react spring Mar 27, 2025

alexfauquette reviewed Mar 28, 2025

View reviewed changes

bernardobelchior requested a review from alexfauquette March 28, 2025 09:39

bernardobelchior force-pushed the remove-react-spring branch from 1e0a071 to 6d5675e Compare March 28, 2025 10:40

bernardobelchior mentioned this pull request Mar 28, 2025

[charts] Expose higher level useAnimate hook #17162

Merged

bernardobelchior and others added 21 commits March 31, 2025 18:01

Animate border radius

f5c4db2

Fix lint and types

f1db081

Gen exports

3103652

Fix types and tests.

1315a77

Fix exports and types.

4abf30c

Fix more animations

c939568

Update docs

e8e2065

Fix bar animation. Document some props.

cac1efb

Gen docs. Remove useIsHydrated.

f3f7489

Fix test

568717d

Remove useIsHydrated

5baec9f

Remove react-spring types from tsconfig.build.json

521bfcb

Re-add pie arc label animation

46e3898

Animate pie arc label oapcity. Export animation hooks. Add docs.

85aa047

Run prettier

8df97b2

Improve docs

cc40cb4

Update docs/data/charts/components/components.md

7b85efd

Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Signed-off-by: Bernardo Belchior <Bernardo.belchior1@gmail.com>

Update docs

6e6f868

Update docs/data/charts/bars/bars.md

7038cc1

Co-authored-by: Jose C Quintas Jr <juniorquintas@gmail.com> Signed-off-by: Bernardo Belchior <Bernardo.belchior1@gmail.com>

Address comments

e99246f

Update docs

2007381

bernardobelchior force-pushed the remove-react-spring branch from 65e2a55 to 2007381 Compare March 31, 2025 16:01

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 31, 2025

This was referenced Mar 31, 2025

[charts][docs] LineChart stackblitz link creates a broken application #13612

Closed

[charts] Replace react-spring with d3-transition/CSS transitions #16945

Closed

JCQuintas approved these changes Mar 31, 2025

View reviewed changes

bernardobelchior merged commit 39de1eb into mui:master Apr 1, 2025
22 checks passed

This was referenced Apr 1, 2025

[charts][core] Replace useIsomorphicLayoutEffect of @react-spring/web for Base UI's helper #13431

Closed

[charts] Make sure charts is compatible with React 19 #13163

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Remove react spring #17123

[charts] Remove react spring #17123

bernardobelchior commented Mar 25, 2025 •

edited

Loading

bernardobelchior Mar 25, 2025

mui-bot commented Mar 25, 2025 •

edited

Loading

github-actions bot commented Mar 26, 2025

bernardobelchior Mar 26, 2025

JCQuintas Mar 26, 2025

bernardobelchior Mar 27, 2025

bernardobelchior Mar 27, 2025

alexfauquette Mar 27, 2025

bernardobelchior Mar 28, 2025

alexfauquette Mar 28, 2025

bernardobelchior Mar 28, 2025

alexfauquette Mar 28, 2025

bernardobelchior Mar 28, 2025

bernardobelchior Mar 28, 2025

codspeed-hq bot commented Mar 27, 2025 •

edited

Loading

alexfauquette left a comment

alexfauquette Mar 28, 2025

bernardobelchior Mar 28, 2025

JCQuintas Mar 31, 2025

bernardobelchior Mar 31, 2025

JCQuintas Mar 31, 2025

alexfauquette Mar 28, 2025

alexfauquette Mar 28, 2025

bernardobelchior Mar 28, 2025 •

edited

Loading

alexfauquette Mar 31, 2025

alexfauquette left a comment

JCQuintas commented Mar 31, 2025 •

edited

Loading

	skipAnimation={skipAnimation ?? false}
	skipAnimation={skipAnimation}

[charts] Remove react spring #17123

[charts] Remove react spring #17123

Conversation

bernardobelchior commented Mar 25, 2025 • edited Loading

Changelog

Choose a reason for hiding this comment

mui-bot commented Mar 25, 2025 • edited Loading

github-actions bot commented Mar 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented Mar 27, 2025 • edited Loading

Merging #17123 will improve performances by 58.76%

Summary

Benchmarks breakdown

alexfauquette left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardobelchior Mar 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfauquette left a comment

Choose a reason for hiding this comment

JCQuintas commented Mar 31, 2025 • edited Loading

bernardobelchior commented Mar 25, 2025 •

edited

Loading

mui-bot commented Mar 25, 2025 •

edited

Loading

codspeed-hq bot commented Mar 27, 2025 •

edited

Loading

bernardobelchior Mar 28, 2025 •

edited

Loading

JCQuintas commented Mar 31, 2025 •

edited

Loading