Fix React <Transition> flicker issue (#1118)

* Fix React transition bug

* use a ref instead of a useCallback (#1108)

This allows us to guarantee that the ref is always referencing the
latest callback. This also allows us to re-run fewer effects because we
don't really care about intermediate callback values, just the last one.

* Fix tests

* Update changelog

Co-authored-by: Robin Malfait <[email protected]>

Author: thecrypticace

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improve SSR for `Tab` component ([#1155](https://github.com/tailwindlabs/headlessui/pull/1155))
 - Fix `hover` scroll ([#1161](https://github.com/tailwindlabs/headlessui/pull/1161))
 - Guarantee DOM sort order when performing actions ([#1168](https://github.com/tailwindlabs/headlessui/pull/1168))
+- Fix `<Transition>` flickering issue ([#1118](https://github.com/tailwindlabs/headlessui/pull/1118))
 
 ## [Unreleased - @headlessui/vue]
 

packages/@headlessui-react/src/components/transitions/transition.tsx

@@ -1,7 +1,6 @@
 import React, {
   Fragment,
   createContext,
-  useCallback,
   useContext,
   useEffect,
   useMemo,
@@ -32,6 +31,7 @@ import { Reason, transition } from './utils/transition'
 import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed'
 import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete'
 import { useSyncRefs } from '../../hooks/use-sync-refs'
+import { useLatestValue } from '../../hooks/use-latest-value'
 
 type ID = ReturnType<typeof useId>
 
@@ -103,8 +103,8 @@ function useParentNesting() {
 
 interface NestingContextValues {
   children: MutableRefObject<{ id: ID; state: TreeStates }[]>
-  register: (id: ID) => () => void
-  unregister: (id: ID, strategy?: RenderStrategy) => void
+  register: MutableRefObject<(id: ID) => () => void>
+  unregister: MutableRefObject<(id: ID, strategy?: RenderStrategy) => void>
 }
 
 let NestingContext = createContext<NestingContextValues | null>(null)
@@ -118,48 +118,38 @@ function hasChildren(
 }
 
 function useNesting(done?: () => void) {
-  let doneRef = useRef(done)
+  let doneRef = useLatestValue(done)
   let transitionableChildren = useRef<NestingContextValues['children']['current']>([])
   let mounted = useIsMounted()
 
-  useEffect(() => {
-    doneRef.current = done
-  }, [done])
-
-  let unregister = useCallback(
-    (childId: ID, strategy = RenderStrategy.Hidden) => {
-      let idx = transitionableChildren.current.findIndex(({ id }) => id === childId)
-      if (idx === -1) return
-
-      match(strategy, {
-        [RenderStrategy.Unmount]() {
-          transitionableChildren.current.splice(idx, 1)
-        },
-        [RenderStrategy.Hidden]() {
-          transitionableChildren.current[idx].state = TreeStates.Hidden
-        },
-      })
-
-      if (!hasChildren(transitionableChildren) && mounted.current) {
-        doneRef.current?.()
-      }
-    },
-    [doneRef, mounted, transitionableChildren]
-  )
+  let unregister = useLatestValue((childId: ID, strategy = RenderStrategy.Hidden) => {
+    let idx = transitionableChildren.current.findIndex(({ id }) => id === childId)
+    if (idx === -1) return
+
+    match(strategy, {
+      [RenderStrategy.Unmount]() {
+        transitionableChildren.current.splice(idx, 1)
+      },
+      [RenderStrategy.Hidden]() {
+        transitionableChildren.current[idx].state = TreeStates.Hidden
+      },
+    })
 
-  let register = useCallback(
-    (childId: ID) => {
-      let child = transitionableChildren.current.find(({ id }) => id === childId)
-      if (!child) {
-        transitionableChildren.current.push({ id: childId, state: TreeStates.Visible })
-      } else if (child.state !== TreeStates.Visible) {
-        child.state = TreeStates.Visible
-      }
-
-      return () => unregister(childId, RenderStrategy.Unmount)
-    },
-    [transitionableChildren, unregister]
-  )
+    if (!hasChildren(transitionableChildren) && mounted.current) {
+      doneRef.current?.()
+    }
+  })
+
+  let register = useLatestValue((childId: ID) => {
+    let child = transitionableChildren.current.find(({ id }) => id === childId)
+    if (!child) {
+      transitionableChildren.current.push({ id: childId, state: TreeStates.Visible })
+    } else if (child.state !== TreeStates.Visible) {
+      child.state = TreeStates.Visible
+    }
+
+    return () => unregister.current(childId, RenderStrategy.Unmount)
+  })
 
   return useMemo(
     () => ({
@@ -226,6 +216,7 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
 
   let { show, appear, initial } = useTransitionContext()
   let { register, unregister } = useParentNesting()
+  let prevShow = useRef(undefined)
 
   let id = useId()
 
@@ -236,14 +227,14 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
     // transitioning ourselves. Otherwise we would unmount before the transitions are finished.
     if (!isTransitioning.current) {
       setState(TreeStates.Hidden)
-      unregister(id)
+      unregister.current(id)
       events.current.afterLeave()
     }
   })
 
   useIsoMorphicEffect(() => {
     if (!id) return
-    return register(id)
+    return register.current(id)
   }, [register, id])
 
   useIsoMorphicEffect(() => {
@@ -258,8 +249,8 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
     }
 
     match(state, {
-      [TreeStates.Hidden]: () => unregister(id),
-      [TreeStates.Visible]: () => register(id),
+      [TreeStates.Hidden]: () => unregister.current(id),
+      [TreeStates.Visible]: () => register.current(id),
     })
   }, [state, id, register, unregister, show, strategy])
 
@@ -290,6 +281,7 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
     let node = container.current
     if (!node) return
     if (skip) return
+    if (show === prevShow.current) return
 
     isTransitioning.current = true
 
@@ -323,7 +315,7 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
             // ourselves.
             if (!hasChildren(nesting)) {
               setState(TreeStates.Hidden)
-              unregister(id)
+              unregister.current(id)
               events.current.afterLeave()
             }
           }
@@ -345,6 +337,10 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
     leaveToClasses,
   ])
 
+  useIsoMorphicEffect(() => {
+    prevShow.current = show
+  }, [show])
+
   let propsWeControl = { ref: transitionRef }
   let passthroughProps = rest