Skip to content

Commit 0695e82

Browse files
crisbetojelbourn
authored andcommitted
fix(youtube-player): unable to bind to events after initialization (#18996)
The `youtube-player` component is set up so that it won't bind any events if there aren't any listeners at the time the player is created. This makes it impossible to bind an event by getting a reference to the player using `ViewChild` and subscribing to it. These changes fix the issue using a lazy observable which binds any events once the user subscribes and transfers them between players.
1 parent d26ba73 commit 0695e82

File tree

5 files changed

+147
-61
lines changed

5 files changed

+147
-61
lines changed

src/youtube-player/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ ng_test_library(
4444
deps = [
4545
":youtube-player",
4646
"@npm//@angular/platform-browser",
47+
"@npm//rxjs",
4748
],
4849
)
4950

src/youtube-player/fake-youtube-player.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,38 @@ export function createFakeYtNamespace(): FakeYtNamespace {
3535
]);
3636

3737
let playerConfig: YT.PlayerOptions | undefined;
38+
const boundListeners = new Map<keyof YT.Events, Set<(event: any) => void>>();
3839
const playerCtorSpy = jasmine.createSpy('Player Constructor');
40+
3941
playerCtorSpy.and.callFake((_el: Element, config: YT.PlayerOptions) => {
4042
playerConfig = config;
4143
return playerSpy;
4244
});
4345

