fix(core): retarget thumbnail listeners when elements change - #2590
Draft
mihar-22 wants to merge 1 commit into
Draft
fix(core): retarget thumbnail listeners when elements change#2590mihar-22 wants to merge 1 commit into
mihar-22 wants to merge 1 commit into
Conversation
`createThumbnail` bound its img listeners and resize observer to the first elements it saw and never looked again, so a render override or remount that replaced them for the same source stopped receiving load, error, and resize updates. Rebind to whichever elements are current on every binding pass and make `connect()` idempotent so it can run on each commit. React's `Thumbnail` now publishes its source from a layout effect instead of during render and projects the pending loading state, correcting before paint when the committed handle settles differently. Refs #1679 Co-Authored-By: Claude Fable 5.1 <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. |
This was referenced Sep 2, 2026
@videojs/core
@videojs/element
@videojs/html
@videojs/media
@videojs/react
@videojs/spf
@videojs/store
@videojs/utils
commit: |
Contributor
📦 Bundle Size Report🎨 @videojs/html — 16 small size changes
Presets (7)
Media (18)
Extensions (2)
Players (5)
Skins (29)
UI Components (50)
⚛️ @videojs/react — 14 small size changes
Presets (7)
Media (22)
Extensions (2)
Players (5)
Skins (26)
UI Components (39)
🧩 @videojs/core — 1 small size change
Entries (76)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (13)
📦 @videojs/media
Entries (23)
📦 @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 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.
Refs #1679
Independent of the #2325 stack; based on
main.Summary
createThumbnailbound itsload/errorlisteners to the first<img>it saw and observed the first container, then never looked again. When a Reactrenderoverride (or any remount) replaces those nodes for the same source, the retained handle kept listening to detached elements, so load/error and resize updates stopped arriving. On the React side,Thumbnailalso wrote the requested source into the retained handle during render, so an abandoned render could restart loading or rebind listeners for a source that was never shown.Changes
packages/core/src/dom/ui/thumbnail.ts: retarget the img listeners and resize observation whenevergetImg()/getContainer()return different elements;destroy()releases both.connect()now notifies only when settling an already-complete image actually changes state, so callers can connect on every commit. The failed-sheet memory from fix: scale seek-bar thumbnails to fill their box #2517 (failedSrcs/markFailed) is preserved unchanged.packages/react/src/ui/thumbnail/thumbnail.tsx: publishupdateSrcandconnectfrom a layout effect on every commit instead of during render and mount only; project the pending source's loading state during render and correct it before paint if the committed handle settles differently (for example a sheet already known to fail); share one statelessThumbnailCoreinstance and useuseForceRender.connect()inpackages/core/src/dom/ui/tests/thumbnail.test.ts; committed-only source publication, SSR projection, StrictMode replay, cached-image settling, failed-sheet re-entry, and render-override retargeting inpackages/react/src/ui/thumbnail/tests/thumbnail.test.tsx.The HTML
<media-thumbnail>element only callsupdateSrcwith a stable img and container, so its behaviour is unchanged.Testing
pnpm -F @videojs/core test: 102 files, 1,493 tests passed (src/dom/ui: 16 files, 365 tests)pnpm -F @videojs/react test: 65 files, 544 tests passedpnpm -F @videojs/html test src/ui/thumbnail: 6 tests passed against the rebuilt corepnpm build --filter=@videojs/core,pnpm build --filter=@videojs/reactpnpm typecheckpnpm lint:fix:fileon touched files andgit diff --check