fix(react): recover from StoreError: DESTROYED on React <Activity> hide/reveal (#1587)

Co-authored-by: Manuel Calleriza <mcalleriza@mux.com>
This commit is contained in:
Ronald Urbina
2026-06-30 10:48:00 -07:00
committed by GitHub
co-authored by Manuel Calleriza
parent 3f2f4a4a5a
commit 058fb8cfdd
2 changed files with 89 additions and 2 deletions
+10 -1
View File
@@ -70,7 +70,7 @@ export function createPlayer<const Features extends AnyPlayerFeature[]>(
export function createPlayer(config: CreatePlayerConfig<AnyPlayerFeature[]>): CreatePlayerResult<AnyPlayerStore> {
function Provider({ children }: ProviderProps): ReactNode {
const [store] = useState(() => createStore<PlayerTarget>()(combine(...config.features)));
const [store, setStore] = useState(() => createStore<PlayerTarget>()(combine(...config.features)));
const [popupGroup] = useState(() => createPopupGroup());
const [media, setMedia] = useState<Media | null>(null);
const [container, setContainer] = useState<HTMLElement | null>(null);
@@ -79,6 +79,15 @@ export function createPlayer(config: CreatePlayerConfig<AnyPlayerFeature[]>): 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 <Activity> hide → reveal). The
// useState initializer does not re-run in this case.
if (store.destroyed) {
setStore(createStore<PlayerTarget>()(combine(...config.features)));
return;
}
return store.attach({ media, container });
}, [media, container, store]);
@@ -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 <Activity>)', () => {
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(
<Provider>
<TestComponent />
</Provider>
);
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<PlayerStore>();
let currentStore!: PlayerStore;
function TestComponent() {
currentStore = usePlayer();
seenStores.add(currentStore);
return null;
}
render(
<StrictMode>
<Provider>
<TestComponent />
</Provider>
</StrictMode>
);
// 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],