Skip to content

Commit ae7e2bb

Browse files
authored
fix: avoid memeory leak in effects (#515)
1 parent 2b63b1c commit ae7e2bb

File tree

1 file changed

+19
-20
lines changed

1 file changed

+19
-20
lines changed

libs/ngu/carousel/src/lib/ngu-carousel/ngu-carousel.component.ts

+19-20
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
afterNextRender,
2626
AfterRenderPhase
2727
} from '@angular/core';
28-
import { EMPTY, Subject, Subscription, fromEvent, interval, merge, timer } from 'rxjs';
28+
import { EMPTY, Subject, fromEvent, interval, merge, timer } from 'rxjs';
2929
import { debounceTime, filter, map, startWith, switchMap, takeUntil } from 'rxjs/operators';
3030

3131
import { IS_BROWSER } from '../symbols';
@@ -80,24 +80,24 @@ export class NguCarousel<T, U extends NgIterable<T> = NgIterable<T>>
8080
readonly activePoint = signal(0);
8181
readonly pointNumbers = signal<number[]>([]);
8282

83-
inputs = input.required<NguCarouselConfig>();
84-
carouselLoad = output<number>();
85-
onMove = output<this>();
83+
readonly inputs = input.required<NguCarouselConfig>();
84+
readonly carouselLoad = output<number>();
85+
readonly onMove = output<this>();
8686

8787
private _defDirectives = contentChildren(NguCarouselDefDirective);
8888

8989
private _nodeOutlet = viewChild.required(NguCarouselOutlet);
9090

91-
nextButton = contentChild(NguCarouselNextDirective, { read: ElementRef });
92-
prevButton = contentChild(NguCarouselPrevDirective, { read: ElementRef });
91+
readonly nextButton = contentChild(NguCarouselNextDirective, { read: ElementRef });
92+
readonly prevButton = contentChild(NguCarouselPrevDirective, { read: ElementRef });
9393

94-
carouselMain1 = viewChild.required('ngucarousel', { read: ElementRef });
95-
_nguItemsContainer = viewChild.required('nguItemsContainer', { read: ElementRef });
96-
_touchContainer = viewChild.required('touchContainer', { read: ElementRef });
94+
readonly carouselMain1 = viewChild.required('ngucarousel', { read: ElementRef });
95+
readonly _nguItemsContainer = viewChild.required('nguItemsContainer', { read: ElementRef });
96+
readonly _touchContainer = viewChild.required('touchContainer', { read: ElementRef });
9797

9898
private _arrayChanges: IterableChanges<T> | null = null;
9999

100-
dataSource = input.required({
100+
readonly dataSource = input.required({
101101
transform: (v: NguCarouselDataSource<T, U>) => v || ([] as never)
102102
});
103103

@@ -125,8 +125,8 @@ export class NguCarousel<T, U extends NgIterable<T> = NgIterable<T>>
125125
* relative to the function to know if a Items should be added/removed/moved.
126126
* Accepts a function that takes two parameters, `index` and `item`.
127127
*/
128-
trackBy = input<TrackByFunction<T>>();
129-
_trackByFn = computed(() => {
128+
readonly trackBy = input<TrackByFunction<T>>();
129+
readonly _trackByFn = computed(() => {
130130
const fn = this.trackBy();
131131
if (NG_DEV_MODE && fn != null && typeof fn !== 'function' && console?.warn) {
132132
console.warn(`trackBy must be a function, but received ${JSON.stringify(fn)}.`);
@@ -165,27 +165,26 @@ export class NguCarousel<T, U extends NgIterable<T> = NgIterable<T>>
165165
{ allowSignalWrites: true }
166166
);
167167

168-
let preSub: Subscription;
169-
effect(() => {
170-
preSub?.unsubscribe();
168+
effect(cleanup => {
171169
const prevButton = this.prevButton();
172170
untracked(() => {
173171
if (prevButton) {
174-
preSub = fromEvent(prevButton.nativeElement, 'click')
172+
const preSub = fromEvent(prevButton.nativeElement, 'click')
175173
.pipe(takeUntil(this._destroy$))
176174
.subscribe(() => this._carouselScrollOne(0));
175+
cleanup(() => preSub.unsubscribe());
177176
}
178177
});
179178
});
180-
let nextSub: Subscription;
181-
effect(() => {
182-
nextSub?.unsubscribe();
179+
180+
effect(cleanup => {
183181
const nextButton = this.nextButton();
184182
untracked(() => {
185183
if (nextButton) {
186-
nextSub = fromEvent(nextButton.nativeElement, 'click')
184+
const nextSub = fromEvent(nextButton.nativeElement, 'click')
187185
.pipe(takeUntil(this._destroy$))
188186
.subscribe(() => this._carouselScrollOne(1));
187+
cleanup(() => nextSub.unsubscribe());
189188
}
190189
});
191190
});

0 commit comments

Comments
 (0)