refactor(core)!: tighten constrained jsx boundary

This commit is contained in:
Rahim
2026-06-20 15:37:34 -07:00
parent 5fcab69d34
commit 99613ad768
31 changed files with 314 additions and 400 deletions
+4
View File
@@ -12,12 +12,16 @@ export * from './ui/captions-radio-group/captions-radio-group-core';
export * from './ui/captions-radio-group/captions-radio-group-data-attrs';
export * from './ui/cast-button/cast-button-core';
export * from './ui/cast-button/cast-button-data-attrs';
export * from './ui/container/container-core';
export * from './ui/controls/controls-core';
export * from './ui/controls/controls-data-attrs';
export * from './ui/error-dialog/error-dialog-core';
export * from './ui/error-dialog/error-dialog-data-attrs';
export * from './ui/fullscreen-button/fullscreen-button-core';
export * from './ui/fullscreen-button/fullscreen-button-data-attrs';
export * from './ui/gesture/gesture-core';
export * from './ui/hotkey/hotkey-core';
export * from './ui/input-action';
export * from './ui/input-feedback/indicator-lifecycle';
export * from './ui/input-feedback/seek-indicator-core';
export * from './ui/input-feedback/seek-indicator-data-attrs';
@@ -7,9 +7,12 @@ import BufferingIndicatorDef from './buffering-indicator/buffering-indicator-com
import CaptionsButtonDef from './captions-button/captions-button-component';
import CaptionsRadioGroupDef from './captions-radio-group/captions-radio-group-component';
import CastButtonDef from './cast-button/cast-button-component';
import ContainerDef from './container/container-component';
import ControlsDef from './controls/controls-component';
import ErrorDialogDef from './error-dialog/error-dialog-component';
import FullscreenButtonDef from './fullscreen-button/fullscreen-button-component';
import GestureDef from './gesture/gesture-component';
import HotkeyDef from './hotkey/hotkey-component';
import LiveButtonDef from './live-button/live-button-component';
import MenuDef from './menu/menu-component';
import MuteButtonDef from './mute-button/mute-button-component';
@@ -38,9 +41,12 @@ export const BufferingIndicator = createComponent(BufferingIndicatorDef);
export const CaptionsButton = createComponent(CaptionsButtonDef);
export const CaptionsRadioGroup = createComponent(CaptionsRadioGroupDef);
export const CastButton = createComponent(CastButtonDef);
export const Container = createComponent(ContainerDef);
export const Controls = createComponent(ControlsDef);
export const ErrorDialog = createComponent(ErrorDialogDef);
export const FullscreenButton = createComponent(FullscreenButtonDef);
export const Gesture = createComponent(GestureDef);
export const Hotkey = createComponent(HotkeyDef);
export const LiveButton = createComponent(LiveButtonDef);
export const Menu = createComponent(MenuDef);
export const MuteButton = createComponent(MuteButtonDef);
@@ -70,9 +76,12 @@ export const COMPONENTS = {
CaptionsButton: CaptionsButtonDef,
CaptionsRadioGroup: CaptionsRadioGroupDef,
CastButton: CastButtonDef,
Container: ContainerDef,
Controls: ControlsDef,
ErrorDialog: ErrorDialogDef,
FullscreenButton: FullscreenButtonDef,
Gesture: GestureDef,
Hotkey: HotkeyDef,
LiveButton: LiveButtonDef,
Menu: MenuDef,
MuteButton: MuteButtonDef,
@@ -0,0 +1,6 @@
import { defineComponent } from '../manifest';
import type { ContainerProps } from './container-core';
export default defineComponent<ContainerProps>()({
name: 'Container',
});
@@ -0,0 +1,2 @@
// biome-ignore lint/suspicious/noEmptyInterface: Container currently owns no source props.
export interface ContainerProps {}
@@ -0,0 +1,6 @@
import { defineComponent } from '../manifest';
import type { GestureProps } from './gesture-core';
export default defineComponent<GestureProps>()({
name: 'Gesture',
});
@@ -0,0 +1,16 @@
import type { StringWithSuggestions } from '@videojs/utils/types';
import type { InputAction } from '../input-action';
export type GesturePointerType = 'mouse' | 'touch' | 'pen';
export type GestureRegion = 'left' | 'center' | 'right';
export type GestureType = 'tap' | 'doubletap';
export interface GestureProps {
type: StringWithSuggestions<GestureType>;
action: InputAction;
value?: number | undefined;
pointer?: GesturePointerType | undefined;
region?: GestureRegion | undefined;
disabled?: boolean | undefined;
}
@@ -0,0 +1,6 @@
import { defineComponent } from '../manifest';
import type { HotkeyProps } from './hotkey-core';
export default defineComponent<HotkeyProps>()({
name: 'Hotkey',
});
@@ -0,0 +1,11 @@
import type { InputAction } from '../input-action';
export type HotkeyTarget = 'player' | 'global';
export interface HotkeyProps {
keys: string;
action: InputAction;
value?: number | undefined;
disabled?: boolean | undefined;
target?: HotkeyTarget | undefined;
}
+24
View File
@@ -0,0 +1,24 @@
import type { StringWithSuggestions } from '@videojs/utils/types';
export type InputActionSource = 'gesture' | 'hotkey';
export type InputAction = StringWithSuggestions<
| 'togglePaused'
| 'toggleMuted'
| 'toggleFullscreen'
| 'toggleSubtitles'
| 'togglePictureInPicture'
| 'toggleControls'
| 'seekStep'
| 'seekToPercent'
| 'volumeStep'
| 'speedUp'
| 'speedDown'
>;
export interface InputActionEvent {
action?: string | undefined;
value?: number | undefined;
source?: InputActionSource | undefined;
key?: string | undefined;
}
@@ -1,21 +1,9 @@
import { clamp } from '@videojs/utils/number';
import { formatTime } from '@videojs/utils/time';
export type InputActionSource = 'gesture' | 'hotkey';
import type { InputAction, InputActionEvent } from '../input-action';
export type InputAction =
| 'togglePaused'
| 'toggleMuted'
| 'toggleFullscreen'
| 'toggleSubtitles'
| 'togglePictureInPicture'
| 'toggleControls'
| 'seekStep'
| 'seekToPercent'
| 'volumeStep'
| 'speedUp'
| 'speedDown'
| (string & {});
export type { InputAction, InputActionEvent, InputActionSource } from '../input-action';
export type IndicatorDirection = 'forward' | 'backward';
export type IndicatorVolumeLevel = 'off' | 'low' | 'high';
@@ -33,13 +21,6 @@ export type IndicatorStatus =
| 'pip'
| 'exit-pip';
export interface InputActionEvent {
action?: string | undefined;
value?: number | undefined;
source?: InputActionSource | undefined;
key?: string | undefined;
}
export interface MediaSnapshot {
paused?: boolean | undefined;
volume?: number | undefined;
@@ -2,6 +2,9 @@
import { describe, it } from 'vitest';
import { createComponent, Slot } from '../../../jsx-runtime';
import type { ContainerProps } from '../container/container-core';
import type { GestureProps } from '../gesture/gesture-core';
import type { HotkeyProps } from '../hotkey/hotkey-core';
import { defineComponent } from '../manifest';
const PlayButton = createComponent(
@@ -27,14 +30,41 @@ const Time = createComponent(
})
);
const Container = createComponent(
defineComponent<ContainerProps>()({
name: 'Container',
})
);
const Hotkey = createComponent(
defineComponent<HotkeyProps>()({
name: 'Hotkey',
})
);
const Gesture = createComponent(
defineComponent<GestureProps>()({
name: 'Gesture',
})
);
describe('constrained JSX', () => {
it('accepts a single component', () => {
void (<PlayButton className="x" />);
void (<PlayButton key="play" />);
});
it('rejects invalid props on a single component', () => {
// @ts-expect-error - className must be a string
void (<PlayButton className={5} />);
// @ts-expect-error - id is a target-specific attr, not a core JSX prop
void (<PlayButton id="play" />);
// @ts-expect-error - hidden is a target-specific attr, not a core JSX prop
void (<PlayButton hidden />);
// @ts-expect-error - commandfor is HTML-specific wiring
void (<PlayButton commandfor="play-tooltip" />);
// @ts-expect-error - render is a React adapter prop, not a core JSX prop
void (<PlayButton render={<PlayButton />} />);
});
it('accepts compound parts inside their root', () => {
@@ -51,6 +81,27 @@ describe('constrained JSX', () => {
it('rejects invalid compound root props', () => {
// @ts-expect-error - `bogus` is not a valid orientation
void (<Slider.Root orientation="bogus" />);
// @ts-expect-error - boundary is target-specific positioning, not a core prop
void (<Slider.Root boundary="viewport" />);
});
it('accepts explicitly modeled input props', () => {
void (<Hotkey keys="k" action="togglePaused" />);
void (<Hotkey keys="f" action="toggleFullscreen" target="global" />);
void (<Gesture type="doubletap" action="seekStep" value={10} pointer="touch" region="right" />);
});
it('rejects invalid input props', () => {
// @ts-expect-error - global hotkeys use `global`, not DOM-specific `document`
void (<Hotkey keys="k" action="togglePaused" target="document" />);
// @ts-expect-error - invalid gesture region
void (<Gesture type="tap" action="togglePaused" region="outside" />);
});
it('keeps container props target-neutral', () => {
void (<Container className="skin" />);
// @ts-expect-error - focusability is target output behavior
void (<Container tabIndex={0} />);
});
it('rejects invalid Time.Value props', () => {
@@ -59,21 +110,20 @@ describe('constrained JSX', () => {
void (<Time.Value type="bogus" />);
});
it('accepts div and span as layout intrinsics', () => {
void (
<div className="row">
<span className="label">hello</span>
</div>
);
// @ts-expect-error - arbitrary HTML attributes (id) are not allowed on layout intrinsics
void (<div id="foo" />);
it('rejects platform-specific intrinsic elements', () => {
// @ts-expect-error - source JSX only exposes Video.js components
void (<div className="row" />);
// @ts-expect-error - source JSX only exposes Video.js components
void (<span className="label" />);
// @ts-expect-error - source JSX only exposes Video.js components
void (<button type="button" />);
});
it('accepts slot primitives', () => {
void (<Slot />);
void (
<Slot name="poster">
<span className="fallback" />
<PlayButton className="fallback" />
</Slot>
);
// @ts-expect-error - slot name must be a string
+4 -1
View File
@@ -1,4 +1,5 @@
import { isFunction } from '@videojs/utils/predicate';
import type { StringWithSuggestions } from '@videojs/utils/types';
import type { AnyPlayerStore } from '../media/types';
import { MEDIA_INPUT_ACTION_OVERRIDES } from '../media-actions';
@@ -33,7 +34,9 @@ const GESTURE_ACTION_OVERRIDES: Partial<Record<GestureActionName, GestureActionR
speedDown: MEDIA_INPUT_ACTION_OVERRIDES.speedDown,
};
export function resolveGestureAction(name: GestureActionName | (string & {})): GestureActionResolver | undefined {
export function resolveGestureAction(
name: StringWithSuggestions<GestureActionName>
): GestureActionResolver | undefined {
const override = GESTURE_ACTION_OVERRIDES[name as GestureActionName];
if (override) return override;
+2 -4
View File
@@ -1,8 +1,6 @@
export type GesturePointerType = 'mouse' | 'touch' | 'pen';
import type { GesturePointerType, GestureRegion, GestureType } from '../../core/ui/gesture/gesture-core';
export type GestureType = 'tap' | 'doubletap';
export type GestureRegion = 'left' | 'center' | 'right';
export type { GesturePointerType, GestureRegion, GestureType } from '../../core/ui/gesture/gesture-core';
export interface GestureOptions {
pointer?: GesturePointerType | undefined;
+4 -4
View File
@@ -56,7 +56,7 @@ export class HotkeyCoordinator {
this.#sortBindings();
// Lazily connect listeners.
if (options.target === 'document') {
if (options.target === 'global') {
this.#connectDocument();
} else {
this.#connect();
@@ -132,8 +132,8 @@ export class HotkeyCoordinator {
}
#maybeDisconnect(): void {
const hasPlayer = this.#bindings.some((b) => b.options.target !== 'document');
const hasDoc = this.#bindings.some((b) => b.options.target === 'document');
const hasPlayer = this.#bindings.some((b) => b.options.target !== 'global');
const hasDoc = this.#bindings.some((b) => b.options.target === 'global');
if (!hasPlayer) {
this.#disconnect?.abort();
@@ -164,7 +164,7 @@ export class HotkeyCoordinator {
if (event.repeat && options.repeatable === false) continue;
// Only consider bindings matching the event's target scope.
const isDocBinding = options.target === 'document';
const isDocBinding = options.target === 'global';
const isDocEvent = event.currentTarget === document;
if (isDocBinding !== isDocEvent) continue;
+3 -2
View File
@@ -1,5 +1,6 @@
import { isMacOS } from '@videojs/utils/dom';
import type { HotkeyTarget } from '../../core/ui/hotkey/hotkey-core';
import { HotkeyCoordinator } from './coordinator';
export type HotkeyModifierKey = 'shift' | 'ctrl' | 'alt' | 'meta';
@@ -15,8 +16,8 @@ export interface ParsedHotkeyBinding {
export interface HotkeyOptions {
keys: string;
onActivate: (event: KeyboardEvent, key: string) => void;
/** Where to listen — `'player'` (container) or `'document'`. */
target?: 'player' | 'document' | undefined;
/** Where to listen — `'player'` (container) or `'global'`. */
target?: HotkeyTarget | undefined;
/** Whether `event.repeat` should fire the callback. */
repeatable?: boolean | undefined;
disabled?: boolean | undefined;
@@ -291,21 +291,21 @@ describe('HotkeyCoordinator', () => {
});
});
describe('document target', () => {
it('listens on document for document-scoped bindings', () => {
describe('global target', () => {
it('listens on document for global bindings', () => {
const c = setup();
const onActivate = vi.fn();
c.add({ keys: 'k', onActivate, target: 'document' });
c.add({ keys: 'k', onActivate, target: 'global' });
keydown(document, 'k');
expect(onActivate).toHaveBeenCalledOnce();
});
it('cleans up document listener when last doc binding removed', () => {
it('cleans up document listener when last global binding removed', () => {
const c = setup();
const onActivate = vi.fn();
const remove = c.add({ keys: 'k', onActivate, target: 'document' });
const remove = c.add({ keys: 'k', onActivate, target: 'global' });
remove();
keydown(document, 'k');
@@ -313,10 +313,10 @@ describe('HotkeyCoordinator', () => {
expect(onActivate).not.toHaveBeenCalled();
});
it('fires document-scoped binding once when key originates in container', () => {
it('fires global binding once when key originates in container', () => {
const c = setup();
const onActivate = vi.fn();
c.add({ keys: 'k', onActivate, target: 'document' });
c.add({ keys: 'k', onActivate, target: 'global' });
// Key in container bubbles to document — doc listener fires once.
keydown(container, 'k');
+2 -1
View File
@@ -1,10 +1,11 @@
import { isString } from '@videojs/utils/predicate';
import type { StringWithSuggestions } from '@videojs/utils/types';
export function forceLayout(element: HTMLElement | null): void {
element?.getBoundingClientRect();
}
export type PositioningBoundary = 'viewport' | 'container' | (string & {}) | Element | null | undefined;
export type PositioningBoundary = StringWithSuggestions<'viewport' | 'container'> | Element | null | undefined;
export interface ResolvePositioningBoundaryOptions {
container?: Element | null;
+5 -6
View File
@@ -46,7 +46,7 @@ export type CreateComponentResult<M> = [InferParts<M>] extends [never]
? Component<InferProps<M>>
: CompoundComponent<M>;
function makePart<Props extends object>(name: string, part: string | null): Component<Props> {
function createComponentPart<Props extends object>(name: string, part: string | null): Component<Props> {
const fn = (_props: BaseProps & Props): ComponentNode => {
throw new Error(`@videojs/core: <${name}${part ? `.${part}` : ''}> can only be evaluated by the compiler.`);
};
@@ -56,7 +56,7 @@ function makePart<Props extends object>(name: string, part: string | null): Comp
return fn as Component<Props>;
}
export const Slot = makePart<SlotProps>('Slot', null);
export const Slot = createComponentPart<SlotProps>('Slot', null);
export function createComponent<
M extends ComponentManifest<object, readonly string[], Partial<Record<string, object>>>,
@@ -64,13 +64,13 @@ export function createComponent<
const parts = manifest.parts ?? [];
if (parts.length === 0) {
return makePart(manifest.name, null) as CreateComponentResult<M>;
return createComponentPart(manifest.name, null) as CreateComponentResult<M>;
}
const compound: Record<string, Component<never>> = {};
for (const part of parts) {
compound[part] = makePart(manifest.name, part);
compound[part] = createComponentPart(manifest.name, part);
}
return compound as CreateComponentResult<M>;
@@ -107,7 +107,6 @@ export namespace JSX {
}
export interface IntrinsicElements {
div: BaseProps;
span: BaseProps;
readonly [intrinsicElement: string]: never;
}
}