[material-ui][ButtonBase] Fix LazyRipple retaining event objects when ripples are disabled - #49000
Open
dizhurnikita wants to merge 1 commit into
Open
[material-ui][ButtonBase] Fix LazyRipple retaining event objects when ripples are disabled#49000dizhurnikita wants to merge 1 commit into
dizhurnikita wants to merge 1 commit into
Conversation
… ripples are disabled
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
silviuaavram
requested review from
silviuaavram
and
a balanced review from Copilot
August 20, 2026 12:45
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes event retention in LazyRipple when ripples are disabled by ensuring its mount promise always settles.
Changes:
- Resolves the mount promise even when no
TouchRipplerenders. - Adds regression and action-forwarding tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/mui-material/src/useLazyRipple/useLazyRipple.ts |
Prevents indefinitely queued ripple actions. |
packages/mui-material/src/useLazyRipple/useLazyRipple.test.tsx |
Tests disabled and mounted ripple behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Fixes #48999
LazyRipple.mountedis resolved inmountEffectonly whenthis.ref.current !== null, i.e. only when aTouchRippleactually mounted. WithdisableRipple(commonly set app-wide viaMuiButtonBase.defaultProps), noTouchRippleever renders, somountednever settles — but the ripple handlers still run on every mousedown/mouseup/blur (they are skipped only fordisableTouchRipple), and eachstart(event)/stop(event)queuesthis.mount().then(() => this.ref.current?.stop(...args))on that forever-pending promise. Every interaction with every ripple-disabled ButtonBase therefore permanently retains its event object; when the event'starget/relatedTargetsits in later-removed DOM (a closed Dialog, an unmounted view), the entire detached tree is pinned for the page lifetime.Live reproduction: https://codesandbox.io/s/s44gdq — 100 clicks retain ~200
MouseEvents, each held viaPromiseReaction→LazyRipple.mounted; three Dialog open/close cycles retain three detachedMuiDialog-containertrees.The fix resolves
mountedinmountEffectregardless of whether a ripple rendered. The queued actions are already null-safe (this.ref.current?.start(...)), so with no ripple they settle as no-ops instead of accumulating. When a ripple does render, the ref is attached during commit — before the effect — so thedidMountbookkeeping and action forwarding behave exactly as before (covered by the second test).Adds
useLazyRipple.test.tsx(the hook previously had no dedicated tests): a regression test assertingmount()settles when no ripple is rendered, and a test asserting queued actions still reach a mounted ripple's actions object.