[charts] Fix LineChart animation being stuck with initial drawing area value

[JCQuintas]

Should improve the behaviour of #13477 when animations are enabled or not 😅

This fix is comprised of two parts

1. Ensure the animation is run correctly on line chart
2. Prevent the chart slowly expanding render to happen

While #2 is not an issue of the chart per-se, we can make it work by adding some additional behaviour to the useChartContainerDimensions hook, so it calculates the necessary size it should take before the first render, while trying to prevent an infinite loop in case the layout can't coalesce into a defined size.

[mui-bot]

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

Generated by 🚫 dangerJS against cb7058e

[codspeed-hq]

CodSpeed Performance Report

Merging #14553 will not alter performance

_{Comparing JCQuintas:improve-unknown-width-behaviour (cb7058e) with master (dc7b629)}

Summary

✅ 3 untouched benchmarks

[alexfauquette]

Could you describe the behavior with for example what is currently happening step by step, and how this PR modify it?

I don't huderstand how adding a useEnhancedEffect could help to prevent inifint loop.

If you want a nice reproduction of an infinit loop there is one here:
#12263 (comment)

[JCQuintas]

This useEffect doesn't prevent an infinite loop. Only computeRun > 20 does. It serves as a circuit breaker in case the layout never settles into a defined size, which would mean this useEffect would run indefinitely.

This useEffect only happens before/during the first render, when the page layout is figuring out what size the components need. Hence it is different from the issues in your example, where the loop in on the render side.

To fix the loop in your example, we can try to handle it on the useEnhancedEffect that has the ResizeObserver inside. Though I'm not sure how useful it would be, since my current fix is only for layouts that will definitely settle into a defined width/height, but your example will never settle 😓

Explanation of what it does

The main idea of this useEnhancedEffect is that everything in it will happen before we see anything, since it is running on the layout effect and committing changes to the state, the state will update synchronously.

The state updates until it settles on a size (new size is the same as last one) or it runs more than 20 times (usually it only requires 5-6 runs from what I tested).

Once one of these two conditions are met, the hook stops looping (width/height stop changing), and the first actual react render happens.

[alexfauquette]

I'm ok with the solution. But this will silently delay the painting without warning the developers about the improvement they could bring. We should add at least a warning if computeRun > 2

We can not do computeRun > 0 because the react strict mode trigger twice the useLayoutEffect. Si the ref is updated twice before the state get updated.

[JCQuintas]

I had added a warning at first but thought it could be unnecessary and removed it 😅

Will add it back

[JCQuintas]

Hum, I actually removed it because it is an actual valid behaviour that wouldn't require a warning. 🤔

Like, if the user uses this inside a grid, this will always warn, even though a grid's width would eventually settle in the screen's width.

[alexfauquette]

it is an actual valid behaviour

I disagree on this point. For me the valid behavior is "The container has a defined size". This PR is adding a nice fallback

I tried with Recharts and they have the same behavior as us currently.

Capture.video.du.2024-09-16.10-02-01.mp4

From my point of view it would be bad to have such loop without informing user since it silently fix something the developers could potentially do better.

In addition to a warning, it could be a warning plus a prop to silence the warning. Or no warning and a prop to activate the feature

[JCQuintas]

As "valid behaviour" I mean it is something the browser does on its own. Not that our behaviour is valid 😅

I would agree with a prop for this though.

Something like:

 /**
  * The chart will try to wait for the parent container to resolve its size
  * before it renders for the first time.
  * 
  * This can be useful in some scenarios where the chart appear to grow after
  * the first render, like when used inside a grid.
  */
  resolveSizeBeforeRender

[alexfauquette]

Good for me. We will see if we get issues about this. In such case, adding a paragraph about this prop in the docs could be the next step