Library does not work with containers that are not initially rendered within a component (React)

[j3lev]

Hey there! Great job with this library, it's such a quick and easy way to improve UX. I encountered an issue where the animations do not work when a container is not initially rendered in the DOM. I created a small codesandbox to demonstrate this here:

Notice parent2 animates as expected, however its children do not. If you change the initial state of showParent2 to true, everything will animate correctly (until parent2 isn't being rendered anymore). An obvious workaround to this issue is to compose your app in a way such that each individual ref is always rendered within each component.

[stevecastaneda]

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.

[mtsknn]

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. 😜)

[mtsknn]

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).

[justin-schroeder]

@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.

[mtsknn]

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.

[justin-schroeder]

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.

[mtsknn]

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]
}

[mtsknn]

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.