44-
const eventHandlerFactory = (name: keyof YT.Events) => {
46+
playerSpy.addEventListener.and.callFake((name: keyof YT.Events, listener: (e: any) => any) => {
47+
if (!boundListeners.has(name)) {
48+
boundListeners.set(name, new Set());
49+
}
50+
boundListeners.get(name)!.add(listener);
51+
});
52+
53+
playerSpy.removeEventListener.and.callFake((name: keyof YT.Events, listener: (e: any) => any) => {
54+
if (boundListeners.has(name)) {
55+
boundListeners.get(name)!.delete(listener);
56+
}
57+
});
58+
59+
function eventHandlerFactory(name: keyof YT.Events) {
4560
return (arg: Object = {}) => {
4661
if (!playerConfig) {
4762
throw new Error(`Player not initialized before ${name} called`);
4863
}
4964

50-
if (playerConfig && playerConfig.events && playerConfig.events[name]) {
51-
playerConfig.events[name]!(arg as any);
52-
}
53-
54-
for (const [event, callback] of playerSpy.addEventListener.calls.allArgs()) {
55-
if (event === name) {
56-
callback(arg as YT.PlayerEvent);
57-
}
65+
if (boundListeners.has(name)) {
66+
boundListeners.get(name)!.forEach(callback => callback(arg));
5867
}
5968
};
60-
};
69+
}
6170

6271
const events: Required<YT.Events> = {
6372
onReady: eventHandlerFactory('onReady'),

src/youtube-player/youtube-player.spec.ts

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {Component, ViewChild} from '@angular/core';
33
import {YouTubePlayerModule} from './youtube-module';
44
import {YouTubePlayer, DEFAULT_PLAYER_WIDTH, DEFAULT_PLAYER_HEIGHT} from './youtube-player';
55
import {createFakeYtNamespace} from './fake-youtube-player';
6+
import {Subscription} from 'rxjs';
67

78
const VIDEO_ID = 'a12345';
89

@@ -22,7 +23,7 @@ describe('YoutubePlayer', () => {
2223

2324
TestBed.configureTestingModule({
2425
imports: [YouTubePlayerModule],
25-
declarations: [TestApp, StaticStartEndSecondsApp],
26+
declarations: [TestApp, StaticStartEndSecondsApp, NoEventsApp],
2627
});
2728

2829
TestBed.compileComponents();
@@ -411,6 +412,48 @@ describe('YoutubePlayer', () => {
411412
jasmine.objectContaining({startSeconds: 42, endSeconds: 1337}));
412413
});
413414

415+
it('should be able to subscribe to events after initialization', () => {
416+
const noEventsApp = TestBed.createComponent(NoEventsApp);
417+
noEventsApp.detectChanges();
418+
events.onReady({target: playerSpy});
419+
noEventsApp.detectChanges();
420+
421+
const player = noEventsApp.componentInstance.player;
422+
const subscriptions: Subscription[] = [];
423+
const readySpy = jasmine.createSpy('ready spy');
424+
const stateChangeSpy = jasmine.createSpy('stateChange spy');
425+
const playbackQualityChangeSpy = jasmine.createSpy('playbackQualityChange spy');
426+
const playbackRateChangeSpy = jasmine.createSpy('playbackRateChange spy');
427+
const errorSpy = jasmine.createSpy('error spy');
428+
const apiChangeSpy = jasmine.createSpy('apiChange spy');
429+
430+
subscriptions.push(player.ready.subscribe(readySpy));
431+
events.onReady({target: playerSpy});
432+
expect(readySpy).toHaveBeenCalledWith({target: playerSpy});
433+
434+
subscriptions.push(player.stateChange.subscribe(stateChangeSpy));
435+
events.onStateChange({target: playerSpy, data: 5});
436+
expect(stateChangeSpy).toHaveBeenCalledWith({target: playerSpy, data: 5});
437+
438+
subscriptions.push(player.playbackQualityChange.subscribe(playbackQualityChangeSpy));
439+
events.onPlaybackQualityChange({target: playerSpy, data: 'large'});
440+
expect(playbackQualityChangeSpy).toHaveBeenCalledWith({target: playerSpy, data: 'large'});
441+
442+
subscriptions.push(player.playbackRateChange.subscribe(playbackRateChangeSpy));
443+
events.onPlaybackRateChange({target: playerSpy, data: 2});
444+
expect(playbackRateChangeSpy).toHaveBeenCalledWith({target: playerSpy, data: 2});
445+
446+
subscriptions.push(player.error.subscribe(errorSpy));
447+
events.onError({target: playerSpy, data: 5});
448+
expect(errorSpy).toHaveBeenCalledWith({target: playerSpy, data: 5});
449+
450+
subscriptions.push(player.apiChange.subscribe(apiChangeSpy));
451+
events.onApiChange({target: playerSpy});
452+
expect(apiChangeSpy).toHaveBeenCalledWith({target: playerSpy});
453+
454+
subscriptions.forEach(subscription => subscription.unsubscribe());
455+
});
456+
414457
});
415458

416459
/** Test component that contains a YouTubePlayer. */
@@ -448,9 +491,18 @@ class TestApp {
448491

449492
@Component({
450493
template: `
451-
<youtube-player [videoId]="videoId" [startSeconds]="42"[endSeconds]="1337"></youtube-player>
494+
<youtube-player [videoId]="videoId" [startSeconds]="42" [endSeconds]="1337"></youtube-player>
452495
`
453496
})
454497
class StaticStartEndSecondsApp {
455498
videoId = VIDEO_ID;
456499
}
500+
501+
502+
@Component({
503+
template: `<youtube-player [videoId]="videoId"></youtube-player>`
504+
})
505+
class NoEventsApp {
506+
@ViewChild(YouTubePlayer) player: YouTubePlayer;
507+
videoId = VIDEO_ID;
508+
}

src/youtube-player/youtube-player.ts

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
ChangeDetectionStrategy,
1515
Component,
1616
ElementRef,
17-
EventEmitter,
1817
Input,
1918
NgZone,
2019
OnDestroy,
@@ -40,6 +39,7 @@ import {
4039
Subject,
4140
of,
4241
BehaviorSubject,
42+
fromEventPattern,
4343
} from 'rxjs';
4444

4545
import {
@@ -55,6 +55,7 @@ import {
5555
take,
5656
takeUntil,
5757
withLatestFrom,
58+
switchMap,
5859
} from 'rxjs/operators';
5960

6061
declare global {
@@ -102,6 +103,15 @@ interface PendingPlayerState {
102103
template: '<div #youtubeContainer></div>',
103104
})
104105
export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
106+
/** Whether we're currently rendering inside a browser. */
107+
private _isBrowser: boolean;
108+
private _youtubeContainer = new Subject<HTMLElement>();
109+
private _destroyed = new Subject<void>();
110+
private _player: Player | undefined;
111+
private _existingApiReadyCallback: (() => void) | undefined;
112+
private _pendingPlayerState: PendingPlayerState | undefined;
113+
private _playerChanges = new BehaviorSubject<Player | undefined>(undefined);
114+
105115
/** YouTube Video ID to view */
106116
@Input()
107117
get videoId(): string | undefined { return this._videoId.value; }
@@ -155,25 +165,28 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
155165
@Input() showBeforeIframeApiLoads: boolean | undefined;
156166

157167
/** Outputs are direct proxies from the player itself. */
158-
@Output() ready = new EventEmitter<YT.PlayerEvent>();
159-
@Output() stateChange = new EventEmitter<YT.OnStateChangeEvent>();
160-
@Output() error = new EventEmitter<YT.OnErrorEvent>();
161-
@Output() apiChange = new EventEmitter<YT.PlayerEvent>();
162-
@Output() playbackQualityChange = new EventEmitter<YT.OnPlaybackQualityChangeEvent>();
163-
@Output() playbackRateChange = new EventEmitter<YT.OnPlaybackRateChangeEvent>();
168+
@Output() ready: Observable<YT.PlayerEvent> =
169+
this._getLazyEmitter<YT.PlayerEvent>('onReady');
170+
171+
@Output() stateChange: Observable<YT.OnStateChangeEvent> =
172+
this._getLazyEmitter<YT.OnStateChangeEvent>('onStateChange');
173+
174+
@Output() error: Observable<YT.OnErrorEvent> =
175+
this._getLazyEmitter<YT.OnErrorEvent>('onError');
176+
177+
@Output() apiChange: Observable<YT.PlayerEvent> =
178+
this._getLazyEmitter<YT.PlayerEvent>('onApiChange');
179+
180+
@Output() playbackQualityChange: Observable<YT.OnPlaybackQualityChangeEvent> =
181+
this._getLazyEmitter<YT.OnPlaybackQualityChangeEvent>('onPlaybackQualityChange');
182+
183+
@Output() playbackRateChange: Observable<YT.OnPlaybackRateChangeEvent> =
184+
this._getLazyEmitter<YT.OnPlaybackRateChangeEvent>('onPlaybackRateChange');
164185

165186
/** The element that will be replaced by the iframe. */
166187
@ViewChild('youtubeContainer')
167188
youtubeContainer: ElementRef<HTMLElement>;
168189

169-
/** Whether we're currently rendering inside a browser. */
170-
private _isBrowser: boolean;
171-
private _youtubeContainer = new Subject<HTMLElement>();
172-
private _destroyed = new Subject<void>();
173-
private _player: Player | undefined;
174-
private _existingApiReadyCallback: (() => void) | undefined;
175-
private _pendingPlayerState: PendingPlayerState | undefined;
176-
177190
constructor(
178191
private _ngZone: NgZone,
179192
/**
@@ -221,7 +234,6 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
221234
iframeApiAvailableObs,
222235
this._width,
223236
this._height,
224-
this.createEventsBoundInZone(),
225237
this._ngZone
226238
).pipe(waitUntilReady(player => {
227239
// Destroy the player if loading was aborted so that we don't end up leaking memory.
@@ -233,6 +245,7 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
233245
// Set up side effects to bind inputs to the player.
234246
playerObs.subscribe(player => {
235247
this._player = player;
248+
this._playerChanges.next(player);
236249

237250
if (player && this._pendingPlayerState) {
238251
this._initializePlayer(player, this._pendingPlayerState);
@@ -257,25 +270,12 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
257270
(playerObs as ConnectableObservable<Player>).connect();
258271
}
259272

273+
/**
274+
* @deprecated No longer being used. To be removed.
275+
* @breaking-change 11.0.0
276+
*/
260277
createEventsBoundInZone(): YT.Events {
261-
const output: YT.Events = {};
262-
const events = new Map<keyof YT.Events, EventEmitter<any>>([
263-
['onReady', this.ready],
264-
['onStateChange', this.stateChange],
265-
['onPlaybackQualityChange', this.playbackQualityChange],
266-
['onPlaybackRateChange', this.playbackRateChange],
267-
['onError', this.error],
268-
['onApiChange', this.apiChange]
269-
]);
270-
271-
events.forEach((emitter, name) => {
272-
// Since these events all trigger change detection, only bind them if something is subscribed.
273-
if (emitter.observers.length) {
274-
output[name] = this._runInZone(event => emitter.emit(event));
275-
}
276-
});
277-
278-
return output;
278+
return {};
279279
}
280280

281281
ngAfterViewInit() {
@@ -288,6 +288,7 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
288288
window.onYouTubeIframeAPIReady = this._existingApiReadyCallback;
289289
}
290290

291+
this._playerChanges.complete();
291292
this._videoId.complete();
292293
this._height.complete();
293294
this._width.complete();
@@ -299,13 +300,6 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
299300
this._destroyed.complete();
300301
}
301302

302-
private _runInZone<T extends (...args: any[]) => void>(callback: T):
303-
(...args: Parameters<T>) => void {
304-
return (...args: Parameters<T>) => this._ngZone.run(() => callback(...args));
305-
}
306-
307-
/** Proxied methods. */
308-
309303
/** See https://developers.google.com/youtube/iframe_api_reference#playVideo */
310304
playVideo() {
311305
if (this._player) {
@@ -518,6 +512,37 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
518512
player.seekTo(seek.seconds, seek.allowSeekAhead);
519513
}
520514
}
515+
516+
/** Gets an observable that adds an event listener to the player when a user subscribes to it. */
517+
private _getLazyEmitter<T extends YT.PlayerEvent>(name: keyof YT.Events): Observable<T> {
518+
// Start with the stream of players. This way the events will be transferred
519+
// over to the new player if it gets swapped out under-the-hood.
520+
return this._playerChanges.pipe(
521+
// Switch to the bound event. `switchMap` ensures that the old event is removed when the
522+
// player is changed. If there's no player, return an observable that never emits.
523+
switchMap(player => {
524+
return player ? fromEventPattern<T>((listener: (event: T) => void) => {
525+
player.addEventListener(name, listener);
526+
}, (listener: (event: T) => void) => {
527+
// The API seems to throw when we try to unbind from a destroyed player and it doesn't
528+
// expose whether the player has been destroyed so we have to wrap it in a try/catch to
529+
// prevent the entire stream from erroring out.
530+
try {
531+
player.removeEventListener(name, listener);
532+
} catch {}
533+
}) : observableOf<T>();
534+
}),
535+
// By default we run all the API interactions outside the zone
536+
// so we have to bring the events back in manually when they emit.
537+
(source: Observable<T>) => new Observable<T>(observer => source.subscribe({
538+
next: value => this._ngZone.run(() => observer.next(value)),
539+
error: error => observer.error(error),
540+
complete: () => observer.complete()
541+
})),
542+
// Ensures that everything is cleared out on destroy.
543+
takeUntil(this._destroyed)
544+
);
545+
}
521546
}
522547

523548
/** Listens to changes to the given width and height and sets it on the player. */
@@ -593,15 +618,14 @@ function createPlayerObservable(
593618
iframeApiAvailableObs: Observable<boolean>,
594619
widthObs: Observable<number>,
595620
heightObs: Observable<number>,
596-
events: YT.Events,
597621
ngZone: NgZone
598622
): Observable<UninitializedPlayer | undefined> {
599623

600624
const playerOptions =
601625
videoIdObs
602626
.pipe(
603627
withLatestFrom(combineLatest([widthObs, heightObs])),
604-
map(([videoId, [width, height]]) => videoId ? ({videoId, width, height, events}) : undefined),
628+
map(([videoId, [width, height]]) => videoId ? ({videoId, width, height}) : undefined),
605629
);
606630

607631
return combineLatest([youtubeContainer, playerOptions, of(ngZone)])

tools/public_api_guard/youtube-player/youtube-player.d.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
export declare class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
2-
apiChange: EventEmitter<YT.PlayerEvent>;
2+
apiChange: Observable<YT.PlayerEvent>;
33
set endSeconds(endSeconds: number | undefined);
4-
error: EventEmitter<YT.OnErrorEvent>;
4+
error: Observable<YT.OnErrorEvent>;
55
get height(): number | undefined;
66
set height(height: number | undefined);
7-
playbackQualityChange: EventEmitter<YT.OnPlaybackQualityChangeEvent>;
8-
playbackRateChange: EventEmitter<YT.OnPlaybackRateChangeEvent>;
9-
ready: EventEmitter<YT.PlayerEvent>;
7+
playbackQualityChange: Observable<YT.OnPlaybackQualityChangeEvent>;
8+
playbackRateChange: Observable<YT.OnPlaybackRateChangeEvent>;
9+
ready: Observable<YT.PlayerEvent>;
1010
showBeforeIframeApiLoads: boolean | undefined;
1111
set startSeconds(startSeconds: number | undefined);
12-
stateChange: EventEmitter<YT.OnStateChangeEvent>;
12+
stateChange: Observable<YT.OnStateChangeEvent>;
1313
set suggestedQuality(suggestedQuality: YT.SuggestedVideoQuality | undefined);
1414
get videoId(): string | undefined;
1515
set videoId(videoId: string | undefined);

0 commit comments

Comments
 (0)