You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
// eslint-disable-next-line no-param-reassign -- mutating a ref is intended
forwardedRef.current=newRef;
}
},[forwardedRef]);
The old code was problematic because it uses ref.. sometimes.
The new code makes the ref handling better / more consistent, so it always returns a ref now.
Overall this is an improvement.
However, the new code triggers errors with mui Tooltips in a minor version upgrade.
(We use legacy material-ui 4.x, but the same problem still appears to apply to mui 5.x)
This only started to cause errors (= webapp "crashes" in our case) in react-dom while running with vite devserver once we upgraded to 1.5.6:
Uncaught Error: Argument appears to not be a ReactComponent. Keys: _cbs,name,path,isLoaded,currentFrame,currentRawFrame,firstFrame,totalFrames,frameRate,frameMult,playSpeed,playDirection,playCount,animationData,assets,isPaused,autoplay,loop,renderer,animationID,assetsPath,timeCompleted,segmentPos,isSubframeEnabled,segments,_idle,_completedLoop,projectInterface,imagePreloader,audioController,markers,configAnimation,onSetupError,onSegmentComplete,drawnFrameEvent,expressionsPlugin,wrapper,animType,autoloadSegments,initialSegment,frameModifier
The error happens because Tooltip internally uses useForkRef to get the children ref. useForkRef creates a function as forwardedRef which was used incorrectly in 1.5.5, so mui didn't see the ref.
In 1.5.6 the function is suddenly used, but it passes a non-DOM element to the Tooltip through forwardedRef.
When Tooltip tries to render the lottie object through react-dom, we get the error above.
We fixed this by wrapping Lottie in a div in our codebase (which clearly had a bug because it didn't meet mui requirements by using a non-DOM element as a child).
Personally, I consider this an issue in mui (expecting ref to be a DOM element), but I think react-lottie-player could also avoid this issue by not using the generic ref name for a non-DOM element.
I think for v3.x the ref prop should be renamed to lottieRef to avoid mistaking it for a DOM ref in popular libraries like mui.
For the main ref (if it even exists) it probably makes more sense to return the DOM container in which the lottie animation plays.
Other than that, this issue is mostly meant as a heads up to other people running into the same error / crash.
Also, if anyone has a linter rule to check for this (mui expecting DOM ref as child, but child isn't a DOM ref) that'd be excellent.
The text was updated successfully, but these errors were encountered:
There was a subtle fix introduced in 7a4d36b:
Old code (1.5.5)
react-lottie-player/src/makeLottiePlayer.js
Lines 66 to 70 in 3c9c093
New code (1.5.6)
react-lottie-player/src/makeLottiePlayer.js
Lines 81 to 89 in 34c6fcd
The old code was problematic because it uses
ref
.. sometimes.The new code makes the ref handling better / more consistent, so it always returns a ref now.
Overall this is an improvement.
However, the new code triggers errors with mui Tooltips in a minor version upgrade.
(We use legacy material-ui 4.x, but the same problem still appears to apply to mui 5.x)
We have code like:
Our code was always broken because
Tooltip
expects the children to be DOM elements withref
.This is documented here: https://mui.com/material-ui/api/tooltip/#props and here https://mui.com/material-ui/guides/composition/#caveat-with-refs
However, we didn't spot this issue.
This only started to cause errors (= webapp "crashes" in our case) in react-dom while running with vite devserver once we upgraded to 1.5.6:
The error happens because
Tooltip
internally usesuseForkRef
to get the children ref.useForkRef
creates a function asforwardedRef
which was used incorrectly in 1.5.5, so mui didn't see the ref.In 1.5.6 the function is suddenly used, but it passes a non-DOM element to the Tooltip through
forwardedRef
.When Tooltip tries to render the lottie object through react-dom, we get the error above.
We fixed this by wrapping
Lottie
in adiv
in our codebase (which clearly had a bug because it didn't meet mui requirements by using a non-DOM element as a child).Personally, I consider this an issue in mui (expecting
ref
to be a DOM element), but I think react-lottie-player could also avoid this issue by not using the genericref
name for a non-DOM element.I think for v3.x the
ref
prop should be renamed tolottieRef
to avoid mistaking it for a DOM ref in popular libraries like mui.For the main
ref
(if it even exists) it probably makes more sense to return the DOM container in which the lottie animation plays.Other than that, this issue is mostly meant as a heads up to other people running into the same error / crash.
Also, if anyone has a linter rule to check for this (mui expecting DOM ref as child, but child isn't a DOM ref) that'd be excellent.
The text was updated successfully, but these errors were encountered: