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

[oliviertassinari]

Steps to reproduce

mui-x/packages/x-charts/src/hooks/useReducedMotion.ts

Line 1 in ba8d2f2

import { useIsomorphicLayoutEffect, Globals } from '@react-spring/web';

looks suboptimal to me. useIsomorphicLayoutEffect should be using Base UI helper instead.

[LukasTy]

@oliviertassinari could you confirm the @mui/base helper you are referring to? 🤔
Maybe it is import useEnhancedEffect from '@mui/utils/useEnhancedEffect'; you were referring to? 🤔
In this case, it seems that the former might have a bit less coverage... 🙈

const useEnhancedEffect = typeof window !== 'undefined' ? React.useLayoutEffect : React.useEffect;

vs

var isSSR = () => typeof window === "undefined" || !window.navigator || /ServerSideRendering|^Deno\//.test(window.navigator.userAgent);
var useIsomorphicLayoutEffect = isSSR() ? useEffect : useLayoutEffect;

[michelengelen]

is this still a valid issue then? 🤔

[LukasTy]

Probably a question to @mui/xcharts. 🤔
IMHO, using the existing effect doesn't hurt, especially if and when charts would drop material-ui dependency (then having a slight discrepancy between behaviors wouldn't have any effect).
On the other hand, WDYT @michaldudak, would it make sense to extend the useEnhancedEffect with the additional checks that @react-spring/web is doing? 🤔

[michaldudak]

I haven't seen any issues regarding the current implementation, but indeed it seems that Deno has window defined during SSR. I'm OK with making it more reliable (I'd also call it useIsomorphicLayoutEffect, because useEnhancedEffect tells users exactly nothing, but that's another story ;).

[oliviertassinari]

could you confirm the @mui/base helper you are referring to?

@LukasTy I was thinking of useEnhancedEffect indeed. This helper still needs to be exposed from @base_ui/react/useEnhancedEffect but this issue will help us track progress / coordinate.

it seems that the former might have a bit less coverage

It feels like useEnhancedEffect is better because it's less bundle size, and I'm not aware of people complaining about its behavior.

using the existing effect doesn't hurt, especially if and when charts would drop material-ui dependency

It's part of why I opened this issue, the value I see:

• As MUI X move to have a great unstyled (and maybe even headless) story, will move some of its dependencies to Base UI. In practice, it will likely just be about removing all reexports from Material UI to have a clear separation for what is from which library.
• This duplication is extra bundle size.
• This behavior difference. If one is better than the other, the other should follow suite.