Comments (8)
@mtsknn This is an interesting topic, and while its true that one or two additional renders might not be a huge deal, it does seem there could be unintentional runaway side effects using DOM elements as state, possibly even infinite loops. I've always avoided putting Nodes into state because it feels like an anti-pattern, but I'm certainly not prepared to articulate it better than that and your examples are compelling, so I'm happy to concede this if there are a quorum of people that think it is an acceptable path forward.
However, the MutationObserver
AutoAnimate uses is capable of deep observation, we intentionally leave this disabled to prevent really deep node trees having a ton of tracking on them (technically this issue should be marked as "working as intended", but I'm leaving it open as I agree the DX should be improved). Perhaps deep observation should be an opt-in behavior people can judiciously add to their projects when it makes sense to do so.
I'd also love to hear from other React devs what their opinions are on putting DOM nodes in state.
from auto-animate.
Good thoughts!
I had a good night's sleep and realized that a callback ref is all that's needed, and it also doesn't cause extra re-renders: demo 3. Hooray! Not sure if the useCallback
is even necessary here. Maybe not because there's no state to trigger possibly infinite re-render loops. But it shouldn't hurt either to have it.
(Edit: if useCallback
is used, options
should be included in the dependency array. I updated the demo link.)
(Edit 2: updated the demo again with a "toggle duration" button. The current useAutoAnimate
hook doesn't support changing the options on the fly because options
is not included in the useEffect
's dependency array, right? So using a callback ref would fix this as well. π Though the current hook would be easy to fix as well, just include options
in the dep array.)
By the way, the current useAutoAnimate
hook has a useEffect
. Is there a need to return a clean-up function, or is there anything to clean up manually after using AutoAnimate? If there is, I'm not sure if the callback ref supports that easily.
from auto-animate.
Ran into this as well. What I ended up doing was splitting up the components. The resulting issue is that the ref isn't assigned on mount so if you log the ref in an effect, you'll see it stays null.
When I moved the conditional render logic to the parent container component, the issue went away.
from auto-animate.
The useAutoAnimate
hook has a ref in the dependency array of a useEffect
:
const element = useRef<T>(null)
useEffect(() => {
// ...
}, [element])
Refs are stable, so including the ref in the dep array does nothing. (Interestingly, ESLint doesn't seem to complain about it.)
Changing the dep array from [element]
to [element.current]
is more correct and almost fixes the problem: demo 1. After clicking the toggle button, the first item addition is not animated, the rest are.
Using useState
instead of useRef
fixes the problem: demo 2. This also causes more re-renders (check the console), though an extra re-render is likely unavoidable with conditional rendering. (And obsessing over the amount of re-renders is overrated IMO. π)
from auto-animate.
an extra re-render is likely unavoidable with conditional rendering
...unless you split up the components like @stevecastaneda.
IMO using useState
in useAutoAnimate
would provide better DX (no need to split up components) even if it causes some extra re-renders (shouldn't probably be a big deal).
from auto-animate.
Currently there is no cleanup because you cannot disable AutoAnimate after it has been initiated. So when the nodes are garbage collected its effects are as well (we use WeakMap
). However at some point we'll have a way to disable it βΒ thats why the hooks return arrays, to they can return a "stopAnimation" value at some point in the future.
from auto-animate.
Oh right, the hook returns a tuple, so could something like this work?
function useAutoAnimate(options = {}) {
const stopAnimation = useRef(null)
const callbackRef = useCallback(
(element) => {
if (element instanceof HTMLElement) {
stopAnimation.current = autoAnimate(element, options)
}
},
[options]
)
// Will be called automatically on unmount
useEffect(() => stopAnimation.current, [])
// Returned so can be also called manually
return [callbackRef, stopAnimation.current]
}
from auto-animate.
By the way, I think useCallback
is mostly unnecessary here because the options
dependency changes so easily that useCallback
will anyway recreate the callback ref function in most cases:
// Changes on every re-render if the `options` parameter defaults to `{}`
const [cbRef] = useAutoAnimate()
// Stable if the `options` parameter defaults to `undefined`
const [cbRef] = useAutoAnimate()
// Changes on every re-render
const [cbRef] = useAutoAnimate({ duration: 300 })
// Stable but convoluted
const options = useRef({ duration: 300 }).current
const [cbRef] = useAutoAnimate(options)
// Dynamic options: changes on every re-render
const [duration, setDuration] = useState(300)
const [cbRef] = useAutoAnimate({ duration })
// Dynamic options: stable but extra convoluted
const [duration, setDuration] = useState(300)
const options = React.useRef({ duration })
useEffect(() => {
options.current = { duration }
}, [duration])
const [cbRef] = useAutoAnimate(options.current)
Does a callback ref even need to be stable? π€ Probably not.
from auto-animate.
Related Issues (20)
- Bug when deleting element of the array in React 18
- Inconsistent jumping inside flex item HOT 1
- Not working HOT 4
- Vue Virtual Scroller
- Wrong arguments order in AutoAnimationPlugin declaration
- Animate across lists? Across all elements inside a `ul`?
- How should I use fixed centering layout? HOT 1
- Incorrect rounding of element sizes while animating HOT 2
- When the page is refreshed, the component has no animation.
- Vue useAutoAnimate not working with v-if
- does not provide an export named 'autoAnimate'
- Animation starting coords are buggy when the container has transitioned positions
- How to add auto-animate to custom components (NuxtUI) HOT 1
- Headless UI + Auto-Animate | Issue with auto-animate conflicting HOT 1
- nuxt3 3.10.3 does not provide an export named 'autoAnimate' HOT 1
- I added the element from the front but the animation still appears from the last one HOT 1
- React.Children.map issue HOT 1
- How to animate single element? HOT 3
- The compatibility of TR tags is a bit difficult to use, and the TR animation effect has become distorted
- CDN support ?? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from auto-animate.