[charts] Migrate some animations from react-spring

[bernardobelchior]

Related to #16281.

This PR proposes a new way to animate components in the charts library.

Where possible, we should use CSS animations and transitions. In the cases where using CSS animations isn't possible, this PR introduces a new hook useAnimate which allows for JS-based animation.

The hook uses a ref to reduce re-renders. This useAnimate hook is low-level and should not be exported. Higher-level and exportable hooks should be created for each specific animation (see useAnimateLine, useAnimatePieArc).

Open Questions

How to handle versioning?
This is a breaking change because the props passed to slots changed from SpringValue<T> to T. The window for merging this into v8 is closing, but we probably need robust testing to ensure this is a good alternative to React Spring.

Changelog

Removed react-spring from the following slots:

• pieArc: used directly in PieArcPlot and transitively in PieChart;
• mark used directly in MarkPlot and transitively in LineChart;
• line used directly in LineElement, and transitively in LinePlot, LineChart and SparkLineChart (if plotType is 'line').

As a result, the SpringValue wrapper in some of pieArc's slot props were removed. This means that the props cornerRadius, endAngle, innerRadius, outerRadius, paddingAngle and startAngle are now number instead of SpringValue<number>.

Additionally, the pieArc slot now receives a skipAnimation prop to configure whether animations should be enabled or disabled.

[github-actions]

Thanks for adding a type label to the PR! 👍

[mui-bot]

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

Generated by 🚫 dangerJS against 1527462

[codspeed-hq]

CodSpeed Performance Report

Merging #16961 will improve performances by 39.89%

_{Comparing bernardobelchior:migrate-css-transitions (1527462) with master (1cc12b9)}

Summary

⚡ 3 improvements
✅ 5 untouched benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
⚡	LineChart with big data amount	316.9 ms	232.7 ms	+36.17%
⚡	LineChartPro with big data amount	146.9 ms	113.2 ms	+29.79%
⚡	PieChart with big data amount	183.7 ms	131.3 ms	+39.89%

[bernardobelchior]

I think I'll need to update the AnimatedLine's animation as part of this PR, otherwise the animations will look disconnected as CSS doesn't have a spring animation:

Screen.Recording.2025-03-14.at.12.14.53.mov

The linear() CSS function can be used to simulate a spring animation, but browser support isn't there yet (source). E.g., it was added in Safari 17, but we support >= 15.4 at the moment.

[JCQuintas]

I think I'll need to update the AnimatedLine's animation as part of this PR, otherwise the animations will look disconnected as CSS doesn't have a spring animation:

Screen.Recording.2025-03-14.at.12.14.53.mov
The linear() CSS function can be used to simulate a spring animation, but browser support isn't there yet (source). E.g., it was added in Safari 17, but we support >= 15.4 at the moment.

Insn't linear just a subset of cubic-bezier?

[bernardobelchior]

I think I'll need to update the AnimatedLine's animation as part of this PR, otherwise the animations will look disconnected as CSS doesn't have a spring animation:
Screen.Recording.2025-03-14.at.12.14.53.mov
The linear() CSS function can be used to simulate a spring animation, but browser support isn't there yet (source). E.g., it was added in Safari 17, but we support >= 15.4 at the moment.

Insn't linear just a subset of cubic-bezier?

No, linear() accepts a variable number of points, while cubic-bezier() only accepts two points. I couldn't find a way to approximate a spring motion using cubic-bezier().

Here's a tool to use the linear() function to simulate a spring: https://linear-easing-generator.netlify.app/

[JCQuintas]

The linear function is different to the linear curve, but they have the same name 🫠

[github-actions]

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

[alexfauquette]

I'm surprised the server does nto complain that useLayoutEffect does not exist on server

[bernardobelchior]

That's a good point. I have no idea why I'm not seeing a warning 😅
Could it be because the playground is running on NextJS pages router?

Nevertheless, I think we might have to use an "isomorphic" layout effect, as none of these options apply here. It would be unfortunate to introduce a component just to support SSR.

[alexfauquette]

I agree about the useIsomorphic. About the abscence of error, it seems to be related to React v19. I tried with v18 and I got the famous

Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://reactjs.org/link/uselayouteffect-ssr for common fixes.
at AnimatedLine (/home/alexandre/dev/mui/mui-x/packages/x-charts/src/LineChart/AnimatedLine.tsx:46:13)

Otherwise the SSR looks good 👍

[bernardobelchior]

Done 👍

[alexfauquette]

