diff --git a/internal/decisions/constrained-jsx-boundaries.md b/internal/decisions/constrained-jsx-boundaries.md new file mode 100644 index 00000000..bf7cde2d --- /dev/null +++ b/internal/decisions/constrained-jsx-boundaries.md @@ -0,0 +1,35 @@ +--- +status: decided +date: 2026-06-20 +--- + +# Constrained JSX Boundaries + +## Decision + +Core owns the Video.js constrained JSX surface: component manifests, generated component symbols, `defineComponent`, `createComponent`, `Slot`, and the JSX runtime/dev-runtime. + +The compiler package remains generic. It may parse, match, transform, rewrite imports, and emit JSX, CSS, or diagnostics, but it must not contain Video.js component names, skin fixtures, runtime concepts, or platform assumptions. + +Source JSX in core is target-neutral. It exposes only Video.js components and explicitly modeled component props. Shared base props are limited to `className` and `children`. Lowercase HTML intrinsics and generic platform attributes such as `id`, `role`, `tabIndex`, `hidden`, `aria-*`, `data-*`, `commandfor`, and `render` are not part of the core JSX source surface. + +Target-specific lowering and adapter layers own platform output. HTML/React/native adapters can set ARIA, data attributes, focusability, command wiring, slots, and native element details as implementation output. + +## Context + +The skin port introduced pressure to share JSX across React, HTML, and future React Native targets. Allowing web-shaped source JSX in core would make the authoring surface look portable while quietly baking in DOM-only details. + +At the same time, component generation had drifted toward the compiler package. That coupled a generic transform tool to Video.js UI manifests and made compiler tests depend on skin-specific fixtures. + +## Alternatives Considered + +- **Keep component generation in compiler** - Rejected because it makes `@videojs/compiler` aware of Video.js UI semantics and fixtures. +- **Allow HTML intrinsics and global attrs in core JSX** - Rejected because it leaks DOM shape into code that should lower to multiple targets. +- **Introduce generic layout primitives now** - Rejected because no shared primitive API has been designed yet; source JSX should use real Video.js components until that need is proven. +- **Let source authors set ARIA/data/focus attrs directly** - Rejected because those attributes are target output and should be derived from component state, defaults, or adapter behavior. + +## Rationale + +This keeps boundaries simple and enforceable. Core defines the portable component contract. Compiler stays reusable and mechanically testable. Target packages remain free to emit platform-specific markup without turning those details into cross-target source API. + +The constrained JSX type surface also makes accidental DOM leakage fail at author time instead of during a later platform port. diff --git a/packages/compiler/src/tests/compile.test.ts b/packages/compiler/src/tests/compile.test.ts index 43b5d96f..572b2695 100644 --- a/packages/compiler/src/tests/compile.test.ts +++ b/packages/compiler/src/tests/compile.test.ts @@ -66,14 +66,14 @@ describe('compile (transformImports — bare-string rule)', () => { describe('compile (transformImports — function rule)', () => { it('rewrites per-identifier source and bucket-merges by resolved target', async () => { - const source = `import { Alpha, Beta } from '@fixture/components';\nconst _ = [Alpha, Beta];`; + const source = `import { Alpha, Beta } from '@fixture/widgets';\nconst _ = [Alpha, Beta];`; const { code } = await compileJsx(source, { imports: { - '@fixture/components': (name) => ({ source: `./ui/${name.toLowerCase()}`, name }), + '@fixture/widgets': (name) => ({ source: `./widgets/${name.toLowerCase()}`, name }), }, }); - expect(code).toContain(`import { Alpha } from "./ui/alpha"`); - expect(code).toContain(`import { Beta } from "./ui/beta"`); + expect(code).toContain(`import { Alpha } from "./widgets/alpha"`); + expect(code).toContain(`import { Beta } from "./widgets/beta"`); }); it('renames identifiers when the rule returns a different `name`', async () => { @@ -154,7 +154,7 @@ describe('childAsProp', () => { describe('replaceJsxChild', () => { it('replaces matched JSX children with expression helpers', async () => { - const source = `function App({ values }){ return ; }`; + const source = `function App({ values }){ return ; }`; const { code } = await compileJsx(source, { transforms: [ replaceJsxChild({ @@ -164,68 +164,64 @@ describe('replaceJsxChild', () => { ], }); - expect(collapse(code)).toContain(collapse(`{values["poster-image"]}`)); + expect(collapse(code)).toContain(collapse(`{values["poster-image"]}`)); }); }); describe('addProp', () => { it('emits a JSX value by default and adds the import', async () => { - const source = `function App(){ return ; }`; + const source = `function App(){ return ; }`; const { code } = await compileJsx(source, { - transforms: [ - addProp({ match: byTag('PlayButton'), prop: 'render', value: { source: './button', name: 'Button' } }), - ], + transforms: [addProp({ match: byTag('Action'), prop: 'render', value: { source: './button', name: 'Button' } })], }); - expect(collapse(code)).toContain(collapse(`}/>`)); + expect(collapse(code)).toContain(collapse(`}/>`)); expect(code).toContain(`import { Button } from "./button"`); }); it('emits a bare reference when kind is "ref"', async () => { - const source = `function App(){ return ; }`; + const source = `function App(){ return ; }`; const { code } = await compileJsx(source, { transforms: [ addProp({ - match: byTag('PlayButton'), + match: byTag('Action'), prop: 'as', value: { source: './button', name: 'Button', kind: 'ref' }, }), ], }); - expect(collapse(code)).toContain(collapse(``)); + expect(collapse(code)).toContain(collapse(``)); }); it('skips elements where the prop is already set', async () => { - const source = `function App(){ return }/>; }`; + const source = `function App(){ return }/>; }`; const { code } = await compileJsx(source, { - transforms: [ - addProp({ match: byTag('PlayButton'), prop: 'render', value: { source: './button', name: 'Button' } }), - ], + transforms: [addProp({ match: byTag('Action'), prop: 'render', value: { source: './button', name: 'Button' } })], }); expect(collapse(code)).toContain(collapse(``)); expect(code).not.toContain('import { Button }'); }); it('overwrites the existing prop when overwrite is true', async () => { - const source = `function App(){ return }/>; }`; + const source = `function App(){ return }/>; }`; const { code } = await compileJsx(source, { transforms: [ addProp({ - match: byTag('PlayButton'), + match: byTag('Action'), prop: 'render', overwrite: true, value: { source: './button', name: 'Button' }, }), ], }); - expect(collapse(code)).toContain(collapse(`}/>`)); + expect(collapse(code)).toContain(collapse(`}/>`)); }); }); describe('matchers', () => { it('byTag supports dotted tags', async () => { - const source = `function App(){ return ; }`; + const source = `function App(){ return ; }`; const { code } = await compileJsx(source, { - transforms: [replace({ match: byTag('Popover.Root'), with: { source: 'pkg', name: 'NewRoot' } })], + transforms: [replace({ match: byTag('Alpha.Root'), with: { source: 'pkg', name: 'NewRoot' } })], }); expect(code).toContain(` - - - - - - - - - - - - - Something went wrong. - - - - OK - - - - - - - - - - - - - - - - - - - - - - - - - {SEEK_TIME} - - - - Seek backward {SEEK_TIME} seconds - - - - - - - - {SEEK_TIME} - - - - Seek forward {SEEK_TIME} seconds - - - - - - - - - - - - - - - - - - - - - - - - - - - Toggle playback rate - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - {/* Hotkeys */} - - - - - - - - - - - - - - - - - - - {/* Gestures */} - - - - - - - ); -} diff --git a/packages/compiler/src/tests/integration.test.ts b/packages/compiler/src/tests/integration.test.ts index 29f1c843..b1678921 100644 --- a/packages/compiler/src/tests/integration.test.ts +++ b/packages/compiler/src/tests/integration.test.ts @@ -1,33 +1,46 @@ -import { readFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; import { beforeAll, describe, expect, it } from 'vitest'; import { compile } from '..'; import { anyTag, byTag, childAsProp, hasChild, jsx, replace } from '../jsx'; import type { ImportRule } from '../transforms'; -const __dirname = dirname(fileURLToPath(import.meta.url)); -const skinSource = resolve(__dirname, 'fixtures/video-skin.tsx'); - /** - * End-to-end smoke test: feed a representative constrained-JSX video skin - * (vendored under `fixtures/`) through `compile()` with the same shape - * a package build hook uses, and sanity-check the output's structural - * shape. Snapshot-style assertions intentionally use `.toContain` over a full - * snapshot to keep the test resilient to incidental whitespace differences - * from the TS printer. + * End-to-end smoke test for the generic JSX transform pipeline. The compiler + * package deliberately avoids Video.js UI semantics here: transforms operate on + * JSX tags and imports only. */ -describe('integration: default/video skin → JSX', () => { - const source = readFileSync(skinSource, 'utf8'); +describe('integration: JSX transform pipeline', () => { + const source = ` +import { Alpha, Beta, Gamma } from '@fixture/widgets'; +import { Icon } from '@fixture/icons/components'; +import { tokens } from '../tokens'; + +export function Example() { + return ( + + + + + + + + + + + + + + ); +} +`; let code = ''; const imports: Record = { - '@fixture/components': (name) => ({ - source: `./src/ui/${name.replace(/^[A-Z]/, (m) => m.toLowerCase()).replace(/[A-Z]/g, (m) => `-${m.toLowerCase()}`)}`, + '@fixture/widgets': (name) => ({ + source: `./widgets/${name.replace(/^[A-Z]/, (m) => m.toLowerCase()).replace(/[A-Z]/g, (m) => `-${m.toLowerCase()}`)}`, name, }), '@fixture/icons/components': '@fixture/icons/jsx', - '../tailwind': '@videojs/skins/default/tailwind', + '../tokens': '@fixture/tokens', }; beforeAll(async () => { @@ -37,13 +50,13 @@ describe('integration: default/video skin → JSX', () => { imports, transforms: [ replace({ - match: byTag('Popover.Root', { - when: hasChild(byTag('Popover.Trigger', { when: hasChild(byTag('MuteButton')) })), + match: byTag('Gamma.Root', { + when: hasChild(byTag('Gamma.Trigger', { when: hasChild(byTag('Beta')) })), }), - with: { source: './volume-popover', name: 'VolumePopover' }, + with: { source: './replacement', name: 'Replacement' }, mapChildren: () => [], }), - childAsProp({ match: anyTag(['Tooltip.Trigger', 'Popover.Trigger']), prop: 'render' }), + childAsProp({ match: anyTag(['Alpha.Trigger', 'Gamma.Trigger']), prop: 'render' }), ], }), }, @@ -51,12 +64,11 @@ describe('integration: default/video skin → JSX', () => { code = result.code; }); - it('rewrites component imports to per-identifier UI sources', () => { - expect(code).toMatch(/import \{ PlayButton \} from "\.\/src\/ui\/play-button"/); - // MuteButton lives under the volume Popover.Root subtree, which is replaced - // wholesale by VolumePopover — its import is correctly dropped by the - // unused-imports cleanup pass. - expect(code).not.toMatch(/import \{ MuteButton \}/); + it('rewrites component imports to per-identifier sources', () => { + expect(code).toMatch(/import \{ Alpha \} from "\.\/widgets\/alpha"/); + expect(code).toMatch(/import \{ Beta \} from "\.\/widgets\/beta"/); + // Gamma only appears under the replaced subtree, so cleanup drops it. + expect(code).not.toMatch(/import \{ Gamma \}/); }); it('rewrites icon component imports', () => { @@ -64,16 +76,16 @@ describe('integration: default/video skin → JSX', () => { expect(code).not.toContain('@fixture/icons/components'); }); - it('substitutes the volume Popover.Root with VolumePopover', () => { - expect(code).toContain(' { + expect(code).toContain(' { - expect(code).toMatch(/ { + expect(code).toMatch(/ { - expect(code).toContain('@videojs/skins/default/tailwind'); + it('keeps unrelated relative token imports re-routed by config', () => { + expect(code).toContain('@fixture/tokens'); }); }); diff --git a/packages/core/src/core/index.ts b/packages/core/src/core/index.ts index 274d9597..e64a8232 100644 --- a/packages/core/src/core/index.ts +++ b/packages/core/src/core/index.ts @@ -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'; diff --git a/packages/core/src/core/ui/components.generated.ts b/packages/core/src/core/ui/components.generated.ts index d58b2e2f..dabde488 100644 --- a/packages/core/src/core/ui/components.generated.ts +++ b/packages/core/src/core/ui/components.generated.ts @@ -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, diff --git a/packages/core/src/core/ui/container/container-component.ts b/packages/core/src/core/ui/container/container-component.ts new file mode 100644 index 00000000..03ef76b1 --- /dev/null +++ b/packages/core/src/core/ui/container/container-component.ts @@ -0,0 +1,6 @@ +import { defineComponent } from '../manifest'; +import type { ContainerProps } from './container-core'; + +export default defineComponent()({ + name: 'Container', +}); diff --git a/packages/core/src/core/ui/container/container-core.ts b/packages/core/src/core/ui/container/container-core.ts new file mode 100644 index 00000000..ce2b26c0 --- /dev/null +++ b/packages/core/src/core/ui/container/container-core.ts @@ -0,0 +1,2 @@ +// biome-ignore lint/suspicious/noEmptyInterface: Container currently owns no source props. +export interface ContainerProps {} diff --git a/packages/core/src/core/ui/gesture/gesture-component.ts b/packages/core/src/core/ui/gesture/gesture-component.ts new file mode 100644 index 00000000..e0b46930 --- /dev/null +++ b/packages/core/src/core/ui/gesture/gesture-component.ts @@ -0,0 +1,6 @@ +import { defineComponent } from '../manifest'; +import type { GestureProps } from './gesture-core'; + +export default defineComponent()({ + name: 'Gesture', +}); diff --git a/packages/core/src/core/ui/gesture/gesture-core.ts b/packages/core/src/core/ui/gesture/gesture-core.ts new file mode 100644 index 00000000..017ec288 --- /dev/null +++ b/packages/core/src/core/ui/gesture/gesture-core.ts @@ -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; + action: InputAction; + value?: number | undefined; + pointer?: GesturePointerType | undefined; + region?: GestureRegion | undefined; + disabled?: boolean | undefined; +} diff --git a/packages/core/src/core/ui/hotkey/hotkey-component.ts b/packages/core/src/core/ui/hotkey/hotkey-component.ts new file mode 100644 index 00000000..3a79b60a --- /dev/null +++ b/packages/core/src/core/ui/hotkey/hotkey-component.ts @@ -0,0 +1,6 @@ +import { defineComponent } from '../manifest'; +import type { HotkeyProps } from './hotkey-core'; + +export default defineComponent()({ + name: 'Hotkey', +}); diff --git a/packages/core/src/core/ui/hotkey/hotkey-core.ts b/packages/core/src/core/ui/hotkey/hotkey-core.ts new file mode 100644 index 00000000..bda52621 --- /dev/null +++ b/packages/core/src/core/ui/hotkey/hotkey-core.ts @@ -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; +} diff --git a/packages/core/src/core/ui/input-action.ts b/packages/core/src/core/ui/input-action.ts new file mode 100644 index 00000000..fb3a02e1 --- /dev/null +++ b/packages/core/src/core/ui/input-action.ts @@ -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; +} diff --git a/packages/core/src/core/ui/input-feedback/status.ts b/packages/core/src/core/ui/input-feedback/status.ts index aa6d346d..333fe01c 100644 --- a/packages/core/src/core/ui/input-feedback/status.ts +++ b/packages/core/src/core/ui/input-feedback/status.ts @@ -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; diff --git a/packages/core/src/core/ui/tests/jsx-runtime.test-d.tsx b/packages/core/src/core/ui/tests/jsx-runtime.test-d.tsx index e74c5fc3..942c23f1 100644 --- a/packages/core/src/core/ui/tests/jsx-runtime.test-d.tsx +++ b/packages/core/src/core/ui/tests/jsx-runtime.test-d.tsx @@ -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()({ + name: 'Container', + }) +); + +const Hotkey = createComponent( + defineComponent()({ + name: 'Hotkey', + }) +); + +const Gesture = createComponent( + defineComponent()({ + name: 'Gesture', + }) +); + describe('constrained JSX', () => { it('accepts a single component', () => { void (); + void (); }); it('rejects invalid props on a single component', () => { // @ts-expect-error - className must be a string void (); + // @ts-expect-error - id is a target-specific attr, not a core JSX prop + void (); + // @ts-expect-error - hidden is a target-specific attr, not a core JSX prop + void (); + // @ts-expect-error - commandfor is HTML-specific wiring + void (); + // @ts-expect-error - render is a React adapter prop, not a core JSX prop + void (} />); }); 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 (); + // @ts-expect-error - boundary is target-specific positioning, not a core prop + void (); + }); + + it('accepts explicitly modeled input props', () => { + void (); + void (); + void (); + }); + + it('rejects invalid input props', () => { + // @ts-expect-error - global hotkeys use `global`, not DOM-specific `document` + void (); + // @ts-expect-error - invalid gesture region + void (); + }); + + it('keeps container props target-neutral', () => { + void (); + // @ts-expect-error - focusability is target output behavior + void (); }); it('rejects invalid Time.Value props', () => { @@ -59,21 +110,20 @@ describe('constrained JSX', () => { void (); }); - it('accepts div and span as layout intrinsics', () => { - void ( - - hello - - ); - // @ts-expect-error - arbitrary HTML attributes (id) are not allowed on layout intrinsics - void (); + it('rejects platform-specific intrinsic elements', () => { + // @ts-expect-error - source JSX only exposes Video.js components + void (); + // @ts-expect-error - source JSX only exposes Video.js components + void (); + // @ts-expect-error - source JSX only exposes Video.js components + void (); }); it('accepts slot primitives', () => { void (); void ( - + ); // @ts-expect-error - slot name must be a string diff --git a/packages/core/src/dom/gesture/actions.ts b/packages/core/src/dom/gesture/actions.ts index 937d2410..bd1fdd52 100644 --- a/packages/core/src/dom/gesture/actions.ts +++ b/packages/core/src/dom/gesture/actions.ts @@ -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 +): GestureActionResolver | undefined { const override = GESTURE_ACTION_OVERRIDES[name as GestureActionName]; if (override) return override; diff --git a/packages/core/src/dom/gesture/gesture.ts b/packages/core/src/dom/gesture/gesture.ts index 1edc7862..49223343 100644 --- a/packages/core/src/dom/gesture/gesture.ts +++ b/packages/core/src/dom/gesture/gesture.ts @@ -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; diff --git a/packages/core/src/dom/hotkey/coordinator.ts b/packages/core/src/dom/hotkey/coordinator.ts index d1465742..43b07acd 100644 --- a/packages/core/src/dom/hotkey/coordinator.ts +++ b/packages/core/src/dom/hotkey/coordinator.ts @@ -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; diff --git a/packages/core/src/dom/hotkey/hotkey.ts b/packages/core/src/dom/hotkey/hotkey.ts index 9a5d7570..9afaefd3 100644 --- a/packages/core/src/dom/hotkey/hotkey.ts +++ b/packages/core/src/dom/hotkey/hotkey.ts @@ -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; diff --git a/packages/core/src/dom/hotkey/tests/coordinator.test.ts b/packages/core/src/dom/hotkey/tests/coordinator.test.ts index 9485638d..61a15e31 100644 --- a/packages/core/src/dom/hotkey/tests/coordinator.test.ts +++ b/packages/core/src/dom/hotkey/tests/coordinator.test.ts @@ -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'); diff --git a/packages/core/src/dom/utils/layout.ts b/packages/core/src/dom/utils/layout.ts index 00dd22d1..e715b506 100644 --- a/packages/core/src/dom/utils/layout.ts +++ b/packages/core/src/dom/utils/layout.ts @@ -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; diff --git a/packages/core/src/jsx-runtime.ts b/packages/core/src/jsx-runtime.ts index c168a7ad..305930d5 100644 --- a/packages/core/src/jsx-runtime.ts +++ b/packages/core/src/jsx-runtime.ts @@ -46,7 +46,7 @@ export type CreateComponentResult = [InferParts] extends [never] ? Component> : CompoundComponent; -function makePart(name: string, part: string | null): Component { +function createComponentPart(name: string, part: string | null): Component { 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(name: string, part: string | null): Comp return fn as Component; } -export const Slot = makePart('Slot', null); +export const Slot = createComponentPart('Slot', null); export function createComponent< M extends ComponentManifest>>, @@ -64,13 +64,13 @@ export function createComponent< const parts = manifest.parts ?? []; if (parts.length === 0) { - return makePart(manifest.name, null) as CreateComponentResult; + return createComponentPart(manifest.name, null) as CreateComponentResult; } const compound: Record> = {}; for (const part of parts) { - compound[part] = makePart(manifest.name, part); + compound[part] = createComponentPart(manifest.name, part); } return compound as CreateComponentResult; @@ -107,7 +107,6 @@ export namespace JSX { } export interface IntrinsicElements { - div: BaseProps; - span: BaseProps; + readonly [intrinsicElement: string]: never; } } diff --git a/packages/html/src/ui/gesture/gesture-element.ts b/packages/html/src/ui/gesture/gesture-element.ts index 36a7cf79..e1faa8c1 100644 --- a/packages/html/src/ui/gesture/gesture-element.ts +++ b/packages/html/src/ui/gesture/gesture-element.ts @@ -1,11 +1,5 @@ -import { - createDoubleTapGesture, - createTapGesture, - type GestureActionName, - type GesturePointerType, - type GestureRegion, - resolveGestureAction, -} from '@videojs/core/dom'; +import type { GestureProps } from '@videojs/core'; +import { createDoubleTapGesture, createTapGesture, resolveGestureAction } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextConsumer } from '@videojs/element/context'; @@ -25,11 +19,11 @@ export class GestureElement extends MediaElement { disabled: { type: Boolean }, }; - type: 'tap' | 'doubletap' | (string & {}) = ''; - action: GestureActionName | (string & {}) = ''; - value: number | undefined = undefined; - pointer: GesturePointerType | undefined = undefined; - region: GestureRegion | undefined = undefined; + type: GestureProps['type'] = ''; + action: GestureProps['action'] = ''; + value: GestureProps['value'] = undefined; + pointer: GestureProps['pointer'] = undefined; + region: GestureProps['region'] = undefined; disabled = false; readonly #player = new PlayerController(this, playerContext); diff --git a/packages/html/src/ui/hotkey/hotkey-element.ts b/packages/html/src/ui/hotkey/hotkey-element.ts index faf203de..962d7991 100644 --- a/packages/html/src/ui/hotkey/hotkey-element.ts +++ b/packages/html/src/ui/hotkey/hotkey-element.ts @@ -1,4 +1,5 @@ -import { createHotkey, type HotkeyActionName, isHotkeyToggleAction, resolveHotkeyAction } from '@videojs/core/dom'; +import type { HotkeyProps } from '@videojs/core'; +import { createHotkey, isHotkeyToggleAction, resolveHotkeyAction } from '@videojs/core/dom'; import type { PropertyDeclarationMap, PropertyValues } from '@videojs/element'; import { ContextConsumer } from '@videojs/element/context'; @@ -17,11 +18,11 @@ export class HotkeyElement extends MediaElement { target: { type: String }, }; - keys = ''; - action: HotkeyActionName | (string & {}) = ''; - value: number | undefined = undefined; + keys: HotkeyProps['keys'] = ''; + action: HotkeyProps['action'] = ''; + value: HotkeyProps['value'] = undefined; disabled = false; - target: 'player' | 'document' = 'player'; + target: NonNullable = 'player'; readonly #player = new PlayerController(this, playerContext); readonly #container = new ContextConsumer(this, { diff --git a/packages/react/src/player/context.tsx b/packages/react/src/player/context.tsx index c72d005f..a4246700 100644 --- a/packages/react/src/player/context.tsx +++ b/packages/react/src/player/context.tsx @@ -1,6 +1,6 @@ 'use client'; -import type { Media } from '@videojs/core'; +import type { ContainerProps as CoreContainerProps, Media } from '@videojs/core'; import type { MediaContainer, PopupGroup } from '@videojs/core/dom'; import type { UnknownState, UnknownStore } from '@videojs/store'; import { useStore } from '@videojs/store/react'; @@ -113,7 +113,7 @@ export function useContainerAttach(): Dispatch { +export interface ContainerProps extends HTMLAttributes, CoreContainerProps { children?: ReactNode; } diff --git a/packages/react/src/ui/gesture/gesture.tsx b/packages/react/src/ui/gesture/gesture.tsx index e68344b2..2536d0d6 100644 --- a/packages/react/src/ui/gesture/gesture.tsx +++ b/packages/react/src/ui/gesture/gesture.tsx @@ -1,28 +1,12 @@ 'use client'; -import { - type AnyPlayerStore, - createDoubleTapGesture, - createTapGesture, - type GestureActionName, - type GesturePointerType, - type GestureRegion, - resolveGestureAction, -} from '@videojs/core/dom'; +import type { GestureProps } from '@videojs/core'; +import { type AnyPlayerStore, createDoubleTapGesture, createTapGesture, resolveGestureAction } from '@videojs/core/dom'; import type { ReactNode } from 'react'; import { useEffect } from 'react'; import { useContainer, usePlayer } from '../../player/context'; -export interface GestureProps { - type: 'tap' | 'doubletap' | (string & {}); - action: GestureActionName | (string & {}); - value?: number; - pointer?: GesturePointerType; - region?: GestureRegion; - disabled?: boolean; -} - export function Gesture({ type, action, value, pointer, region, disabled }: GestureProps): ReactNode { const store = usePlayer() as AnyPlayerStore; const container = useContainer(); @@ -53,6 +37,8 @@ export namespace Gesture { export type Props = GestureProps; } +export type { GestureProps }; + /** @deprecated Use `GestureProps` instead. */ export type MediaGestureProps = GestureProps; diff --git a/packages/react/src/ui/hotkey/hotkey.tsx b/packages/react/src/ui/hotkey/hotkey.tsx index 5107bc3f..a607c8a9 100644 --- a/packages/react/src/ui/hotkey/hotkey.tsx +++ b/packages/react/src/ui/hotkey/hotkey.tsx @@ -1,25 +1,12 @@ 'use client'; -import { - type AnyPlayerStore, - createHotkey, - type HotkeyActionName, - isHotkeyToggleAction, - resolveHotkeyAction, -} from '@videojs/core/dom'; +import type { HotkeyProps } from '@videojs/core'; +import { type AnyPlayerStore, createHotkey, isHotkeyToggleAction, resolveHotkeyAction } from '@videojs/core/dom'; import type { ReactNode } from 'react'; import { useEffect } from 'react'; import { useContainer, usePlayer } from '../../player/context'; -export interface HotkeyProps { - keys: string; - action: HotkeyActionName | (string & {}); - value?: number; - disabled?: boolean; - target?: 'player' | 'document'; -} - export function Hotkey({ keys, action, value, disabled, target }: HotkeyProps): ReactNode { const store = usePlayer() as AnyPlayerStore; const container = useContainer(); @@ -50,6 +37,8 @@ export namespace Hotkey { export type Props = HotkeyProps; } +export type { HotkeyProps }; + /** @deprecated Use `HotkeyProps` instead. */ export type MediaHotkeyProps = HotkeyProps; diff --git a/packages/react/src/ui/hotkey/use-hotkey.ts b/packages/react/src/ui/hotkey/use-hotkey.ts index 09138c28..3ad1c250 100644 --- a/packages/react/src/ui/hotkey/use-hotkey.ts +++ b/packages/react/src/ui/hotkey/use-hotkey.ts @@ -1,5 +1,6 @@ 'use client'; +import type { HotkeyTarget } from '@videojs/core'; import { createHotkey } from '@videojs/core/dom'; import { useEffect } from 'react'; @@ -9,7 +10,7 @@ import { useLatestRef } from '../../utils/use-latest-ref'; export interface UseHotkeyOptions { keys: string; onActivate: (event: KeyboardEvent, key: string) => void; - target?: 'player' | 'document'; + target?: HotkeyTarget; repeatable?: boolean; disabled?: boolean; } diff --git a/packages/utils/src/dom/types.ts b/packages/utils/src/dom/types.ts index e3bf9376..593a2d36 100644 --- a/packages/utils/src/dom/types.ts +++ b/packages/utils/src/dom/types.ts @@ -1,3 +1,5 @@ +import type { StringWithSuggestions } from '../types'; + // Method syntax is required here for TypeScript's class inheritance checking. // Using property syntax (e.g., `connectedCallback?: () => void`) causes TS2425 // when a class extends a generic mixin that defines lifecycle callbacks. @@ -18,7 +20,7 @@ export type QueriedElement = S extends keyo ? HTMLElementTagNameMap[S] : E; -export type EventType = (keyof Events & string) | (string & {}); +export type EventType = StringWithSuggestions; export type EventListenerFor = | ((event: K extends keyof Events ? Events[K] : Event) => void) diff --git a/packages/utils/src/types/tests/types.test.ts b/packages/utils/src/types/tests/types.test.ts new file mode 100644 index 00000000..ad76823f --- /dev/null +++ b/packages/utils/src/types/tests/types.test.ts @@ -0,0 +1,12 @@ +import { describe, expectTypeOf, it } from 'vitest'; +import type { StringWithSuggestions } from '../types'; + +describe('StringWithSuggestions', () => { + it('accepts arbitrary strings without widening literal unions', () => { + type Action = StringWithSuggestions<'play' | 'pause'>; + + expectTypeOf().toMatchTypeOf(); + expectTypeOf().toMatchTypeOf(); + expectTypeOf>().toEqualTypeOf<'play'>(); + }); +}); diff --git a/packages/utils/src/types/types.ts b/packages/utils/src/types/types.ts index 0ada39d4..4cbbfdff 100644 --- a/packages/utils/src/types/types.ts +++ b/packages/utils/src/types/types.ts @@ -15,6 +15,8 @@ export type MixinReturn, Props> = Constructor = T | false | null | undefined; +export type StringWithSuggestions = Value | (string & {}); + export type EnsureFunction = T extends (...args: any[]) => any ? T : never; export type Simplify = { [KeyType in keyof T]: T[KeyType] } & {};