refactor(html)!: replace ContainerMixin with ContainerElement - #2280
Merged
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.
|
@videojs/core
@videojs/element
@videojs/html
@videojs/media
@videojs/react
@videojs/spf
@videojs/store
@videojs/utils
commit: |
Contributor
📦 Bundle Size Report🎨 @videojs/html
Small changes (83, ≤ 300 B)
Presets (7)
Media (19)
Players (5)
Skins (29)
UI Components (44)
⚛️ @videojs/react — 42 small size changes
Presets (7)
Media (23)
Players (5)
Skins (26)
UI Components (37)
🧩 @videojs/core — 2 small size changes
Entries (76)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — 2 small size changes
Entries (13)
📦 @videojs/media — no changesEntries (20)
📦 @videojs/spf — no changesEntries (10)
ℹ️ How to interpretEach entry is independently bundled, minified, and brotli-compressed. Initial size includes its static import graph; lazy dynamic chunks are reported separately. Entries are not additive because their dependency graphs overlap. Preset rows represent realistic combined bundles. Changes of 300 B or less across initial, lazy, and total size are collapsed, not discarded. Run |
This was referenced Aug 20, 2026
mihar-22
force-pushed
the
t3code/html-container-element
branch
from
August 21, 2026 20:51
50d2f3c to
2cbc807
Compare
mihar-22
marked this pull request as ready for review
August 21, 2026 20:56
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2cbc807. Configure here.
luwes
approved these changes
Aug 22, 2026
luwes
left a comment
luwes
left a comment
Collaborator
There was a problem hiding this comment.
LGTM!
just reminder of the import weirdness, can possibly look at after.
heads up: this pr also changes the changelog of a previous beta.
Merged
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.
Refs #2364
Summary
Replace the container mixin with one concrete
ContainerElement, move its registration path to@videojs/html/ui/container, and make player/media/container attachment safe when elements connect, disconnect, reorder, or upgrade late.This replaces #2244, which GitHub marked merged into the old feature branch rather than
mainwhen that stack was rebuilt.Before
After
<media-container>keeps the same tag name. Registration and lifecycle docs, demos, the VJSC HTML target, and API-doc fixtures move with the implementation.Testing
pnpm -F @videojs/html test— 401 testspnpm -F @videojs/html buildpnpm --dir packages/skins exec vitest run vjsc/tests/vite.test.ts— 6 testspnpm --dir site test— 608 testspnpm typecheckpnpm check:workspaceNote
High Risk
Breaking public HTML API: mixin removal, import path change, and attach lifecycle rewrite can mis-attach media or containers if registration/release is wrong.
Overview
Replaces HTML
ContainerMixinwith a concreteContainerElement(<media-container>). Custom containers now subclass that class instead of composing mixins, and registration moves from@videojs/html/media/containerto@videojs/html/ui/container.Media and container attach now use identity-safe
register*callbacks that return a release function, so disconnecting an older element cannot clear a newer one. Native<video>/<audio>are tracked with a subtreeMutationObserverinstead of a one-shot querySelector fallback.Docs, demos, skins targeting, and API-doc fixtures follow the new import path and drop the mixin composition examples.
Reviewed by Cursor Bugbot for commit 1539ad6. Bugbot is set up for automated code reviews on this repo. Configure here.