Skip to content

Commit

Permalink
fix(module:image): resolve memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
arturovt committed Jul 13, 2021
1 parent 503c6f9 commit 3e319b6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
10 changes: 7 additions & 3 deletions components/image/image-preview-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@

import { ESCAPE, hasModifierKey } from '@angular/cdk/keycodes';
import { OverlayRef } from '@angular/cdk/overlay';
import { filter, take } from 'rxjs/operators';
import { Subject } from 'rxjs';
import { filter, take, takeUntil } from 'rxjs/operators';

import { NzImagePreviewOptions } from './image-preview-options';
import { NzImagePreviewComponent } from './image-preview.component';

export class NzImagePreviewRef {
private destroy$ = new Subject<void>();

constructor(
public previewInstance: NzImagePreviewComponent,
private config: NzImagePreviewOptions,
Expand All @@ -28,11 +31,11 @@ export class NzImagePreviewRef {
this.overlayRef.dispose();
});

previewInstance.containerClick.pipe(take(1)).subscribe(() => {
previewInstance.containerClick.pipe(take(1), takeUntil(this.destroy$)).subscribe(() => {
this.close();
});

previewInstance.closeClick.pipe(take(1)).subscribe(() => {
previewInstance.closeClick.pipe(take(1), takeUntil(this.destroy$)).subscribe(() => {
this.close();
});

Expand Down Expand Up @@ -63,6 +66,7 @@ export class NzImagePreviewRef {
}

private dispose(): void {
this.destroy$.next();
this.overlayRef.dispose();
}
}
34 changes: 20 additions & 14 deletions components/image/image.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
Optional,
SimpleChanges
} from '@angular/core';
import { Subject } from 'rxjs';
import { fromEvent, Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';

import { NzConfigKey, NzConfigService, WithConfig } from 'ng-zorro-antd/core/config';
Expand Down Expand Up @@ -142,19 +142,25 @@ export class NzImageDirective implements OnInit, OnChanges, OnDestroy {
this.getElement().nativeElement.srcset = this.nzSrcset;
}

this.backLoadImage.onload = () => {
this.status = 'normal';
this.getElement().nativeElement.src = this.nzSrc;
this.getElement().nativeElement.srcset = this.nzSrcset;
};

this.backLoadImage.onerror = () => {
this.status = 'error';
if (this.nzFallback) {
this.getElement().nativeElement.src = this.nzFallback;
this.getElement().nativeElement.srcset = '';
}
};
// The `nz-image` directive can be destroyed before the `load` or `error` event is dispatched,
// so there's no sense to keep capturing `this`.
fromEvent(this.backLoadImage, 'load')
.pipe(takeUntil(this.destroy$))
.subscribe(() => {
this.status = 'normal';
this.getElement().nativeElement.src = this.nzSrc;
this.getElement().nativeElement.srcset = this.nzSrcset;
});

fromEvent(this.backLoadImage, 'error')
.pipe(takeUntil(this.destroy$))
.subscribe(() => {
this.status = 'error';
if (this.nzFallback) {
this.getElement().nativeElement.src = this.nzFallback;
this.getElement().nativeElement.srcset = '';
}
});
}
}
}
5 changes: 2 additions & 3 deletions components/image/image.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ describe('Placeholder', () => {
fixture.detectChanges();
tick(300);
fixture.detectChanges();
// @ts-ignore
imageComponent.backLoadImage.onload({});
imageComponent.backLoadImage.dispatchEvent(new Event('load'));
fixture.detectChanges();
expect(imageElement.getAttribute('src')).toBe(QUICK_SRC);
}));
Expand Down Expand Up @@ -114,7 +113,7 @@ describe('Fallback', () => {
context.src = 'error.png';
context.fallback = FALLBACK;
fixture.detectChanges();
context.image.backLoadImage.onerror!('');
context.image.backLoadImage.dispatchEvent(new ErrorEvent('error'));
tick();
fixture.detectChanges();
const image = debugElement.nativeElement.querySelector('img');
Expand Down

0 comments on commit 3e319b6

Please sign in to comment.