feat: content metadata via provider config and derived store state - #1886
Draft
decepulis wants to merge 8 commits into
Draft
feat: content metadata via provider config and derived store state#1886decepulis wants to merge 8 commits into
decepulis wants to merge 8 commits into
Conversation
A slice may declare `derived` formulas alongside its state. The store computes them eagerly inside `patch` and folds the answers into the same frozen snapshot, before it notifies — so a derived key stays an ordinary value and every existing read path (store getters, selectors, `store.state`, `shallowEqual`) keeps working unchanged. Three rules keep it small: - Every formula reruns on every patch that changed something. No dependency tracking, so formulas must be cheap. - A formula reads source keys only. Reading another formula's output is a compile error, which removes run ordering and cycle detection. - The formula is the only writer of its key. `patch` strips derived keys from incoming partials and warns in `__DEV__`. That last rule is what makes `detach` order-independent: whether cleanup or the reset runs first, the source keys end up the same and the formula produces the same answer. `patch` also now iterates with `Reflect.ownKeys` rather than `for...in`, so symbol-keyed entries participate — previously a symbol patch was a silent no-op. It copies the state object lazily, only once a real change is found, which drops the separate `changed` flag and the `hasOwnProperty` guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three per-field capabilities — `contentTitle`, `contentPoster`, and `contentPosterAlt` — each with its own change event and predicate, plus host getters and setters following the `streamType` pattern. These are deliberately separate from `title` and `poster` and are not reconciled with them. Those are the developer's settings on the element — the native tooltip, the `<video poster>` frame. The `content*` properties are facts about the content, usually from a backend. Absence is `null` or `undefined`; an empty string is a real value meaning deliberately blank, and a media must never manufacture one. The host's backing fields start as `null` rather than `undefined` so a declaring host stays capable even before a value arrives — otherwise capability would flicker, and since a feature checks it once at attach, a value arriving later would never reach the store. `CustomMediaElement` declarations use `empty: null` for the same reason, following `preload` rather than the neighbouring `poster` and `src`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… feature
A feature may declare fields the developer can set on the player provider, as
React props and HTML attributes. Each declaration names one of the feature's own
actions as the write target, because tier slots are private to a feature and the
provider is generic — naming an action is how a generic provider hands a value to
a feature that knows what to do with it. It also means the declarative and
imperative paths run through one function and cannot drift.
`createPlayer` collects declarations once, so a prop exists only when its feature
is composed. A `__DEV__` check catches duplicate names and names that already
exist on the provider element's prototype chain — tested with `in` rather than
`getOwnPropertyDescriptor`, which sees only own properties and would miss an
inherited DOM property like `title`.
The new `contentMetadataFeature` resolves each field from three symbol-keyed
slots: `user ?? media ?? developerDefault ?? ''`. Separate slots rather than one
slot with write-time precedence, so they resolve identically however the writes
arrive. Media slots are seeded in `state()` so `detach` clears them; developer
slots are left out so the reset cannot reach them. It joins all four real presets.
Orientation lock moves onto the same mechanism and the configurable-feature path
in `feature.ts` is deleted. Its value is now read at lock time rather than
captured at attach, which is the actual behavioural change. It declares one prop,
not two — nothing donates an orientation preference, so a `default*` tier would
just be a second developer value with lower precedence.
BREAKING CHANGE: `definePlayerFeature` no longer takes a second config argument.
`orientationLockFeature({ type })` becomes the `orientation-lock` provider
attribute or `store.setOrientationLock()`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Provider props are seeded inside the `useState` initializer that builds the store, and later changes go through a layout effect. Not the render body: a store built during render has no subscribers yet, so a render React throws away discards the seeded values with it, but writing to an already-subscribed store during render can leave a value behind from a render that never committed. Seeding still happens during render, including on the server, so server HTML carries the right values and there is no hydration flash. The accepted cost is that on the pass where a prop changes, children render the stale value first — children render before the parent's layout effect. Nothing flashes, because the notification's microtask drains before paint. A passive effect would be worse: it permits a paint in between. `<Poster>` with no `src` now reads the resolved value from the store, and fills `alt` from `contentPosterAlt`. A local `src` short-circuits that and the store is not consulted at all, so this is not a fourth precedence tier — the component only decides whether to ask. `srcSet`, `loading`, and the render prop are untouched. `alt` is decided by presence, never emptiness, since `alt=""` is the platform's marker for a decorative image. BREAKING CHANGE: the skins' `poster` prop is removed. Set `content-poster` on the provider instead, or pass `src` to `<Poster>` directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e store Declared fields become ordinary reactive properties on the provider element, so `<video-player content-title="…">` runs through the attribute engine the element already has rather than a third mechanism. `ReactiveElement`'s generated accessor keeps its own symbol-keyed backing value, which matters: after `el.contentTitle = undefined` hands control back to the media, reading the property must still report what was assigned, not the media's title. The write happens in two places, deliberately. An explicit sync in `connectedCallback` before `#tryAttach()` — attributes are parsed during upgrade, but the first `update()` only runs a microtask after connect, so without it the store would attach and the media would report its own metadata before the developer's values landed. Later changes take the ordinary Lit path via `willUpdate`. `<media-poster>` fills an empty `src` and a missing `alt` on the image the author supplied, and never creates one — a bare `<media-poster>` with no image renders nothing, by design. Default skins get zero-markup behaviour from a fallback `<img>` inside their own poster slot; an author-supplied `slot="poster"` image still wins. No shadow root is added. Ownership of each attribute is snapshotted the first time an image is seen rather than re-derived from presence, because after the first fill the attribute is present either way — re-deriving would freeze a playlist on the first video's poster. The platform asymmetry with React's `<Poster>` is deliberate: React's component owns its image, HTML's element wraps yours. Behaviour is identical either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The React skins no longer take a `poster` prop, so the sandbox templates and the e2e page generator set `content-poster` on the provider instead. The HTML templates are unchanged — an author-supplied `slot="poster"` image still wins over the skin's fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nfig Two new records and three amendments, per the design notes' paperwork list: - `store/derived-state.md` — the primitive, and why tracking, compute-on-read, a widened state type, and visible string keys for tier slots were all rejected. - `media/content-metadata.md` — the capability contract, including the half that is not structurally enforced: dispatching on change is, clearing on source change is convention with nothing checking it. - `store/reactive-state.md` — marked partly superseded. Derivation is now eager, and a `derived` map means a patch can change keys the caller did not name, which is a real reduction in the explicitness that record valued. Also notes that its "computed values" most likely describes selectors, so this work should not be framed as finishing a ratified design. - `rfc/player-api.md` — amended for the provider-config surface, since it changes a public API that RFC settled. Reconciles the deferred symbol-feature-keys alternative and the provider-complexity concerns from the player records. - The two poster design records — reconciled. The poster fills in rather than takes over, so `poster.md`'s rejection of a component-managed `src` still holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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. |
decepulis
changed the title
Contributor
📦 Bundle Size Report🎨 @videojs/html
Presets (7)
Media (10)
Players (5)
Skins (30)
UI Components (39)
Sizes are marginal over the root entry point. ⚛️ @videojs/react
Presets (7)
Media (9)
Skins (27)
UI Components (33)
Sizes are marginal over the root entry point. 🧩 @videojs/core
Entries (72)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (12)
📦 @videojs/media
Entries (14)
📦 @videojs/spf — no changesEntries (4)
ℹ️ How to interpretJS sizes are initial static graph totals (minified + brotli). Lazy dynamic chunks are shown separately when present.
Run |
Q30 asked that the derived-map merge in `combine` not repeat the existing duplicate-state-key warning's habit of dropping the slice name. It did. The derived warning now names the declaring slice, which matters more for a formula than for a state key — you cannot grep for it. Q21's rendering rule is that an empty resolved value means render nothing, "no poster element", rather than an image with an empty source. `<Poster>` was returning an `<img>` with no `src` attribute, which avoided the `src=""` request-the-current-page bug but not the element. Since the skins now render `<Poster />` unconditionally, that put a stray `<img>` in every player without a poster — and the blur-up placeholder keys off `img[data-visible]:not([data-loaded])`, so a skin given a `placeholder` but no `content-poster` would show it forever, waiting for a load that can never happen. `<Poster>` now returns null when there is nothing to render, keeping the `srcSet` and `render` escape hatches, and a local `src=""` takes the same path — the deliberate local exception Q19 anticipated. The JSDoc explaining why the `content*` fields exist beside `title` and `poster` — Q21's open obligation — was attached to `MediaContentTitleEvents` rather than the capability. The site builder extracts JSDoc for API reference pages, so it was documenting the wrong export. Moved onto the capability, with the poster and alt capabilities pointing at it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Warning
Draft. AI-generated in one shot, for reference — not for review.
Implements the metadata design end to end: developers provide metadata through
the provider, media can provide it too, and the store reconciles them by
precedence. All three fields —
contentTitle,contentPoster,contentPosterAlt— across store, media, core, React, and HTML. Orientation lock is migrated onto
the same mechanism and the old configurable-feature path is deleted.
Layers
Seven commits, one per layer, in the order the design notes suggested.
feat(store)derivedprimitive; symbol-aware, lazily-copyingpatchfeat(media)feat(core)contentMetadataFeature, presets, orientation lockfeat(react)<Poster>feat(html)<media-poster>filling the author's imagechore(sandbox)docs(design)Checks
pnpm typecheckclean.pnpm lintclean on all 68 changed source files.2,582 tests pass across the five packages (139 store / 427 media / 1362 core /
374 react / 280 html), including ~90 new ones.
pnpm check:workspacepasses 9 of 10 — the failure is.claude/plans: must resolve to .agents/plans, which is pre-existing local environment state(confirmed by running it on a clean tree) and unrelated to this branch.
The gate checklist from the design notes holds: derived values recompute on tier
writes; symbol tiers stay absent from
store.state, selector output, andcombine's duplicate-key warning; detach-then-reattach preserves developer tiers,clears the media tier, and leaves the resolved value correct; provider props are
composition-gated, proven with
@ts-expect-error; the seeding path survives thedevelopment double render; and three fields share the machinery without the
authoring cost feeling silly.
The one thing the design left open, and what I decided
The HTML write path was explicitly "early ideas only" and was the largest gap.
I decided it as follows, and this is the part most worth a second opinion:
element already has does the work. No third attribute system.
connectedCallbackbefore#tryAttach(), because attributes are parsedduring upgrade but the first
update()only runs a microtask after connect — sowithout it the store would attach, and the media would report its own metadata,
before the developer's values landed. Later changes take the ordinary Lit path
via
willUpdate.ReactiveElement's generated accessor alreadykeeps its own symbol-keyed backing value, so the property keeps reporting what
was assigned rather than the store's resolved value. No custom accessor needed.
Two smaller open items I also had to decide:
providerProps, notconfig. "Config" isalready taken three times over — store lifecycle callbacks, the media host's
config bag, and the path this replaces.
three-tier shape was free once the machinery existed.
Two bugs the design would have shipped
Both found by writing the tests, and both fixed here:
createScreenOrientationLockdestructuredtypeeagerly, so "read thevalue at lock time" silently did not work — the store half looks finished while
the lock still uses whatever was set at attach. Now read from
config.typeinside
lock().<media-poster>decidingalt/srcownership by presence on every passwould freeze a playlist on the first video's poster, because after the first
fill the attribute is present either way. Ownership is now snapshotted once.
Known gaps
posterprop is removed, andsite/src/content/docs/still documents it (along with thepackages/cli/docs/bundle generated from it). Deliberately out of scope for a reference PR —
content writing deserves the
write-docsworkflow, not a one-shot.AttachContext.get()now returns source and derived state. A smallwidening the notes did not anticipate, needed because orientation lock reads its
own resolved value at lock time. Writes stay source-only.
EmptyObjectwas added to@videojs/utils/typesso the new genericdefaults could avoid a bare
{}. The only in-repo precedent for that lint wasin
packages/spf, which I was told to treat as a separate project and did notread.
cannot reach resolved values — they sit below the store.
google-cast-provider.tsstill builds Cast metadata from the raw element poster and sets no title.
contentPlaceholderis not built. Still a genuinely different concept.🤖 Generated with Claude Code