fix(react): project core state per render instead of mutating retained cores - #2589
Draft
mihar-22 wants to merge 1 commit into
Draft
fix(react): project core state per render instead of mutating retained cores#2589mihar-22 wants to merge 1 commit into
mihar-22 wants to merge 1 commit into
Conversation
…d cores Media buttons, LiveButton, Controls.Root, Title, Time.Value, Poster, the popover, tooltip, menu, and dialog roots, VolumePopover.Root, SliderSegments, and the radio option hooks kept a component-lifetime core in `useState` and wrote this render's props, media, and input into it during render, so an abandoned render could leave the committed tree reading another render's values. Build those projections from render-local cores (or a shared instance when the core holds no state) and drop the `'use no memo'` directives that only guarded the retained mutation. Refs #1679 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
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 — no changesPresets (7)
Media (18)
Extensions (2)
Players (5)
Skins (29)
UI Components (50)
⚛️ @videojs/react — 51 small size changes
Presets (7)
Media (22)
Extensions (2)
Players (5)
Skins (26)
UI Components (39)
🧩 @videojs/core — no changesEntries (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 was referenced Sep 2, 2026
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
Depends on #2588.
Summary
Many UI roots kept a component-lifetime core in
useState(() => new *Core())and wrote this render's props, media, and transition input into it during render. Under concurrent rendering a speculative or abandoned render could therefore leave the committed tree reading another render's values, and memoizing compilers had to be kept away with'use no memo'. These cores are pure projections, so build them per render (or share one instance when the core holds no state at all).Changes
Converted to render-local projections:
createMediaButton(every media button),LiveButton,Controls.Root,Time.Value,PosterPopover.Root,Tooltip.Root,VolumePopover.Root, andMenu.Root(inside its existinguseMemo)useDialogRoot, shared byDialog.Root,AlertDialog.Root, andErrorDialog.RootcreateRadioOptionsHook: render-local core, plainsetValueclosure, and removal of the'use no memo'directive there and inuseQualityOptions,useCaptionsOptions, anduseAudioTrackOptions, whose only reason was the retained mutable coreTitleandSliderSegments: their cores hold no state, so one module-level instance projects every renderRetained on purpose (not render-mutated projections):
BufferingIndicatorCore: owns a state store and delay timers; props already apply in a layout effectStatusAnnouncerCore,TooltipGroupCore, and the input indicator cores: stateful, props committed in fix(react): commit stateful provider props before subscriptions #2588PopupPositioner(use-popup-position.ts): DOM side effects run in layout effects onlyTimeSliderChaptersCore(time-slider-chapters.tsx,time-slider-chapter-title.tsx): only holds an input-keyed memo cache read insideuseMemoThumbnailCore(thumbnail.tsx): handled with the thumbnail listener retargeting in fix(core): retarget thumbnail listeners when elements change #2590Follow-up (not mechanical, left unchanged):
SliderCore(slider-root.tsx),TimeSliderCore(time-slider-root.tsx),VolumeSliderCore(volume-slider-root.tsx): mutated during render and insideuseSlider'scomputeStateand drag callbacks, andTimeSliderCorecarries drag state (startDrag/endDrag). Converting these needs the slider callback contract reworked so callbacks do not capture a render-local core.Testing
pnpm -F @videojs/react test: 75 files, 579 tests passedpnpm build --filter=@videojs/reactpnpm typecheckpnpm lint:fix:fileon touched files andgit diff --check