fix(react): add missing destroy cleanups (#1096)

biome.json

@@ -67,6 +67,10 @@
               {
                 "name": "useForceRender",
                 "stableResult": true
+              },
+              {
+                "name": "useLatestRef",
+                "stableResult": true
               }
             ]
           }

packages/core/src/core/ui/buffering-indicator/buffering-indicator-core.ts

@@ -28,6 +28,10 @@ export class BufferingIndicatorCore {
     this.#props = defaults(props, BufferingIndicatorCore.defaultProps);
   }
 
+  destroy(): void {
+    this.#clearTimer();
+  }
+
   update(media: MediaPlaybackState): void {
     const buffering = media.waiting && !media.paused;
 
@@ -37,14 +41,17 @@ export class BufferingIndicatorCore {
         this.state.patch({ visible: true });
       }, this.#props.delay);
     } else if (!buffering) {
-      if (this.#timer !== null) {
-        clearTimeout(this.#timer);
-        this.#timer = null;
-      }
-
+      this.#clearTimer();
       this.state.patch({ visible: false });
     }
   }
+
+  #clearTimer(): void {
+    if (this.#timer !== null) {
+      clearTimeout(this.#timer);
+      this.#timer = null;
+    }
+  }
 }
 
 export namespace BufferingIndicatorCore {

packages/react/src/media/dash-video/index.tsx

@@ -1,21 +1,15 @@
 import { DashMedia } from '@videojs/core/dom/media/dash';
 import type { PropsWithChildren, VideoHTMLAttributes } from 'react';
-import { forwardRef, useMemo } from 'react';
-import { useMediaAttach } from '../../player/context';
+import { forwardRef } from 'react';
 import { attachMediaElement } from '../../utils/attach-media-element';
 import { mediaProps } from '../../utils/media-props';
 import { useComposedRefs } from '../../utils/use-composed-refs';
-import { useDestroy } from '../../utils/use-destroy';
+import { useMediaInstance } from '../../utils/use-media-instance';
 
 export type DashVideoProps = PropsWithChildren<VideoHTMLAttributes<HTMLVideoElement>>;
 
 export const DashVideo = forwardRef<HTMLVideoElement, DashVideoProps>(({ children, ...props }, ref) => {
-  const mediaApi = useMemo(() => new DashMedia(), []);
-  const setMedia = useMediaAttach();
-
-  useDestroy(mediaApi, () => {
-    setMedia?.(mediaApi);
-  });
+  const mediaApi = useMediaInstance(DashMedia);
 
   const composedRef = useComposedRefs(attachMediaElement(mediaApi), ref);
 

packages/react/src/media/hls-video/index.tsx

@@ -1,21 +1,15 @@
 import { HlsMedia } from '@videojs/core/dom/media/hls';
 import type { PropsWithChildren, VideoHTMLAttributes } from 'react';
-import { forwardRef, useMemo } from 'react';
-import { useMediaAttach } from '../../player/context';
+import { forwardRef } from 'react';
 import { attachMediaElement } from '../../utils/attach-media-element';
 import { mediaProps } from '../../utils/media-props';
 import { useComposedRefs } from '../../utils/use-composed-refs';
-import { useDestroy } from '../../utils/use-destroy';
+import { useMediaInstance } from '../../utils/use-media-instance';
 
 export type HlsVideoProps = PropsWithChildren<VideoHTMLAttributes<HTMLVideoElement>>;
 
 export const HlsVideo = forwardRef<HTMLVideoElement, HlsVideoProps>(({ children, ...props }, ref) => {
-  const mediaApi = useMemo(() => new HlsMedia(), []);
-  const setMedia = useMediaAttach();
-
-  useDestroy(mediaApi, () => {
-    setMedia?.(mediaApi);
-  });
+  const mediaApi = useMediaInstance(HlsMedia);
 
   const composedRef = useComposedRefs(attachMediaElement(mediaApi), ref);
 

packages/react/src/media/simple-hls-video/index.tsx

@@ -1,20 +1,15 @@
 import { SimpleHlsMedia } from '@videojs/core/dom/media/simple-hls';
 import type { PropsWithChildren, VideoHTMLAttributes } from 'react';
-import { forwardRef, useEffect, useMemo } from 'react';
-import { useMediaAttach } from '../../player/context';
+import { forwardRef } from 'react';
 import { attachMediaElement } from '../../utils/attach-media-element';
 import { mediaProps } from '../../utils/media-props';
 import { useComposedRefs } from '../../utils/use-composed-refs';
+import { useMediaInstance } from '../../utils/use-media-instance';
 
 export type SimpleHlsVideoProps = PropsWithChildren<VideoHTMLAttributes<HTMLVideoElement>>;
 
 export const SimpleHlsVideo = forwardRef<HTMLVideoElement, SimpleHlsVideoProps>(({ children, ...props }, ref) => {
-  const mediaApi = useMemo(() => new SimpleHlsMedia(), []);
-  const setMedia = useMediaAttach();
-
-  useEffect(() => {
-    setMedia?.(mediaApi);
-  }, [mediaApi, setMedia]);
+  const mediaApi = useMediaInstance(SimpleHlsMedia);
 
   const composedRef = useComposedRefs(attachMediaElement(mediaApi), ref);
 

packages/react/src/ui/buffering-indicator/buffering-indicator.tsx

@@ -7,6 +7,7 @@ import { forwardRef, useState, useSyncExternalStore } from 'react';
 
 import { usePlayer } from '../../player/context';
 import type { UIComponentProps } from '../../utils/types';
+import { useDestroy } from '../../utils/use-destroy';
 import { renderElement } from '../../utils/use-render';
 
 export interface BufferingIndicatorProps
@@ -40,6 +41,7 @@ export const BufferingIndicator = forwardRef(function BufferingIndicator(
   const playback = usePlayer(selectPlayback);
 
   const [core] = useState(() => new BufferingIndicatorCore());
+  useDestroy(core);
   core.setProps({ delay });
 
   if (playback) core.update(playback);

packages/react/src/ui/tooltip/tooltip-root.tsx

@@ -6,6 +6,7 @@ import { useSnapshot } from '@videojs/store/react';
 import type { ReactNode } from 'react';
 import { useEffect, useState } from 'react';
 
+import { useDestroy } from '../../utils/use-destroy';
 import { useLatestRef } from '../../utils/use-latest-ref';
 import { useSafeId } from '../../utils/use-safe-id';
 import { TooltipContextProvider } from './context';
@@ -89,8 +90,7 @@ export function TooltipRoot({
     }
   }, [controlledOpen, tooltip]);
 
-  // Cleanup on unmount
-  useEffect(() => () => tooltip.destroy(), [tooltip]);
+  useDestroy(tooltip);
 
   const input = useSnapshot(tooltip.input);
   core.setInput(input);

packages/react/src/utils/tests/use-destroy.test.tsx

@@ -109,4 +109,40 @@ describe('useDestroy', () => {
     // Setup should only run once — the re-mount skips it
     expect(setup).toHaveBeenCalledOnce();
   });
+
+  it('calls teardown before destroy on unmount', () => {
+    const order: string[] = [];
+    const instance = { destroy: vi.fn(() => order.push('destroy')) };
+    const teardown = vi.fn(() => order.push('teardown'));
+
+    const { unmount } = renderHook(() => useDestroy(instance, undefined, teardown));
+
+    unmount();
+    vi.runAllTimers();
+
+    expect(teardown).toHaveBeenCalledOnce();
+    expect(instance.destroy).toHaveBeenCalledOnce();
+    expect(order).toEqual(['teardown', 'destroy']);
+  });
+
+  it('does not call teardown in StrictMode double-mount', () => {
+    const instance = { destroy: vi.fn() };
+    const teardown = vi.fn();
+
+    function TestComponent() {
+      useDestroy(instance, undefined, teardown);
+      return null;
+    }
+
+    render(
+      <StrictMode>
+        <TestComponent />
+      </StrictMode>
+    );
+
+    vi.runAllTimers();
+
+    expect(teardown).not.toHaveBeenCalled();
+    expect(instance.destroy).not.toHaveBeenCalled();
+  });
 });

packages/react/src/utils/use-destroy.ts

@@ -2,6 +2,8 @@
 
 import { useEffect, useRef } from 'react';
 
+import { useLatestRef } from './use-latest-ref';
+
 interface Destroyable {
   destroy(): void;
 }
@@ -17,20 +19,27 @@ interface Destroyable {
  * @param instance - Object with a `destroy()` method.
  * @param setup - Optional setup called on first mount. Skipped on StrictMode
  *   re-mount since the previous setup was never torn down.
+ * @param teardown - Optional teardown called right before `destroy()` on real
+ *   unmount. Skipped on StrictMode simulated unmount.
  */
-export function useDestroy(instance: Destroyable, setup?: () => void): void {
+export function useDestroy(instance: Destroyable, setup?: () => void, teardown?: () => void): void {
   const pendingRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const setupRef = useLatestRef(setup);
+  const teardownRef = useLatestRef(teardown);
 
   useEffect(() => {
     if (pendingRef.current !== null) {
       clearTimeout(pendingRef.current);
       pendingRef.current = null;
     } else {
-      setup?.();
+      setupRef.current?.();
     }
 
     return () => {
-      pendingRef.current = setTimeout(() => instance.destroy(), 0);
+      pendingRef.current = setTimeout(() => {
+        teardownRef.current?.();
+        instance.destroy();
+      }, 0);
     };
-  }, [instance, setup]);
+  }, [instance]);
 }

packages/react/src/utils/use-media-instance.ts

@@ -0,0 +1,29 @@
+'use client';
+
+import type { Media } from '@videojs/core/dom';
+import { useState } from 'react';
+
+import { useMediaAttach } from '../player/context';
+import { useDestroy } from './use-destroy';
+
+/**
+ * Create and manage a media instance lifecycle within a player context.
+ *
+ * Instantiates the media class once, attaches it to the player on mount,
+ * and safely detaches on unmount using a functional updater to avoid race
+ * conditions when swapping media components (e.g. DashVideo → HlsVideo).
+ */
+export function useMediaInstance<Instance extends Media & { destroy(): void }>(
+  MediaClass: new () => Instance
+): Instance {
+  const [instance] = useState(() => new MediaClass());
+  const setMedia = useMediaAttach();
+
+  useDestroy(
+    instance,
+    () => setMedia?.(instance),
+    () => setMedia?.((prev) => (prev === instance ? null : prev))
+  );
+
+  return instance;
+}