Fix enter transitions for the Transition component

packages/@headlessui-react/CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Expose missing `data-disabled` and `data-focus` attributes on the `TabsPanel`, `MenuButton`, `PopoverButton` and `DisclosureButton` components ([#3061](https://github.com/tailwindlabs/headlessui/pull/3061))
 - Fix cursor position when re-focusing the `ComboboxInput` component ([#3065](https://github.com/tailwindlabs/headlessui/pull/3065))
 - Keep focus inside of the `<ComboboxInput />` component ([#3073](https://github.com/tailwindlabs/headlessui/pull/3073))
+- Fix enter transitions for the `Transition` component ([#3074](https://github.com/tailwindlabs/headlessui/pull/3074))
 
 ### Changed
 

packages/@headlessui-react/src/components/transition/__snapshots__/transition.test.tsx.snap

@@ -136,6 +136,7 @@ exports[`Setup API transition classes should be possible to passthrough the tran
 exports[`Setup API transition classes should be possible to passthrough the transition classes and immediately apply the enter transitions when appear is set to true 1`] = `
 <div
   class="enter enter-from"
+  style=""
 >
   Children
 </div>

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

@@ -343,6 +343,7 @@ describe('Setup API', () => {
           <div
             class="foo1
         foo2 foo1 foo2 leave"
+            style=""
           >
             Children
           </div>

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

@@ -4,7 +4,6 @@ import React, {
   Fragment,
   createContext,
   useContext,
-  useEffect,
   useMemo,
   useRef,
   useState,
@@ -18,6 +17,7 @@ import { useFlags } from '../../hooks/use-flags'
 import { useIsMounted } from '../../hooks/use-is-mounted'
 import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect'
 import { useLatestValue } from '../../hooks/use-latest-value'
+import { useOnDisappear } from '../../hooks/use-on-disappear'
 import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete'
 import { useSyncRefs } from '../../hooks/use-sync-refs'
 import { useTransition } from '../../hooks/use-transition'
@@ -259,26 +259,6 @@ function useNesting(done?: () => void, parent?: NestingContextValues) {
   )
 }
 
-function noop() {}
-let eventNames = ['beforeEnter', 'afterEnter', 'beforeLeave', 'afterLeave'] as const
-function ensureEventHooksExist(events: TransitionEvents) {
-  let result = {} as Record<keyof typeof events, () => void>
-  for (let name of eventNames) {
-    result[name] = events[name] ?? noop
-  }
-  return result
-}
-
-function useEvents(events: TransitionEvents) {
-  let eventsRef = useRef(ensureEventHooksExist(events))
-
-  useEffect(() => {
-    eventsRef.current = ensureEventHooksExist(events)
-  }, [events])
-
-  return eventsRef
-}
-
 // ---
 
 let DEFAULT_TRANSITION_CHILD_TAG = 'div' as const
@@ -349,7 +329,7 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
     leaveTo: splitClasses(leaveTo),
   })
 
-  let events = useEvents({
+  let events = useLatestValue({
     beforeEnter,
     afterEnter,
     beforeLeave,
@@ -369,6 +349,7 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
   let immediate = appear && show && initial
 
   let transitionDirection = (() => {
+    if (immediate) return 'enter'
     if (!ready) return 'idle'
     if (skip) return 'idle'
     return show ? 'enter' : 'leave'
@@ -380,11 +361,11 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
     return match(direction, {
       enter: () => {
         transitionStateFlags.addFlag(State.Opening)
-        events.current.beforeEnter()
+        events.current.beforeEnter?.()
       },
       leave: () => {
         transitionStateFlags.addFlag(State.Closing)
-        events.current.beforeLeave()
+        events.current.beforeLeave?.()
       },
       idle: () => {},
     })
@@ -394,26 +375,29 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
     return match(direction, {
       enter: () => {
         transitionStateFlags.removeFlag(State.Opening)
-        events.current.afterEnter()
+        events.current.afterEnter?.()
       },
       leave: () => {
         transitionStateFlags.removeFlag(State.Closing)
-        events.current.afterLeave()
+        events.current.afterLeave?.()
       },
       idle: () => {},
     })
   })
 
+  let isTransitioning = useRef(false)
+
   let nesting = useNesting(() => {
-    // When all children have been unmounted we can only hide ourselves if and only if we are not
-    // transitioning ourselves. Otherwise we would unmount before the transitions are finished.
+    // When all children have been unmounted we can only hide ourselves if and
+    // only if we are not transitioning ourselves. Otherwise we would unmount
+    // before the transitions are finished.
+    if (isTransitioning.current) return
+
     setState(TreeStates.Hidden)
     unregister(container)
   }, parentNesting)
 
-  let isTransitioning = useRef(false)
   useTransition({
-    immediate,
     container,
     classes,
     direction: transitionDirection,
@@ -426,8 +410,8 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
       nesting.onStop(container, direction, afterEvent)
 
       if (direction === 'leave' && !hasChildren(nesting)) {
-        // When we don't have children anymore we can safely unregister from the parent and hide
-        // ourselves.
+        // When we don't have children anymore we can safely unregister from the
+        // parent and hide ourselves.
         setState(TreeStates.Hidden)
         unregister(container)
       }
@@ -437,10 +421,10 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
   let theirProps = rest
   let ourProps = { ref: transitionRef }
 
+  // Already apply the `enter` and `enterFrom` on the server if required
   if (immediate) {
     theirProps = {
       ...theirProps,
-      // Already apply the `enter` and `enterFrom` on the server if required
       className: classNames(rest.className, ...classes.current.enter, ...classes.current.enterFrom),
     }
   }
@@ -457,6 +441,21 @@ function TransitionChildFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_
     if (theirProps.className === '') delete theirProps.className
   }
 
+  // If we were never transitioning, or we're not transitioning anymore, then
+  // apply the `enterTo` and `leaveTo` classes as the final state.
+  else {
+    theirProps.className = classNames(
+      rest.className,
+      container.current?.className,
+      ...match(transitionDirection, {
+        enter: [...classes.current.enterTo, ...classes.current.entered],
+        leave: classes.current.leaveTo,
+        idle: [],
+      })
+    )
+    if (theirProps.className === '') delete theirProps.className
+  }
+
   return (
     <NestingContext.Provider value={nesting}>
       <OpenClosedProvider
@@ -504,7 +503,7 @@ function TransitionRootFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_C
     show = (usesOpenClosedState & State.Open) === State.Open
   }
 
-  if (![true, false].includes(show as unknown as boolean)) {
+  if (show === undefined) {
     throw new Error('A <Transition /> is used but it is missing a `show={true | false}` prop.')
   }
 
@@ -532,27 +531,18 @@ function TransitionRootFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_C
   }, [changes, show])
 
   let transitionBag = useMemo<TransitionContextValues>(
-    () => ({ show: show as boolean, appear, initial }),
+    () => ({ show, appear, initial }),
     [show, appear, initial]
   )
 
+  // Ensure we change the tree state to hidden once the transition becomes hidden
+  useOnDisappear(internalTransitionRef, () => setState(TreeStates.Hidden))
+
   useIsoMorphicEffect(() => {
     if (show) {
       setState(TreeStates.Visible)
     } else if (!hasChildren(nestingBag)) {
       setState(TreeStates.Hidden)
-    } else if (
-      process.env.NODE_ENV !==
-      'test' /* TODO: Remove this once we have real tests! JSDOM doesn't "render", therefore getBoundingClientRect() will always result in `0`. */
-    ) {
-      let node = internalTransitionRef.current
-      if (!node) return
-      let rect = node.getBoundingClientRect()
-
-      if (rect.x === 0 && rect.y === 0 && rect.width === 0 && rect.height === 0) {
-        // The node is completely hidden, let's hide it
-        setState(TreeStates.Hidden)
-      }
     }
   }, [show, nestingBag])
 

packages/@headlessui-react/src/components/transition/utils/transition.test.ts

@@ -26,9 +26,9 @@ it('should be possible to transition', async () => {
   )
 
   await new Promise<void>((resolve) => {
-    transition(
-      element,
-      {
+    transition(element, {
+      direction: 'enter', // Show
+      classes: {
         base: [],
         enter: ['enter'],
         enterFrom: ['enterFrom'],
@@ -38,9 +38,8 @@ it('should be possible to transition', async () => {
         leaveTo: [],
         entered: ['entered'],
       },
-      true, // Show
-      resolve
-    )
+      done: resolve,
+    })
   })
 
   await new Promise((resolve) => d.nextFrame(resolve))
@@ -49,13 +48,13 @@ it('should be possible to transition', async () => {
   expect(snapshots[0].content).toEqual('<div></div>')
 
   // Start of transition
-  expect(snapshots[1].content).toEqual('<div class="enter enterFrom"></div>')
+  expect(snapshots[1].content).toEqual('<div style="" class="enter enterFrom"></div>')
 
   // NOTE: There is no `enter enterTo`, because we didn't define a duration. Therefore it is not
   // necessary to put the classes on the element and immediately remove them.
 
   // Cleanup phase
-  expect(snapshots[2].content).toEqual('<div class="enterTo entered"></div>')
+  expect(snapshots[2].content).toEqual('<div style="" class="entered enterTo enter"></div>')
 
   d.dispose()
 })
@@ -84,9 +83,9 @@ it('should wait the correct amount of time to finish a transition', async () =>
   )
 
   await new Promise<void>((resolve) => {
-    transition(
-      element,
-      {
+    transition(element, {
+      direction: 'enter', // Show
+      classes: {
         base: [],
         enter: ['enter'],
         enterFrom: ['enterFrom'],
@@ -96,9 +95,8 @@ it('should wait the correct amount of time to finish a transition', async () =>
         leaveTo: [],
         entered: ['entered'],
       },
-      true, // Show
-      resolve
-    )
+      done: resolve,
+    })
   })
 
   await new Promise((resolve) => d.nextFrame(resolve))
@@ -154,9 +152,9 @@ it('should keep the delay time into account', async () => {
   )
 
   await new Promise<void>((resolve) => {
-    transition(
-      element,
-      {
+    transition(element, {
+      direction: 'enter', // Show
+      classes: {
         base: [],
         enter: ['enter'],
         enterFrom: ['enterFrom'],
@@ -166,9 +164,8 @@ it('should keep the delay time into account', async () => {
         leaveTo: [],
         entered: ['entered'],
       },
-      true, // Show
-      resolve
-    )
+      done: resolve,
+    })
   })
 
   await new Promise((resolve) => d.nextFrame(resolve))