diff --git a/packages/react/src/ui/input-indicator/tests/use-input-indicator-root.test.tsx b/packages/react/src/ui/input-indicator/tests/use-input-indicator-root.test.tsx new file mode 100644 index 0000000000..9b491e3f19 --- /dev/null +++ b/packages/react/src/ui/input-indicator/tests/use-input-indicator-root.test.tsx @@ -0,0 +1,100 @@ +import { cleanup, render } from '@testing-library/react'; +import type { IndicatorLifecycleState } from '@videojs/core'; +import { createState } from '@videojs/store'; +import { type ReactNode, StrictMode } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vite-plus/test'; + +import { createPlayerWrapper, MockErrorBoundary } from '../../../testing/mocks'; +import { useInputIndicatorRoot } from '../use-input-indicator-root'; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +function createCore() { + return { + state: createState({ + open: false, + generation: 0, + transitionStarting: false, + transitionEnding: false, + }), + setProps: vi.fn(), + destroy: vi.fn(), + close: vi.fn(), + processEvent: vi.fn(() => false), + }; +} + +function Thrower({ abandon }: { abandon: boolean }): ReactNode { + if (abandon) throw new Error('abandon render'); + + return null; +} + +describe('useInputIndicatorRoot', () => { + it('does not publish props from an abandoned render to the retained core', () => { + const core = createCore(); + const { Wrapper } = createPlayerWrapper(); + + vi.spyOn(console, 'error').mockImplementation(() => {}); + + function Probe({ value }: { value: string }) { + useInputIndicatorRoot(() => core, { value }); + return null; + } + + const { rerender } = render( + + + + + + + ); + + expect(core.setProps).toHaveBeenCalledWith({ value: 'committed' }); + + rerender( + + + + + + + ); + + expect(core.setProps).not.toHaveBeenCalledWith({ value: 'abandoned' }); + }); + + it('publishes the latest committed props under StrictMode', () => { + const core = createCore(); + const { Wrapper } = createPlayerWrapper(); + + function Probe({ value }: { value: string }) { + useInputIndicatorRoot(() => core, { value }); + return null; + } + + const { rerender } = render( + + + + + + ); + + expect(core.setProps).toHaveBeenLastCalledWith({ value: 'first' }); + + rerender( + + + + + + ); + + expect(core.setProps).toHaveBeenLastCalledWith({ value: 'second' }); + }); +}); diff --git a/packages/react/src/ui/input-indicator/use-indicator-visibility.ts b/packages/react/src/ui/input-indicator/use-indicator-visibility.ts index 183b255661..1b8d456af2 100644 --- a/packages/react/src/ui/input-indicator/use-indicator-visibility.ts +++ b/packages/react/src/ui/input-indicator/use-indicator-visibility.ts @@ -3,11 +3,11 @@ import { getIndicatorVisibilityCoordinator } from '@videojs/core/dom'; import { useCallback, useEffect, useRef, useState } from 'react'; import { useContainer } from '../../player/context'; -import { useLatestRef } from '../../utils/use-latest-ref'; +import { useCommittedRef } from '../../utils/use-committed-ref'; export function useIndicatorVisibility(close: () => void): () => void { const container = useContainer(); - const closeRef = useLatestRef(close); + const closeRef = useCommittedRef(close); const coordinatorRef = useRef | null>(null); const [handle] = useState(() => ({ close: () => closeRef.current(), diff --git a/packages/react/src/ui/input-indicator/use-input-action-subscription.ts b/packages/react/src/ui/input-indicator/use-input-action-subscription.ts index cd2584c47f..f1a131ff47 100644 --- a/packages/react/src/ui/input-indicator/use-input-action-subscription.ts +++ b/packages/react/src/ui/input-indicator/use-input-action-subscription.ts @@ -3,13 +3,13 @@ import { getMediaSnapshot, subscribeToInputActions } from '@videojs/core/dom'; import { useEffect } from 'react'; import { useContainer, usePlayer } from '../../player/context'; -import { useLatestRef } from '../../utils/use-latest-ref'; +import { useCommittedRef } from '../../utils/use-committed-ref'; export function useInputActionSubscription(callback: (event: InputActionEvent, snapshot: MediaSnapshot) => void): void { const container = useContainer(); const store = usePlayer(); - const callbackRef = useLatestRef(callback); - const storeRef = useLatestRef(store); + const callbackRef = useCommittedRef(callback); + const storeRef = useCommittedRef(store); useEffect(() => { if (!container) return; diff --git a/packages/react/src/ui/input-indicator/use-input-indicator-root.ts b/packages/react/src/ui/input-indicator/use-input-indicator-root.ts index 5bb8eca124..6df58336da 100644 --- a/packages/react/src/ui/input-indicator/use-input-indicator-root.ts +++ b/packages/react/src/ui/input-indicator/use-input-indicator-root.ts @@ -3,6 +3,7 @@ import type { State as StoreState } from '@videojs/store'; import { useState, useSyncExternalStore } from 'react'; import { useDestroy } from '../../utils/use-destroy'; +import { useIsomorphicLayoutEffect } from '../../utils/use-isomorphic-layout-effect'; import { useIndicatorVisibility } from './use-indicator-visibility'; import { useInputActionSubscription } from './use-input-action-subscription'; import { type RenderedIndicatorOptions, useRenderedIndicatorState } from './use-rendered-indicator-state'; @@ -23,7 +24,10 @@ export function useInputIndicatorRoot core.setProps(props), [core, props]); + const showIndicator = useIndicatorVisibility(() => core.close()); useInputActionSubscription((event, snapshot) => { diff --git a/packages/react/src/ui/status-announcer/status-announcer.tsx b/packages/react/src/ui/status-announcer/status-announcer.tsx index 3dcccf5752..8c91f1c69c 100644 --- a/packages/react/src/ui/status-announcer/status-announcer.tsx +++ b/packages/react/src/ui/status-announcer/status-announcer.tsx @@ -7,6 +7,7 @@ import { useLocale, useTranslator } from '../../i18n/context'; import { useContainer, usePlayer } from '../../player/context'; import type { UIComponentProps } from '../../utils/types'; import { useDestroy } from '../../utils/use-destroy'; +import { useIsomorphicLayoutEffect } from '../../utils/use-isomorphic-layout-effect'; import { renderElement } from '../../utils/use-render'; export interface StatusAnnouncerProps @@ -26,14 +27,18 @@ export const StatusAnnouncer = forwardRef(function StatusAnnouncer( const container = useContainer(); useDestroy(core); - core.setProps({ - closeDelay, - labels: { - ...createStatusAnnouncerLabels(translator, locale), - ...labels, - }, - shouldAnnounce: () => shouldAnnounceStatusChange(container), - }); + + // Commit props before the passive store subscription below so announcements never read props from an abandoned render. + useIsomorphicLayoutEffect(() => { + core.setProps({ + closeDelay, + labels: { + ...createStatusAnnouncerLabels(translator, locale), + ...labels, + }, + shouldAnnounce: () => shouldAnnounceStatusChange(container), + }); + }, [core, closeDelay, translator, locale, labels, container]); useEffect(() => subscribeToStatusAnnouncer(store, core), [core, store]); diff --git a/packages/react/src/ui/status-announcer/tests/status-announcer.test.tsx b/packages/react/src/ui/status-announcer/tests/status-announcer.test.tsx index 417fa7da01..e49defd00d 100644 --- a/packages/react/src/ui/status-announcer/tests/status-announcer.test.tsx +++ b/packages/react/src/ui/status-announcer/tests/status-announcer.test.tsx @@ -1,9 +1,12 @@ import { act, cleanup, render } from '@testing-library/react'; +import { StatusAnnouncerCore } from '@videojs/core'; import type { UnknownStore } from '@videojs/store'; -import type { ReactNode } from 'react'; +import { isString } from '@videojs/utils/predicate'; +import { type ReactNode, StrictMode } from 'react'; import { afterEach, describe, expect, it, vi } from 'vite-plus/test'; import { PlayerContextProvider, type PlayerContextValue } from '../../../player/context'; +import { MockErrorBoundary } from '../../../testing/mocks'; import { StatusAnnouncer } from '../status-announcer'; afterEach(cleanup); @@ -67,6 +70,68 @@ describe('StatusAnnouncer', () => { expect(getByRole('status').textContent).toBe('Custom playing'); }); + it('does not publish props from an abandoned render to the retained core', () => { + const labels: string[] = []; + const originalSetProps = StatusAnnouncerCore.prototype.setProps; + const setProps = vi + .spyOn(StatusAnnouncerCore.prototype, 'setProps') + .mockImplementation(function (this: StatusAnnouncerCore, props) { + if (isString(props.labels?.playing)) labels.push(props.labels.playing); + + originalSetProps.call(this, props); + }); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + const value = createPlayerContextValue(createTestStore().store); + + function Thrower({ abandon }: { abandon: boolean }): ReactNode { + if (abandon) throw new Error('abandon render'); + + return null; + } + + const { rerender } = render( + + + + + + + ); + + expect(labels).toContain('Committed'); + + rerender( + + + + + + + ); + + expect(labels).not.toContain('Abandoned'); + setProps.mockRestore(); + consoleError.mockRestore(); + }); + + it('announces with the committed labels under StrictMode', async () => { + const { store, setState } = createTestStore({ paused: true }); + const { getByRole } = render( + + + + + + ); + + await act(async () => {}); + + setState({ paused: false }); + await act(async () => {}); + + expect(getByRole('status').textContent).toBe('Strict playing'); + }); + it('uses the next store snapshot as baseline when the store changes', async () => { const first = createTestStore({ paused: false }); const second = createTestStore({ paused: false }); diff --git a/packages/react/src/ui/tooltip/tests/tooltip-provider.test.tsx b/packages/react/src/ui/tooltip/tests/tooltip-provider.test.tsx new file mode 100644 index 0000000000..786185f21c --- /dev/null +++ b/packages/react/src/ui/tooltip/tests/tooltip-provider.test.tsx @@ -0,0 +1,110 @@ +import { cleanup, render } from '@testing-library/react'; +import type { TooltipGroupCore } from '@videojs/core'; +import { type ReactNode, StrictMode, useLayoutEffect } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vite-plus/test'; + +import { MockErrorBoundary } from '../../../testing/mocks'; +import { useTooltipGroup } from '../group-context'; +import { TooltipProvider } from '../tooltip-provider'; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +function LayoutGroupDelay({ commit, observed }: { commit: string; observed: number[] }) { + const group = useTooltipGroup(); + + useLayoutEffect(() => { + if (commit && group) observed.push(group.delay); + }, [commit, group, observed]); + + return null; +} + +function CaptureGroup({ onGroup }: { onGroup: (group: TooltipGroupCore | undefined) => void }) { + onGroup(useTooltipGroup()); + return null; +} + +function Thrower({ abandon }: { abandon: boolean }): ReactNode { + if (abandon) throw new Error('abandon render'); + + return null; +} + +describe('TooltipProvider', () => { + it('publishes props before descendant layout effects', () => { + const observed: number[] = []; + const { rerender } = render( + + + + ); + + rerender( + + + + ); + + expect(observed).toEqual([100, 200]); + }); + + it('does not publish props from an abandoned render', () => { + let group: TooltipGroupCore | undefined; + const capture = (value: TooltipGroupCore | undefined) => { + group = value; + }; + + vi.spyOn(console, 'error').mockImplementation(() => {}); + const { rerender } = render( + + + + + + + ); + + expect(group?.delay).toBe(100); + + rerender( + + + + + + + ); + + expect(group?.delay).toBe(100); + }); + + it('publishes the latest props under StrictMode', () => { + let group: TooltipGroupCore | undefined; + const capture = (value: TooltipGroupCore | undefined) => { + group = value; + }; + + const { rerender } = render( + + + + + + ); + + expect(group?.delay).toBe(100); + + rerender( + + + + + + ); + + expect(group?.delay).toBe(200); + }); +}); diff --git a/packages/react/src/ui/tooltip/tooltip-provider.tsx b/packages/react/src/ui/tooltip/tooltip-provider.tsx index 119f973ae2..5f589d2945 100644 --- a/packages/react/src/ui/tooltip/tooltip-provider.tsx +++ b/packages/react/src/ui/tooltip/tooltip-provider.tsx @@ -1,6 +1,6 @@ import { TooltipGroupCore, type TooltipGroupProps } from '@videojs/core'; import type { ReactNode } from 'react'; -import { useState } from 'react'; +import { useInsertionEffect, useState } from 'react'; import { TooltipGroupContextProvider } from './group-context'; @@ -11,7 +11,11 @@ export interface TooltipProviderProps extends TooltipGroupProps { export function TooltipProvider({ delay, closeDelay, timeout, children }: TooltipProviderProps): ReactNode { const [group] = useState(() => new TooltipGroupCore({ delay, closeDelay, timeout })); - group.setProps({ delay, closeDelay, timeout }); + // Insertion timing publishes the props before any descendant tooltip's layout effect reads the shared group, while + // still skipping abandoned renders. + useInsertionEffect(() => { + group.setProps({ delay, closeDelay, timeout }); + }, [group, delay, closeDelay, timeout]); return {children}; }