Skip to content

Commit

Permalink
fix(material/button): resolve memory leaks in ripples (#28254)
Browse files Browse the repository at this point in the history
The `MatRippleLoader` that is used in the button doesn't track any ripples, but instead patches the ripples onto the DOM nodes which in theory should avoid leaks since the ripple will be collected together with the node. The problem is that each ripple registers itself with the `RippleEventManager` which needs to be notified on destroy so that it can dereference the DOM nodes and remove the event listeners.

These changes avoid the leaks by:
1. Destroying the ripple when the trigger is destroyed.
2. Cleaning up all the ripples when the ripple loader is destroyed.
3. No longer patching directives onto the DOM nodes.

Fixes #28240.

(cherry picked from commit a962bb7)
  • Loading branch information
crisbeto committed Jan 11, 2024
1 parent 214306f commit 16e4a87
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/material/button/button-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export class MatButtonBase

ngOnDestroy() {
this._focusMonitor.stopMonitoring(this._elementRef);
this._rippleLoader?.destroyRipple(this._elementRef.nativeElement);
}

/** Focuses the button. */
Expand Down
1 change: 1 addition & 0 deletions src/material/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ export class MatChip

ngOnDestroy() {
this._focusMonitor.stopMonitoring(this._elementRef);
this._rippleLoader?.destroyRipple(this._elementRef.nativeElement);
this._actionChanges?.unsubscribe();
this.destroyed.emit({chip: this});
this.destroyed.complete();
Expand Down
43 changes: 33 additions & 10 deletions src/material/core/private/ripple-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const matRippleDisabled = 'mat-ripple-loader-disabled';
*
* This service allows us to avoid eagerly creating & attaching MatRipples.
* It works by creating & attaching a ripple only when a component is first interacted with.
*
* @docs-private
*/
@Injectable({providedIn: 'root'})
export class MatRippleLoader implements OnDestroy {
Expand All @@ -49,6 +51,7 @@ export class MatRippleLoader implements OnDestroy {
private _globalRippleOptions = inject(MAT_RIPPLE_GLOBAL_OPTIONS, {optional: true});
private _platform = inject(Platform);
private _ngZone = inject(NgZone);
private _hosts = new Map<HTMLElement, MatRipple>();

constructor() {
this._ngZone.runOutsideAngular(() => {
Expand All @@ -59,6 +62,12 @@ export class MatRippleLoader implements OnDestroy {
}

ngOnDestroy() {
const hosts = this._hosts.keys();

for (const host of hosts) {
this.destroyRipple(host);
}

for (const event of rippleInteractionEvents) {
this._document?.removeEventListener(event, this._onInteraction, eventListenerOptions);
}
Expand Down Expand Up @@ -98,15 +107,13 @@ export class MatRippleLoader implements OnDestroy {

/** Returns the ripple instance for the given host element. */
getRipple(host: HTMLElement): MatRipple | undefined {
if ((host as any).matRipple) {
return (host as any).matRipple;
}
return this.createRipple(host);
const ripple = this._hosts.get(host);
return ripple || this._createRipple(host);
}

/** Sets the disabled state on the ripple instance corresponding to the given host element. */
setDisabled(host: HTMLElement, disabled: boolean): void {
const ripple = (host as any).matRipple as MatRipple | undefined;
const ripple = this._hosts.get(host);

// If the ripple has already been instantiated, just disable it.
if (ripple) {
Expand Down Expand Up @@ -134,19 +141,24 @@ export class MatRippleLoader implements OnDestroy {

const element = eventTarget.closest(`[${matRippleUninitialized}]`);
if (element) {
this.createRipple(element as HTMLElement);
this._createRipple(element as HTMLElement);
}
};

/** Creates a MatRipple and appends it to the given element. */
createRipple(host: HTMLElement): MatRipple | undefined {
private _createRipple(host: HTMLElement): MatRipple | undefined {
if (!this._document) {
return;
}

const existingRipple = this._hosts.get(host);
if (existingRipple) {
return existingRipple;
}

// Create the ripple element.
host.querySelector('.mat-ripple')?.remove();
const rippleEl = this._document!.createElement('span');
const rippleEl = this._document.createElement('span');
rippleEl.classList.add('mat-ripple', host.getAttribute(matRippleClassName)!);
host.append(rippleEl);

Expand All @@ -166,8 +178,19 @@ export class MatRippleLoader implements OnDestroy {
return ripple;
}

attachRipple(host: Element, ripple: MatRipple): void {
attachRipple(host: HTMLElement, ripple: MatRipple): void {
host.removeAttribute(matRippleUninitialized);
(host as any).matRipple = ripple;
this._hosts.set(host, ripple);
}

destroyRipple(host: HTMLElement) {
const ripple = this._hosts.get(host);

if (ripple) {
// Since this directive is created manually, it needs to be destroyed manually too.
// tslint:disable-next-line:no-lifecycle-invocation
ripple.ngOnDestroy();
this._hosts.delete(host);
}
}
}
5 changes: 3 additions & 2 deletions tools/public_api_guard/material/core.md
Original file line number Diff line number Diff line change
Expand Up @@ -399,13 +399,14 @@ export class MatRipple implements OnInit, OnDestroy, RippleTarget {
export class MatRippleLoader implements OnDestroy {
constructor();
// (undocumented)
attachRipple(host: Element, ripple: MatRipple): void;
attachRipple(host: HTMLElement, ripple: MatRipple): void;
configureRipple(host: HTMLElement, config: {
className?: string;
centered?: boolean;
disabled?: boolean;
}): void;
createRipple(host: HTMLElement): MatRipple | undefined;
// (undocumented)
destroyRipple(host: HTMLElement): void;
getRipple(host: HTMLElement): MatRipple | undefined;
// (undocumented)
ngOnDestroy(): void;
Expand Down

0 comments on commit 16e4a87

Please sign in to comment.