Skip to content

Commit ab98e71

Browse files
arturovtAndrewKushnir
authored andcommitted
fix(common): remove placeholder image listeners once view is removed
Prior to this commit, attempting to resolve a `ChangeDetectorRef` after views or app have been destroyed would result in an error. In this commit, we clean up listeners once the view is destroyed, before the placeholder loads or fails to load. (cherry picked from commit feb86e3)
1 parent e5a19cb commit ab98e71

File tree

2 files changed

+51
-6
lines changed

2 files changed

+51
-6
lines changed

packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ export class NgOptimizedImage implements OnInit, OnChanges {
285285
private renderer = inject(Renderer2);
286286
private imgElement: HTMLImageElement = inject(ElementRef).nativeElement;
287287
private injector = inject(Injector);
288+
private destroyRef = inject(DestroyRef);
288289

289290
// An LCP image observer should be injected only in development mode.
290291
// Do not assign it to `null` to avoid having a redundant property in the production bundle.
@@ -410,10 +411,7 @@ export class NgOptimizedImage implements OnInit, OnChanges {
410411
if (ngDevMode) {
411412
this.lcpObserver = this.injector.get(LCPImageObserver);
412413

413-
// Using `DestroyRef` to avoid having an empty `ngOnDestroy` method since this
414-
// is only run in development mode.
415-
const destroyRef = inject(DestroyRef);
416-
destroyRef.onDestroy(() => {
414+
this.destroyRef.onDestroy(() => {
417415
if (!this.priority && this._renderedSrc !== null) {
418416
this.lcpObserver!.unregisterImage(this._renderedSrc);
419417
}
@@ -440,7 +438,7 @@ export class NgOptimizedImage implements OnInit, OnChanges {
440438
// This leaves the Angular zone to avoid triggering unnecessary change detection cycles when
441439
// `load` tasks are invoked on images.
442440
ngZone.runOutsideAngular(() =>
443-
assertNonZeroRenderedHeight(this, this.imgElement, this.renderer),
441+
assertNonZeroRenderedHeight(this, this.imgElement, this.renderer, this.destroyRef),
444442
);
445443
} else {
446444
assertNonEmptyWidthAndHeight(this);
@@ -453,7 +451,7 @@ export class NgOptimizedImage implements OnInit, OnChanges {
453451
// Only check for distorted images when not in fill mode, where
454452
// images may be intentionally stretched, cropped or letterboxed.
455453
ngZone.runOutsideAngular(() =>
456-
assertNoImageDistortion(this, this.imgElement, this.renderer),
454+
assertNoImageDistortion(this, this.imgElement, this.renderer, this.destroyRef),
457455
);
458456
}
459457
assertValidLoadingInput(this);
@@ -749,6 +747,14 @@ export class NgOptimizedImage implements OnInit, OnChanges {
749747
const removeLoadListenerFn = this.renderer.listen(img, 'load', callback);
750748
const removeErrorListenerFn = this.renderer.listen(img, 'error', callback);
751749

750+
// Clean up listeners once the view is destroyed, before the image
751+
// loads or fails to load, to avoid element from being captured
752+
// in memory and redundant change detection.
753+
this.destroyRef.onDestroy(() => {
754+
removeLoadListenerFn();
755+
removeErrorListenerFn();
756+
});
757+
752758
callOnLoadIfImageIsLoaded(img, callback);
753759
}
754760

@@ -1057,6 +1063,7 @@ function assertNoImageDistortion(
10571063
dir: NgOptimizedImage,
10581064
img: HTMLImageElement,
10591065
renderer: Renderer2,
1066+
destroyRef: DestroyRef,
10601067
) {
10611068
const callback = () => {
10621069
removeLoadListenerFn();
@@ -1164,6 +1171,14 @@ function assertNoImageDistortion(
11641171
removeErrorListenerFn();
11651172
});
11661173

1174+
// Clean up listeners once the view is destroyed, before the image
1175+
// loads or fails to load, to avoid element from being captured
1176+
// in memory and redundant change detection.
1177+
destroyRef.onDestroy(() => {
1178+
removeLoadListenerFn();
1179+
removeErrorListenerFn();
1180+
});
1181+
11671182
callOnLoadIfImageIsLoaded(img, callback);
11681183
}
11691184

@@ -1209,6 +1224,7 @@ function assertNonZeroRenderedHeight(
12091224
dir: NgOptimizedImage,
12101225
img: HTMLImageElement,
12111226
renderer: Renderer2,
1227+
destroyRef: DestroyRef,
12121228
) {
12131229
const callback = () => {
12141230
removeLoadListenerFn();
@@ -1236,6 +1252,14 @@ function assertNonZeroRenderedHeight(
12361252
removeErrorListenerFn();
12371253
});
12381254

1255+
// Clean up listeners once the view is destroyed, before the image
1256+
// loads or fails to load, to avoid element from being captured
1257+
// in memory and redundant change detection.
1258+
destroyRef.onDestroy(() => {
1259+
removeLoadListenerFn();
1260+
removeErrorListenerFn();
1261+
});
1262+
12391263
callOnLoadIfImageIsLoaded(img, callback);
12401264
}
12411265

packages/common/test/directives/ng_optimized_image_spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,6 +1291,27 @@ describe('Image directive', () => {
12911291
);
12921292
});
12931293

1294+
it('should remove placeholder event listeners once view is removed', () => {
1295+
const addEventListenerSpy = spyOn(HTMLImageElement.prototype, 'addEventListener');
1296+
const removeEventListenerSpy = spyOn(HTMLImageElement.prototype, 'removeEventListener');
1297+
setupTestingModule();
1298+
const template =
1299+
'<img ngSrc="path/img.png" width="400" height="300" placeholder="https://mysite.com/assets/my-image.png" />';
1300+
const fixture = createTestComponent(template);
1301+
fixture.detectChanges();
1302+
// The `load` event listener is being set up twice: once in
1303+
// `assertNoImageDistortion` and once in `removePlaceholderOnLoad`.
1304+
expect(
1305+
addEventListenerSpy.calls.all().filter((info) => info.args[0] === 'load').length,
1306+
).toEqual(2);
1307+
1308+
fixture.destroy();
1309+
1310+
expect(
1311+
removeEventListenerSpy.calls.all().filter((info) => info.args[0] === 'load').length,
1312+
).toEqual(2);
1313+
});
1314+
12941315
it('should replace the placeholder with the actual image on load', () => {
12951316
setupTestingModule();
12961317
const template = '<img ngSrc="path/img.png" width="400" height="300" placeholder="true" />';

0 commit comments

Comments
 (0)