I don't understand why we need to restart an animation if props get updated. If the color of a bar is updated. I don't see a reason to restart an animation about it's x attribute

[bernardobelchior]

These aren't all props. These are only props that will be animated. We need to restart the animation when those props change.

IMO we shouldn't expose useAnimate directly. This is for internal use only. What we export is useAnimatePieArc or useAnimateLine and those will only pass the animatable props.

[bernardobelchior]

React 18 tests are failing for a curious reason, so leaving this comment for the future.

React 19 introduced double calling of ref callbacks when StrictMode is enabled. By default, createRenderer from @mui/internal-test-utils return a render function that will wrap the component in StrictMode, so I thought that was the problem. However, it seems like StrictMode isn't working with React 19 even when we create the render function using const { render } = createRenderer({ strict: true, strictEffects: true }).

It is working on React 18, though, which is why the tests were failing: the useAnimate hook has a bug in Strict Mode. In React 19, it isn't working, which was why tests were passing. I'll dig deeper to understand what's going on with React 19.

[bernardobelchior]

This breaks in Strict Mode. D3 transitions aren't resumable, so if we interrupt them we need to start another one, which we aren't at the moment.

[bernardobelchior]

Due to this, I think I'll need to implement resumable transitions, which D3 transitions doesn't support.

The idea is to store a transition in a ref and stop it on ref cleanups. When the ref callback is called again with an element, we check if it's the same DOM element and same props: if so, resume transition; if not, start new transition.

[bernardobelchior]

I was unsure whether to pass ref as a normal prop or a second argument. Since ref is now a normal prop I'm inclined to keep this API so a user can pass the props directly. Otherwise, they'll need to destructure ref from the props to pass it as a second parameter.

This means that React 18 users will need to do useAnimatePieArc({ ...props, ref }), which is unfortunate.

@JCQuintas @alexfauquette let me know what you think

[JCQuintas]

This could be a small issue. Should we allow useAnimatePieArc({ ...props, ref? }, ref?) And use whichever is passed in?

Right now charts don't work properly with react 19 anyways. Though with this PR it comes closer being react 19-able.

[bernardobelchior]

Should we allow useAnimatePieArc({ ...props, ref? }, ref?) And use whichever is passed in?

We can, but then we'd have to deprecate this behavior in future releases, that's why I'm more inclined towards spreading props. As people migrate to React >=19, the spreading becomes unnecessary.

Do you think it would be that cumbersome to spread the props and add the ref for React <19 that it's worth adding this behavior?

[JCQuintas]

I'm not sure how far we would want to push this 😅

It would be possible to properly type it for different react versions by using something like this:

type React19<R19, R18> =
  Exclude<ReturnType<React.RefCallback<number>>, void> extends never ? R18 : R19;

type UseAnimatePieArcParams = React19<
  [params: { something: number } & { ref?: React.Ref<SVGPathElement> }],
  [params: { something: number }, ref?: React.Ref<SVGPathElement>]
>;

export function useAnimatePieArc(...params: UseAnimatePieArcParams): any;
export function useAnimatePieArc(
  params: { something: number; ref?: React.Ref<SVGPathElement> },
  ref?: React.Ref<SVGPathElement>,
): any {
  const internalRef = React.useRef<SVGPathElement>(null);
  const myRef = params.ref ?? ref ?? internalRef;

  return myRef;
}

const WorksIn19 = () => {
  const internalRef = React.useRef<SVGPathElement>(null);
  const ref = useAnimatePieArc({ something: 1, ref: internalRef });
  return ref;
};

const WorksIn18 = () => {
  const internalRef = React.useRef<SVGPathElement>(null);
  const ref = useAnimatePieArc({ something: 1 }, internalRef);
  return ref;
};

[bernardobelchior]

Yeah, but if we wanted to remove the second parameter it would still be a breaking change.

I think the number of people using our exported animation hooks will be relatively low, so I think it would be better to keep the ref-inside-props API. It is slightly annoying to spread the ref into the props object, but it's temporary and it will stop being problem as soon as users upgrade to React 19.

What do you think? Do you think it's that cumbersome that's worth us having two code paths to avoid an object spread for some users?

[JCQuintas]

No, I think you are right. I don't think this will be heavily used enough to provide two different APIs, and the use-case where the user will need their own access to ref would be even lower

[bernardobelchior]

Maybe a changelog message in the PR description, and we should be good 👍

It was there already 😄 any improvement suggestion?

[JCQuintas]

[JCQuintas]

Massive 👍