docs: JSDoc every public-API export in react, html, and core#1570
Draft
decepulis wants to merge 14 commits into
Draft
docs: JSDoc every public-API export in react, html, and core#1570decepulis wants to merge 14 commits into
decepulis wants to merge 14 commits into
Conversation
Establishes a stronger rule for public-API exports: every symbol reachable via a package.json "exports" entry carries a JSDoc summary, including class members and prop fields that the leaf adds. Inherited fields keep their JSDoc at the base. Monolithic surfaces (skins, createPlayer) get richer prose plus a single @see link to an existing docs page — no @example blocks. Scopes the existing "no JSDoc for self-documenting code" rule to internal code so it no longer contradicts the published-export requirement. Absorbs the prior "API reference exports are different" subsection into the new rule. https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98
Apply the JSDoc rule from CLAUDE.md to every symbol reachable via packages/html/package.json "exports": custom-element classes, mixins, controllers, context modules, player factory, skin elements, and the *PlayerElement shells. Skin element classes (incl. Tailwind variants) get richer prose + a single @see link to /docs/framework/html/concepts/skins. createPlayer factory overloads link to /docs/framework/html/reference/html-create-player. Shells (VideoPlayerElement etc.) get a one-line summary per Ring 1. Public class members (tagName, methods, properties) carry their own summaries. Inherited fields keep their JSDoc on the base. No @example blocks: replaced the dev-only @example on createPlayer with the @see link convention. No TODO placeholders. Refs #1558. https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98
Apply the JSDoc rule from CLAUDE.md to every symbol reachable via packages/react/package.json "exports": player factory, context module, hooks, UI primitives, skins, media adapters, and util helpers. Skin variants (incl. Tailwind siblings) get richer prose + a single @see link to /docs/framework/react/concepts/skins. createPlayer overloads link to /docs/framework/react/reference/create-player. Leaf Props interfaces that add no new fields get a one-line summary; leaves that add fields document only the added ones. Inherited fields stay at the base. Replaces stale @example blocks (which drift with the source) with the @see link convention. Fixes a malformed JSDoc on useOptionalPlayer where the standalone summary above the overloads wasn't attached to a symbol. No TODO placeholders. Refs #1558. https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98
Apply the JSDoc rule from CLAUDE.md to every symbol reachable via packages/core/package.json "exports": *Core classes and their members, *Props / *State / *Input / *Capability interfaces, feature factories, selectors, media element classes (HlsMedia, DashMedia, NativeHlsMedia, SimpleHlsMedia, MuxVideoMedia, MuxAudioMedia, GoogleCastMixin), gesture + hotkey coordinators, DOM utilities, and the dom/ui factory functions (createMenu, createPopover, createSlider, createThumbnail, createTransition, createTooltip, createPopupGroup, etc.). *Core classes are the highest-leverage targets in this sweep — the control-bar primitives that AI agents read most aggressively per the bench data in #1558. Each class gets a class-level summary plus a summary on every public method, property, and the constructor. Foundational *Props and *State interfaces in core are the bases that react/html leaves extend, so per-field JSDoc here propagates upward via TypeScript inheritance. Refs #1558. https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (8)
Players (5)
Skins (30)
UI Components (35)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (7)
Skins (27)
UI Components (29)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (9)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
Voice A is the standard for new JSDoc on previously-undocumented exports — not a license to delete or shrink existing prose, @example blocks, or @returns descriptions that an author wrote thoughtfully. Adds a "preserve existing authored content" caveat under the "JSDoc on Published Exports" rule so future sweeps don't over-edit. Refs #1558, #1570. https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98
The initial sweep was too eager. It collapsed multi-paragraph descriptions, removed @example blocks, and reworded prose that the original author had written deliberately. Restore that content while keeping the sweep's new summaries on previously-undocumented exports and the new @see links on Ring 3 monolithic surfaces. Restored content in 22 files across @videojs/core, @videojs/html, and @videojs/react: - @example blocks: parseHotkeyPattern + createHotkey (core), createPlayer + PlayerController (html), PlayButton, PiPButton, PlaybackRateButton, Poster, SeekButton, Time.Group/Separator/Value, BufferingIndicator, useButton, LiveButton, mergeProps, composeRefs, useComposedRefs, renderElement (react). - Multi-paragraph descriptions on LiveButton (composition + i18n) and the live-video / live-audio skin presets (flexible spacer). - The "useful for components that can operate without player context" prose on useOptionalPlayer (kept the structural overload-attachment fix from the original sweep). - The concrete SliderTrackElement subclass example on ContextPartElement (showed how to extend the abstract base). Refs #1558, #1570. https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98
…om JSDoc
Carves out two cases where JSDoc adds noise without serving the rule's
priorities (props in API references, AI-useful info on props and imports):
- Leaf wrapper types with empty bodies (`interface Foo extends Bar {}`,
`type Foo = Bar`) skip the summary; the base's JSDoc covers it.
- Uniform plumbing methods on `*Core` classes (`setProps`, `getState`,
`getAttrs`, `setMedia`) skip JSDoc; the names are predictable across
every core. Behavior methods still get JSDoc.
The sweep's intent was to add JSDoc to undocumented exports, not to rewrite existing authored content. In 32 files it reworded prose that was already on main — sometimes losing information (mergeProps had its event-handler ordering flipped from correct to wrong). Restores the original wording verbatim across react, html, and core. Keeps the one structural fix in react/src/player/context.tsx where the sweep correctly merged an orphaned prose block into the @Label JSDoc that was hiding it from useOptionalPlayer's first overload.
Leaf wrappers like `interface PlayButtonProps extends UIComponentProps,
PlayButtonCore.Props {}` and `type LiveAudioSkinProps = BaseSkinProps`
add nothing the base hasn't already said. The summary just restates
"Props for the X component" — noise on top of the inherited JSDoc.
Per CLAUDE.md update, leaf wrappers with empty bodies skip the summary.
Wrappers that add fields still get a summary.
setProps/getState/getAttrs/setMedia follow a predictable pattern across every *Core class — the method name carries the meaning and the JSDoc just restates it. Behavior methods (toggle, cycle, seek, etc.) keep their JSDoc because they encode domain semantics. Per CLAUDE.md update. Files where a CRUD method has a non-uniform description (e.g. AlertDialogCore.setProps notes the platform-layer contract) keep their JSDoc as a carve-out.
Accuracy sweep against actual implementations. Corrections so far: - StatusIndicator.value no longer claims playback-rate as an example (status indicators output volume percent, not playback rate). - TooltipProvider no longer claims "only one opens at a time" — it shares hover/focus timing so the next tooltip opens instantly while the group is warm. - Small fixes across seek-indicator / volume-slider / time / slider preview elements where prose drifted from implementation.
CaptionsButtonElement toggles subtitle/caption visibility on the media via the text-track feature's toggleSubtitles, which flips all subtitle and caption tracks — not a single "default" track.
- seek-indicator-core: currentTime is updated on every event in the burst, not anchored to the start. Replaced "at the start of the burst" with "at the latest seek event". - seek-indicator-root (react): the indicator stays mounted through closeDelay (~800ms) plus the close transition, not only while a seek is in flight. Rewrote to "mounts when a seek burst starts and unmounts after the close transition". - popover/menu/tooltip element boundary: the field accepts viewport and container keywords, a selector, or an Element. Replaced "Element the X is constrained to" with "Region positioning is clamped within". - provider-mixin factory: dropped the "once" promise. The factory is re-entered if the store is accessed after destroyCallback, so the guarantee was inaccurate. - error-dialog-core: the class itself only overrides setProps to a no-op; the "driven by media error state" wiring lives in the consumer elements. Rewrote to reflect both — "ignores props" plus "open state is driven externally by media error state".
4 tasks
Moves all skin JSDoc additions to #1574 — a smaller, focused PR that ships the eject-discoverability fix from #1558 with low review burden. This PR keeps the broader sweep (every public-API export across react / html / core, plus the CLAUDE.md rule that justifies it) for team discussion. Reverts: - All 16 React skin presets (audio, video, background, live-audio, live-video; including Tailwind siblings). - All 16 HTML skin defines (matching set). - Background plumbing: define/background/{skin,player}.ts, define/media/background-video.ts, react/media/background-video/index.tsx.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1558.
Why
Per the bench data in #1558, AI coding agents Read 15–27 distinct
.d.tsfiles per Video.js install —node_modules/@videojs/{react,html,core}/dist/dev/**/*.d.ts. The file is already in the agent's context window when it Reads it. JSDoc on the published.d.tsships documentation alongside the symbol — zero extra round trips. The same comment feeds three downstream channels:This PR fills the gap and codifies the rule so future exports don't slip back.
What changed
1.
CLAUDE.md— "JSDoc on Published Exports" rule (docs(claude)commit). Every public-API export carries JSDoc that survives into the shipped.d.ts. Voice A: one-sentence, declarative, active, present tense, says what the type doesn't. Inherited fields stay at the base; leaves document only the fields they add. Public class members (non-#, non-@internal) get their own summaries. Monolithic surfaces (skins +createPlayer) get richer prose + a single@seelink.Two carve-outs added after review:
interface FooProps extends BarProps {}).*Coreclasses (setProps,getState,getAttrs,setMedia). Behavior methods still get JSDoc.2.
@videojs/reactsweep (~80 files post-split). UI primitives, hooks, player factory + context, media adapters, util helpers.createPlayeroverloads get@see reference/create-player. Skin files now ship via #1574.3.
@videojs/htmlsweep (~90 files post-split). Custom-element classes, mixins, controllers, context modules, player factory,*PlayerElementshells.createPlayeroverloads get@see reference/html-create-player. Skin defines now ship via #1574.4.
@videojs/coresweep (~126 files). Every*Coreclass (minus uniform CRUD) + members + constructor; every*Props/*State/*Input/*Capabilityinterface with per-field JSDoc; feature factories; selectors; media element classes (HLS, Dash, Native, Mux, Cast, etc.); gesture + hotkey coordinators; DOM-side factories (createMenu,createPopover,createSlider, etc.).5. Accuracy audit (
docs(packages)commit + 6 audit fixes). Every behavior-describing JSDoc the sweep wrote was verified against the actual implementation. Nine wrong claims fixed (mute-button event ordering, captions-button "default track", seek-indicator timing claims, volume-slider 0–1 vs 0–100, etc.). Six ambiguous claims rewritten after design-philosophy discussion (boundary field wording, factory call-frequency promise, error-dialog responsibility framing).Test plan
pnpm typecheckcleanpnpm lintcleanpnpm build:packages— all 9 package builds succeed;.d.tsfiles emit cleanly@videojs/core*Coreclasses for behavior-method JSDoc accuracy — largest single chunk of agent-authored prose. Audit pass already flagged and fixed the obvious ones.Follow-up
pnpm check:workspaceso missing JSDoc on new public exports fails CI.https://claude.ai/code/session_01YDMP31nT51YrdWKSqvkT98