Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bug(youtube-player): YT.Player is not a constructor #20598

Closed
dwolverton opened this issue Sep 17, 2020 · 6 comments · Fixed by #20616
Closed

bug(youtube-player): YT.Player is not a constructor #20598

dwolverton opened this issue Sep 17, 2020 · 6 comments · Fixed by #20616
Labels
area: youtube-player help wanted P3

Comments

@dwolverton
Copy link

@dwolverton dwolverton commented Sep 17, 2020

This is an intermittent exception due to a race condition.

The problem is in youtube-player.ts line 213. It checks if (!window.YT) but window.YT exists before window.YT.Player exists. There is an intermediate loading state where YT.loaded = 0. I believe a solution could be to change this if to if (!window.YT || !window.TY.loaded) or alternatively if (!window.YT || !window.TY.Player).

Reproduction

This Stackblitz has similar code to our app where the problem is happening, however it did not reproduce the race condition for me. https://stackblitz.com/edit/components-issue-dkswjg?file=src%2Findex.html

Steps to reproduce:

  1. Get the youtube iframe API by adding this to index.html: <script src="https://www.youtube.com/iframe_api"></script>
  2. Use the in a component that loads fairly immediately.

Expected Behavior

The player shows the video.

Actual Behavior

The player did not load. An error is logged to the console.

ERROR TypeError: YT.Player is not a constructor
at youtube-player.js:529
at ZoneDelegate.invoke (zone-evergreen.js:364)
at Zone.run (zone-evergreen.js:123)
at NgZone.runOutsideAngular (core.js:27364)
at ScanSubscriber.syncPlayerState [as accumulator] (youtube-player.js:529)
at ScanSubscriber._tryNext (scan.js:49)
at ScanSubscriber._next (scan.js:42)
at ScanSubscriber.next (Subscriber.js:49)
at MapSubscriber._next (map.js:35)
at MapSubscriber.next (Subscriber.js:49)

Environment

  • Angular: 10.0.6
  • CDK/Material: 10.2.1
  • Browser(s): Chrome
  • Operating System (e.g. Windows, macOS, Ubuntu): macOS

Possibly some commenters on #19871 were seeing this same problem.
@dwolverton dwolverton added the needs triage label Sep 17, 2020
@jelbourn jelbourn added area: youtube-player help wanted P3 and removed needs triage labels Sep 17, 2020
@jelbourn
Copy link
Member

@jelbourn jelbourn commented Sep 17, 2020

@dwolverton great report, feel free to send a PR with your proposed solution

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Sep 21, 2020 •
edited

I tried looking into this, but I wasn't able to reproduce it, even if I throttle the request for YouTube's API for long after the Angular app is done loading. I don't doubt that it's happening, but I have a hard time understanding what is going on, because the way we have the component set up is that it'll wait for window.onYouTubeIframeAPIReady to be called by YouTube itself. The only way it could happen is if YouTube decides to invoke the callback before it is actually ready which seems counter-intuitive.

As you mentioned, it can happen if YT is defined, but YT.Player doesn't exist yet, but that seems like a very narrow window of time between it loading and our code being run.

Ronll added a commit to Ronll/components that referenced this issue Sep 21, 2020
@Ronll
fix(youtube-player): YT.Player is not a constructor
06c2784 
fixes a race condition where window.YT would be truthy
but window.YT.Player would not be ready.

Fixes angular#20598
@Ronll Ronll mentioned this issue Sep 21, 2020
Merged
@dwolverton
Copy link
Author

@dwolverton dwolverton commented Sep 21, 2020 •
edited

I'll try to have another look in the next few days to see if I can find a way to get a sample that reproduces it.

@Ronll
Copy link
Contributor

@Ronll Ronll commented Sep 22, 2020

I am having the same issue, can confirm that @dwolverton 's fix works (if (!window.YT || !window.TY.Player))

made a pr in case it helps :)

@dwolverton
Copy link
Author

@dwolverton dwolverton commented Sep 28, 2020

I have been able to create a stackblitz that reproduces and illustrates the issue. https://stackblitz.com/edit/components-issue-2tnqui?file=src%2Fapp%2Fexample-component.ts The example-components.ts file has three different points where the YoutubePlayer can be created by setting this.videoId. One is early enough to work, one is late enough to work, and one is right at the time when this failure happens. Those that would work are commented out.

Also instructive is the YouTube API loading code itself... https://www.youtube.com/iframe_api

I see that @Ronll made a pull request, but let me know if you need any more help from me.

Ronll added a commit to Ronll/components that referenced this issue Sep 28, 2020
@Ronll
fix(youtube-player): YT.Player is not a constructor test
9ea1924 
added a test to prevent reggression, the test mimicks a possible state
of
the YT object when loading, when window.YT exists but window.YT.Player
does not.

Fixes angular#20598
Ronll added a commit to Ronll/components that referenced this issue Sep 28, 2020
@Ronll
fix(youtube-player): YT.Player is not a constructor test
40024bd 
added a test to prevent reggression, the test mimics a possible state
of
the YT object when loading, when window.YT exists but window.YT.Player
does not.

Fixes angular#20598
mmalerba pushed a commit that referenced this issue Sep 29, 2020
@Ronll
fix(youtube-player): YT.Player is not a constructor (#20616)
b0d0388 
* fix(youtube-player): YT.Player is not a constructor

fixes a race condition where window.YT would be truthy
but window.YT.Player would not be ready.

Fixes #20598

* fix(youtube-player): YT.Player is not a constructor test

added a test to prevent reggression, the test mimics a possible state
of
the YT object when loading, when window.YT exists but window.YT.Player
does not.

Fixes #20598
mmalerba pushed a commit that referenced this issue Sep 29, 2020
@Ronll @mmalerba
fix(youtube-player): YT.Player is not a constructor (#20616)
3393ec7 
* fix(youtube-player): YT.Player is not a constructor

fixes a race condition where window.YT would be truthy
but window.YT.Player would not be ready.

Fixes #20598

* fix(youtube-player): YT.Player is not a constructor test

added a test to prevent reggression, the test mimics a possible state
of
the YT object when loading, when window.YT exists but window.YT.Player
does not.

Fixes #20598

(cherry picked from commit b0d0388)
@angular-automatic-lock-bot
Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Oct 30, 2020

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 30, 2020
wagnermaciel pushed a commit to wagnermaciel/components that referenced this issue Jan 14, 2021
@Ronll @wagnermaciel
fix(youtube-player): YT.Player is not a constructor (angular#20616)
ea92ca0 
* fix(youtube-player): YT.Player is not a constructor

fixes a race condition where window.YT would be truthy
but window.YT.Player would not be ready.

Fixes angular#20598

* fix(youtube-player): YT.Player is not a constructor test

added a test to prevent reggression, the test mimics a possible state
of
the YT object when loading, when window.YT exists but window.YT.Player
does not.

Fixes angular#20598
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: youtube-player help wanted P3
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(youtube-player): YT.Player is not a constructor Ronll/components
4 participants
@jelbourn @dwolverton @crisbeto @Ronll