Skip to content

Commit ab0f30d

Browse files
crisbetojelbourn
authored andcommitted
fix(youtube-player): clean up internal observables (#17835)
Switches the internal observables of the `youtube-player` to be `Subject` so they're consistent with the rest of the repo and ensures that they're completed on destroy.
1 parent d4a2a0c commit ab0f30d

File tree

1 file changed

+27
-18
lines changed

1 file changed

+27
-18
lines changed

src/youtube-player/youtube-player.ts

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -93,48 +93,48 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
9393
get videoId(): string | undefined { return this._videoId; }
9494
set videoId(videoId: string | undefined) {
9595
this._videoId = videoId;
96-
this._videoIdObs.emit(videoId);
96+
this._videoIdObs.next(videoId);
9797
}
9898
private _videoId: string | undefined;
99-
private _videoIdObs = new EventEmitter<string | undefined>();
99+
private _videoIdObs = new Subject<string | undefined>();
100100

101101
/** Height of video player */
102102
@Input()
103103
get height(): number | undefined { return this._height; }
104104
set height(height: number | undefined) {
105105
this._height = height || DEFAULT_PLAYER_HEIGHT;
106-
this._heightObs.emit(this._height);
106+
this._heightObs.next(this._height);
107107
}
108108
private _height = DEFAULT_PLAYER_HEIGHT;
109-
private _heightObs = new EventEmitter<number>();
109+
private _heightObs = new Subject<number>();
110110

111111
/** Width of video player */
112112
@Input()
113113
get width(): number | undefined { return this._width; }
114114
set width(width: number | undefined) {
115115
this._width = width || DEFAULT_PLAYER_WIDTH;
116-
this._widthObs.emit(this._width);
116+
this._widthObs.next(this._width);
117117
}
118118
private _width = DEFAULT_PLAYER_WIDTH;
119-
private _widthObs = new EventEmitter<number>();
119+
private _widthObs = new Subject<number>();
120120

121121
/** The moment when the player is supposed to start playing */
122122
@Input() set startSeconds(startSeconds: number | undefined) {
123-
this._startSeconds.emit(startSeconds);
123+
this._startSeconds.next(startSeconds);
124124
}
125-
private _startSeconds = new EventEmitter<number | undefined>();
125+
private _startSeconds = new Subject<number | undefined>();
126126

127127
/** The moment when the player is supposed to stop playing */
128128
@Input() set endSeconds(endSeconds: number | undefined) {
129-
this._endSeconds.emit(endSeconds);
129+
this._endSeconds.next(endSeconds);
130130
}
131-
private _endSeconds = new EventEmitter<number | undefined>();
131+
private _endSeconds = new Subject<number | undefined>();
132132

133133
/** The suggested quality of the player */
134134
@Input() set suggestedQuality(suggestedQuality: YT.SuggestedVideoQuality | undefined) {
135-
this._suggestedQuality.emit(suggestedQuality);
135+
this._suggestedQuality.next(suggestedQuality);
136136
}
137-
private _suggestedQuality = new EventEmitter<YT.SuggestedVideoQuality | undefined>();
137+
private _suggestedQuality = new Subject<YT.SuggestedVideoQuality | undefined>();
138138

139139
/**
140140
* Whether the iframe will attempt to load regardless of the status of the api on the
@@ -157,8 +157,8 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
157157

158158
/** Whether we're currently rendering inside a browser. */
159159
private _isBrowser: boolean;
160-
private _youtubeContainer = new EventEmitter<HTMLElement>();
161-
private _destroyed = new EventEmitter<undefined>();
160+
private _youtubeContainer = new Subject<HTMLElement>();
161+
private _destroyed = new Subject<void>();
162162
private _player: Player | undefined;
163163

164164
constructor(
@@ -215,7 +215,7 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
215215
this.createEventsBoundInZone(),
216216
).pipe(waitUntilReady(), takeUntil(this._destroyed), publish());
217217

218-
/** Set up side effects to bind inputs to the player. */
218+
// Set up side effects to bind inputs to the player.
219219
playerObs.subscribe(player => this._player = player);
220220

221221
bindSizeToPlayer(playerObs, widthObs, heightObs);
@@ -256,15 +256,24 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
256256
}
257257

258258
ngAfterViewInit() {
259-
this._youtubeContainer.emit(this.youtubeContainer.nativeElement);
259+
this._youtubeContainer.next(this.youtubeContainer.nativeElement);
260260
}
261261

262262
ngOnDestroy() {
263263
if (this._player) {
264264
this._player.destroy();
265265
window.onYouTubeIframeAPIReady = undefined;
266-
this._destroyed.emit();
267266
}
267+
268+
this._videoIdObs.complete();
269+
this._heightObs.complete();
270+
this._widthObs.complete();
271+
this._startSeconds.complete();
272+
this._endSeconds.complete();
273+
this._suggestedQuality.complete();
274+
this._youtubeContainer.complete();
275+
this._destroyed.next();
276+
this._destroyed.complete();
268277
}
269278

270279
private _runInZone<T extends (...args: any[]) => void>(callback: T):
@@ -521,7 +530,7 @@ function bindCueVideoCall(
521530
startSecondsObs: Observable<number | undefined>,
522531
endSecondsObs: Observable<number | undefined>,
523532
suggestedQualityObs: Observable<YT.SuggestedVideoQuality | undefined>,
524-
destroyed: Observable<undefined>,
533+
destroyed: Observable<void>,
525534
) {
526535
const cueOptionsObs = combineLatest([startSecondsObs, endSecondsObs])
527536
.pipe(map(([startSeconds, endSeconds]) => ({startSeconds, endSeconds})));

0 commit comments

Comments
 (0)