fix(react): strict mode support (#742)

This commit is contained in:
rahim
2026-03-05 13:03:16 -08:00
committed by GitHub
parent 4ae7f41a80
commit 44d874ddc2
7 changed files with 189 additions and 12 deletions
+3 -2
View File
@@ -15,8 +15,9 @@ import type { InferStoreState } from '@videojs/store';
import { combine, createStore } from '@videojs/store';
import { useStore } from '@videojs/store/react';
import type { FC, ReactNode } from 'react';
import { useEffect, useState } from 'react';
import { useState } from 'react';
import { useDestroy } from '../utils/use-destroy';
import { Container, PlayerContextProvider, useMedia, usePlayerContext } from './context';
export interface CreatePlayerConfig<Features extends AnyPlayerFeature[]> {
@@ -71,7 +72,7 @@ export function createPlayer(config: CreatePlayerConfig<AnyPlayerFeature[]>): Cr
const [store] = useState(() => createStore<PlayerTarget>()(combine(...config.features)));
const [media, setMedia] = useState<Media | null>(null);
useEffect(() => () => store.destroy(), [store]);
useDestroy(store);
return <PlayerContextProvider value={{ store, media, setMedia }}>{children}</PlayerContextProvider>;
}
@@ -2,6 +2,7 @@ import { 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 { createPlayer } from '../create-player';
@@ -39,6 +40,8 @@ describe('createPlayer', () => {
});
it('destroys store on unmount', () => {
vi.useFakeTimers();
const { Provider, usePlayer } = createPlayer({ features: [mockSlice] });
let store!: PlayerStore;
@@ -56,8 +59,35 @@ describe('createPlayer', () => {
const destroySpy = vi.spyOn(store, 'destroy');
unmount();
vi.runAllTimers();
expect(destroySpy).toHaveBeenCalled();
vi.useRealTimers();
});
it('survives React StrictMode without StoreError', () => {
const { Provider, usePlayer } = createPlayer({ features: [mockSlice] });
let store!: PlayerStore;
function TestComponent() {
store = usePlayer();
return null;
}
expect(() => {
render(
<StrictMode>
<Provider>
<TestComponent />
</Provider>
</StrictMode>
);
}).not.toThrow();
expect(store).toBeDefined();
expect(store.destroyed).toBe(false);
});
it('uses displayName when provided', () => {
+3 -3
View File
@@ -10,7 +10,8 @@ import {
} from '@videojs/core/dom';
import { useSnapshot } from '@videojs/store/react';
import { isRTL } from '@videojs/utils/dom';
import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react';
import { useCallback, useLayoutEffect, useRef, useState } from 'react';
import { useDestroy } from '../../utils/use-destroy';
import { useForceRender } from '../../utils/use-force-render';
import { useLatestRef } from '../../utils/use-latest-ref';
@@ -82,8 +83,7 @@ export function useSlider<State extends SliderState = SliderState>(
return createSlider(stableOptions);
});
// Cleanup on unmount.
useEffect(() => () => slider.destroy(), [slider]);
useDestroy(slider);
// Subscribe to slider input state.
const input = useSnapshot(slider.input);
@@ -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 { PopoverContextProvider } from './context';
@@ -85,8 +86,7 @@ export function PopoverRoot({
}
}, [controlledOpen, popover]);
// Cleanup on unmount
useEffect(() => () => popover.destroy(), [popover]);
useDestroy(popover);
const input = useSnapshot(popover.input);
core.setInput(input);
@@ -9,10 +9,11 @@ import {
} from '@videojs/core';
import { createThumbnail, selectTextTrack } from '@videojs/core/dom';
import type { CSSProperties } from 'react';
import { forwardRef, useEffect, useMemo, useRef, useState } from 'react';
import { forwardRef, useMemo, useRef, useState } from 'react';
import { useOptionalPlayer } from '../../player/context';
import type { UIComponentProps } from '../../utils/types';
import { useDestroy } from '../../utils/use-destroy';
import { renderElement } from '../../utils/use-render';
export interface ThumbnailProps extends UIComponentProps<'div', ThumbnailCore.State>, ThumbnailCore.Props {
@@ -49,10 +50,7 @@ export const Thumbnail = forwardRef<HTMLDivElement, ThumbnailProps>(function Thu
})
);
useEffect(() => {
handle.connect();
return () => handle.destroy();
}, [handle]);
useDestroy(handle, () => handle.connect());
// Resolve thumbnails: external prop takes priority over auto <track> path.
const thumbnails = useMemo(() => {
@@ -0,0 +1,112 @@
import { render, renderHook } from '@testing-library/react';
import { StrictMode } from 'react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { useDestroy } from '../use-destroy';
describe('useDestroy', () => {
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it('destroys instance after unmount', () => {
const instance = { destroy: vi.fn() };
const { unmount } = renderHook(() => useDestroy(instance));
expect(instance.destroy).not.toHaveBeenCalled();
unmount();
vi.runAllTimers();
expect(instance.destroy).toHaveBeenCalledOnce();
});
it('does not destroy synchronously in cleanup', () => {
const instance = { destroy: vi.fn() };
const { unmount } = renderHook(() => useDestroy(instance));
unmount();
// Destroy is deferred — not called yet
expect(instance.destroy).not.toHaveBeenCalled();
vi.runAllTimers();
expect(instance.destroy).toHaveBeenCalledOnce();
});
it('does not destroy in StrictMode double-mount', () => {
const instance = { destroy: vi.fn() };
function TestComponent() {
useDestroy(instance);
return null;
}
render(
<StrictMode>
<TestComponent />
</StrictMode>
);
vi.runAllTimers();
// StrictMode runs cleanup then re-mount — destroy should be cancelled
expect(instance.destroy).not.toHaveBeenCalled();
});
it('destroys after real unmount in StrictMode', () => {
const instance = { destroy: vi.fn() };
function TestComponent() {
useDestroy(instance);
return null;
}
const { unmount } = render(
<StrictMode>
<TestComponent />
</StrictMode>
);
vi.runAllTimers();
expect(instance.destroy).not.toHaveBeenCalled();
unmount();
vi.runAllTimers();
expect(instance.destroy).toHaveBeenCalledOnce();
});
it('calls setup once on mount', () => {
const instance = { destroy: vi.fn() };
const setup = vi.fn();
renderHook(() => useDestroy(instance, setup));
expect(setup).toHaveBeenCalledOnce();
});
it('calls setup once in StrictMode (skips re-mount)', () => {
const instance = { destroy: vi.fn() };
const setup = vi.fn();
function TestComponent() {
useDestroy(instance, setup);
return null;
}
render(
<StrictMode>
<TestComponent />
</StrictMode>
);
// Setup should only run once — the re-mount skips it
expect(setup).toHaveBeenCalledOnce();
});
});
+36
View File
@@ -0,0 +1,36 @@
'use client';
import { useEffect, useRef } from 'react';
interface Destroyable {
destroy(): void;
}
/**
* Destroy an instance on unmount, deferring destruction to survive React
* StrictMode's simulated unmount/re-mount cycle.
*
* StrictMode runs effects, then immediately runs cleanup, then re-runs
* effects all synchronously. By deferring `destroy()` to a macrotask,
* the re-mount effect can cancel it before it fires.
*
* @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.
*/
export function useDestroy(instance: Destroyable, setup?: () => void): void {
const pendingRef = useRef<ReturnType<typeof setTimeout> | null>(null);
useEffect(() => {
if (pendingRef.current !== null) {
clearTimeout(pendingRef.current);
pendingRef.current = null;
} else {
setup?.();
}
return () => {
pendingRef.current = setTimeout(() => instance.destroy(), 0);
};
}, [instance, setup]);
}