fix(react): commit retained callback refs - #2594
Draft
mihar-22 wants to merge 1 commit into
Draft
Conversation
Retained event machinery in the gesture, hotkey, menu, popover, tooltip and dialog roots read their callbacks and options through useLatestRef, which writes during render. A speculative or abandoned render could therefore hand a stale or never-committed callback to listeners and timers that outlive the render. Route those refs through useCommittedRef so only committed renders publish into retained closures. The public useLatestRef hook is unchanged. 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 — 30 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 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 #2589.
Summary
The gesture, hotkey, menu, popover, tooltip and dialog roots hand callbacks and options to retained machinery (
createTooltip,createPopover,createMenu,createDialog,createHotkey,createTapGesture,createDoubleTapGesture) that outlives any single render. Those closures readuseLatestRefrefs, which are written during render, so a speculative or abandoned render could hand a stale or never-committed callback to a listener or timer. This is theuseLatestReftouseCommittedRefaudit dropped from #2325: every ref read by retained machinery now publishes from the insertion phase, so only committed renders reach the retained closures. The publicuseLatestRefhook is unchanged.Changes
Converted to
useCommittedRef(each ref is read by a retained listener, timer, or core callback):Tooltip.Root:onOpenChange,onOpenChangeComplete,delay,closeDelay,disableHoverablePopup,disabled,sticky, tooltip group, popup groupPopover.Root:onOpenChange,onOpenChangeComplete,closeOnEscape,closeOnOutsideClick,openOnHover,delay,closeDelay, popup groupMenu.Root:onOpenChange,onOpenChangeComplete,closeOnEscape,closeOnOutsideClick, popup group,isSubmenuuseDialogRoot(shared byDialog.Root,AlertDialog.Root,ErrorDialog.Root):onOpenChange,onOpenChangeComplete,closeOnEscapeErrorDialog.Root: the error-state ref itsonOpenChangedismisses throughuseTapGesture,useDoubleTapGesture,useHotkey:onActivateIntentionally unconverted:
useLatestRefitself: public hook with a render-current contract.useSlider,TimeSlider.Root,VolumeSlider.Root: their refs are entangled with the retained slider cores and are converted together with the render-local slider projection in the follow-up PR.useDestroyalready useuseCommittedReffrom fix(react): add commit-safe ref and logging utilities #2325/fix(react): commit stateful provider props before subscriptions #2588.No site was found where a
useLatestRefvalue was only read during render or in same-component effects with correct deps; every remaining call was feeding a retained closure.Testing
use-hotkey.test.tsx: the retained hotkey calls the latest committedonActivatewithout re-registering, including under StrictMode.popover-root.test.tsx,dialog-root.test.tsx,tooltip-root.test.tsx: a re-render abandoned insideMockErrorBoundarydoes not get itsonOpenChangeadopted by the retained popup instance (verified to fail againstuseLatestRef);popover-root.test.tsxalso checks the latest committed callback is used under StrictMode.pnpm -F @videojs/react test: 76 files, 585 tests passedpnpm build --filter=@videojs/reactpnpm typecheckpnpm lint:fix:fileon touched files andgit diff --check🤖 Generated with Claude Code