From 058fb8cfdd32d3ee514b43a48e3dbf2dd09b1020 Mon Sep 17 00:00:00 2001 From: Ronald Urbina <140639086+ronald-urbina@users.noreply.github.com> Date: Tue, 30 Jun 2026 14:48:00 -0300 Subject: [PATCH] fix(react): recover from StoreError: DESTROYED on React hide/reveal (#1587) Co-authored-by: Manuel Calleriza --- packages/react/src/player/create-player.tsx | 11 ++- .../src/player/tests/create-player.test.tsx | 80 ++++++++++++++++++- 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/packages/react/src/player/create-player.tsx b/packages/react/src/player/create-player.tsx index f8d0adac..d74b9307 100644 --- a/packages/react/src/player/create-player.tsx +++ b/packages/react/src/player/create-player.tsx @@ -70,7 +70,7 @@ export function createPlayer( export function createPlayer(config: CreatePlayerConfig): CreatePlayerResult { function Provider({ children }: ProviderProps): ReactNode { - const [store] = useState(() => createStore()(combine(...config.features))); + const [store, setStore] = useState(() => createStore()(combine(...config.features))); const [popupGroup] = useState(() => createPopupGroup()); const [media, setMedia] = useState(null); const [container, setContainer] = useState(null); @@ -79,6 +79,15 @@ export function createPlayer(config: CreatePlayerConfig): Cr useEffect(() => { if (!media) return; + + // The store may have been destroyed during an asynchronous gap between React + // effect cleanup and re-setup (e.g., React hide → reveal). The + // useState initializer does not re-run in this case. + if (store.destroyed) { + setStore(createStore()(combine(...config.features))); + return; + } + return store.attach({ media, container }); }, [media, container, store]); diff --git a/packages/react/src/player/tests/create-player.test.tsx b/packages/react/src/player/tests/create-player.test.tsx index 54bfc319..f981ad82 100644 --- a/packages/react/src/player/tests/create-player.test.tsx +++ b/packages/react/src/player/tests/create-player.test.tsx @@ -1,9 +1,10 @@ -import { render, renderHook } from '@testing-library/react'; +import { act, render, renderHook } from '@testing-library/react'; import type { PlayerStore } from '@videojs/core/dom'; import { defineSlice } from '@videojs/store'; import type { ReactNode } from 'react'; import { StrictMode } from 'react'; import { describe, expect, it, vi } from 'vitest'; +import { usePlayerContext } from '../context'; import { createPlayer } from '../create-player'; describe('createPlayer', () => { @@ -66,6 +67,47 @@ describe('createPlayer', () => { vi.useRealTimers(); }); + it('recovers after Activity-style async destroy (React )', () => { + const { Provider, usePlayer } = createPlayer({ features: [mockSlice] }); + + let store!: PlayerStore; + // Captured inside TestComponent so we can trigger a media-dep change + // from the test body, simulating Activity reveal re-running the attach effect. + let setMediaFn!: (media: HTMLMediaElement | null) => void; + + function TestComponent() { + store = usePlayer(); + const { setMedia } = usePlayerContext(); + setMediaFn = setMedia; + return null; + } + + render( + + + + ); + + const originalStore = store; + expect(originalStore.destroyed).toBe(false); + + // Simulate the Activity gap: the deferred timeout fires before React gets + // a chance to re-run effects, leaving the store destroyed. + originalStore.destroy(); + expect(originalStore.destroyed).toBe(true); + + // Mirrors the real app: Activity reveals the subtree with an already-attached media element. + expect(() => { + act(() => { + setMediaFn(document.createElement('video')); + }); + }).not.toThrow(); + + expect(store).toBeDefined(); + expect(store.destroyed).toBe(false); + expect(store).not.toBe(originalStore); + }); + it('survives React StrictMode without StoreError', () => { const { Provider, usePlayer } = createPlayer({ features: [mockSlice] }); @@ -90,6 +132,42 @@ describe('createPlayer', () => { expect(store.destroyed).toBe(false); }); + it('StrictMode: preserves the same store instance and cancels the pending destroy', () => { + vi.useFakeTimers(); + + const { Provider, usePlayer } = createPlayer({ features: [mockSlice] }); + + // Track every store instance the component sees across all renders. + const seenStores = new Set(); + let currentStore!: PlayerStore; + + function TestComponent() { + currentStore = usePlayer(); + seenStores.add(currentStore); + return null; + } + + render( + + + + + + ); + + // Flush timers — the deferred destroy was scheduled during StrictMode's + // simulated cleanup. If it was NOT cancelled by the re-mount effect, the + // store would be destroyed here. + vi.runAllTimers(); + + // The Activity guard must not have fired: one store instance, not two. + // (setStore would have been called and produced a second instance.) + expect(seenStores.size).toBe(1); + expect(currentStore.destroyed).toBe(false); + + vi.useRealTimers(); + }); + it('uses displayName when provided', () => { const { Provider } = createPlayer({ features: [mockSlice],