refactor(core): invoke media methods via callProp#1695
Closed
luwes wants to merge 1 commit into
Closed
Conversation
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (9)
Players (5)
Skins (30)
UI Components (37)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (8)
Skins (27)
UI Components (31)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (11)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (4)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
luwes
added a commit
that referenced
this pull request
Jun 17, 2026
Split media utils into components/config modules and rework how component config is exposed on the host. - Component config is no longer bridged via Object.defineProperty on the config object. The getter enumerates registered components live under their configKey; the setter routes matching keys onto the component. - Assigning a new config object now resets the free-form host/engine bag (start-fresh intent) instead of merging. Component state is preserved and only overwritten per-key by the incoming config. - Reading config returns a fresh object, so mutating the result no longer writes through to components; use the setter instead. Stacked on #1695.
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.
Fix for feedback from #1661
Summary
Media methods (
play,pause,load,canPlayType,addTextTrack) now invoke through the owner directly instead of reading a function prop and calling it detached. This removes the need to bind on every access — and the referential-stability cache that bind required — since a function reference is never handed out.Changes
callProp(host, prop, ...args)to resolve the owning component/target and invoke the method with the owner asthis, returningundefinedwhen no owner exposes it.getPropto return the raw owner value (noisFunctionbranch, nobind).callProp.Why not cache the bound function?
Every consumer invokes immediately and discards the function, so no caller observes referential identity. The
bindonly ever existed to fixthison a detached call. Resolving and invoking in one step makes the cache unnecessary.callPropis typed against the concreteTargetLikeconstraint soParameters/ReturnTypeinference resolves inside the generic host.Testing
Covered by
utils.test.ts(callPropreceiver/return/no-owner) and existingmedia-host.test.ts.pnpm -F @videojs/core buildand thecoretypecheck pass.Note
Medium Risk
Touches core playback/control delegation (
play,pause, etc.) where component overrides must keep correctthis; behavior is intended to be equivalent but is on a sensitive path.Overview
Media host methods no longer pull detached function references from
getPropand call them without the correct receiver.callPropresolves the owning component override or attached target and invokes the method with that owner asthis.getPropnow returns the raw property value only (nobind/ function branch).play,pause,load,canPlayType, andaddTextTrackonHTMLMediaElementHostroute throughcallProp, with the same fallbacks when nothing is attached (e.g. rejectedplay).callPropis re-exported from the media host module.Adds
utils.test.tscoveringgetPropandcallProp(receiver, return value, missing owner). MinoraddComponentnaming/comments only.Reviewed by Cursor Bugbot for commit 9b2f997. Bugbot is set up for automated code reviews on this repo. Configure here.