destroying before removing element causes error with youtube

[ashleahhill]

This may not be an "issue" but any guidance would be helpful.

Expected behaviour

Using Plyr.js destroy method in Vue.js component lifecycle hooks should destroy Plyr instance before component is destroyed.

Actual behaviour

My app uses Plyr for two types of sources: HLS and YouTube.

I load Plyr inside of a Vue.js component and destroy it by calling instance.destroy() when the component is unloaded in its beforeDestroy lifecycle hook. This is how I used Plyr in v2 and it worked fine. Please note that Vue.js lifecycle hooks are synchronous and they are not interested in changing that at the moment.

Now, however, YouTube instances can't be destroyed without an error:

vue.runtime.esm.js?ff9b:1683 Error: The YouTube player is not attached to the DOM.
    at Y.h.B (www-widgetapi.js:99)
    at X (www-widgetapi.js:91)
    at Y.(anonymous function).getCurrentTime.cb.(anonymous function) [as stopVideo] (https://s.ytimg.com/yts/jsbin/www-widgetapi-vflQSvpsZ/www-widgetapi.js:109:124)
    at HTMLDivElement.player.media.stop (youtube.js?6edf:285)
    at plyr_Plyr.stop (plyr.js?1bb6:403)
    at plyr_Plyr.destroy (plyr.js?1bb6:1078)
    at VueComponent.beforeDestroy (video-player.vue?53d5:335)
    at callHook (vue.runtime.esm.js?ff9b:2813)
    at VueComponent.Vue.$destroy (vue.runtime.esm.js?ff9b:2598)
    at destroy (vue.runtime.esm.js?ff9b:4027)

A breakpoint in the YouTube plugin's stop method shows that the player DOM has been unloaded by Vue long before we get to this stop call.

The HLS.js plyr appears to destroy fine.

Guarding the call to this.media.stop inside destroy kept YouTube to check if the plyr was playing didn't prevent the error. It also caused the timeupdate event to continue firing on the HLS version.

    if (this.playing) {
            this.stop();
    }

At first I thought this must be due to the destroy method being asynchronous in v3, but I'm not sure if that's the case for all source types.

Not sure where to go next. There's an async hook farther up in the application structure that I may be able to wire plyr into, but it would be a shame to not have the ability to wrap the plugin in a reusable component like I could before.

I am not using [https://github.com/redxtech/vue-plyr] but as of writing it looks like they have a similar issue: https://github.com/redxtech/vue-plyr/issues/9

Environment

• Browser: Chrome
• Version: 66
• Operating System: Mac OS
• Version: 10.13.4

Steps to reproduce

• Make a plyr instance in a Vue.js component with YouTube as a source
• Destroy the instance in the beforeDestroy hook of the component
• Make parent component(s) for testing
• Put the plyr component inside of the parents
• Destroy the parent component.
• See error.

[friday]

Youtube and Vimeo events are async.

destroy() restores the element to how it was before Plyr transformed it (removes event listeners and Plyr elements). It also stops the media playback. With youtube you may not have any original element (see the part about "progressive enhancement": https://github.com/sampotts/plyr/#youtube-embed), in which case the error may happen (not sure), or like in your case, Vue may be removing the element before stopVideo is executed in which case this may also happen.

I'd say it's a bug on our side. We should either not stop from destroy, add an optional argument to control it, or handle errors in stop due to being called from destroy.

You may not need destroy though, if you're going to remove the plyr container element, and have no other references to Plyr. Event listeners and such gets destroyed along with their elements, and garbage collection kicks in when there are no more references to something.

[friday]

I renamed the issue, since it's not unique to Vue.js.

[julkue]

It seems that I have the same issue.

In a React app inside componentDidMount I'm initializing Plyr successfully. However, upon page change I need to destroy it. Therefore I'm calling .destroy() in componentWillUnmount. Even though I'm getting an error after destroy was called by React:

Seems like Plyr is still executing something after I have called destroy. I'm using a normal HTML5 video, no Vimeo/YouTube. In the source could I could figure out that there's an option to pass a callback, this doesn't seem to be documented. So I'm assuming that something async is going on? That's a problem for me since I can not have any async code in componentWillUnmount. It just has to be synchronously that React can remove elements without problems.

[friday]

It's not the same culprit, but at least both are caused by destroy().

[FlipgridMartin]

@julmot I think your issue is more related to #853

[julkue]

@friday @FlipgridMartin Maybe it's also related to all of them, I don't know. However, do you know a workaround for integrating Plyr into a Next.js/react app with client-side routing? Currently for example when Plyr is initialized and Next.js does hot reloading then I'm getting an error by React that he can't remove elements.

[friday]

@julmot How is that related to this issue? This is not a place for support anyway. See https://github.com/sampotts/plyr/wiki/FAQ (last question)

[julkue]

@friday ??? Seriously.

How is that related to this issue?

Look at the second sentence in this ticket:

Using Plyr.js destroy method in Vue.js component lifecycle hooks should destroy Plyr instance before component is destroyed.

Replace "Vue.js" with "React" and there you go. That's my problem.

This is not a place for support anyway.

This is not support. This is a bug report. Player .destroy() doesn't seem to be synchronously, that's why I think we're experiencing issues whenever client-side routing (and therefore replacement of existing elements) is the case. We're destroying Plyr and afterwards the framework (Vue.js/React) replaces old children with new children. But since Plyr still seems to depend on them, some error happens.

[friday]

@julmot You've been told three time now that you're being off topic: https://github.com/sampotts/plyr/issues/1001#issuecomment-394711506 https://github.com/sampotts/plyr/issues/1001#issuecomment-394770938 https://github.com/sampotts/plyr/issues/1001#issuecomment-395360193

In addition to that you're now being rude.

For other than background context, it doesn't matter for Plyr which type of frameworks and libraries you are using, unless the issue is happening specifically with one of those frameworks or libraries, and can't be reproduced without it. We already know why this issue is happening, and it was there to read before your first comment.

Please don't comment on this issue again. It's disrespectful to the author and subscribers who are here because they are interested in the issue.

[julkue]

@friday

You've been told three time now that you're being off topic:

Actually, as my comment above shows, this is not off topic. Rather I think it is directly related. And even if I don't think it would have been necessary, I've already created follow-up issue #1010.

you're now being rude.

I'm annoyed, that's all. Instead of helping or answering with helpful information or trying to understand the issue or the relationship to the original issue, you're saying me that it's off topic, even if it isn't.

For other than background context, it doesn't matter for Plyr which type of frameworks and libraries you are using, unless the issue is happening specifically with one of those frameworks or libraries, and can't be reproduced without it.

Which is the case.

We already know why this issue is happening, and it was there to read before your first comment.

Not in the context of Next.js/React, and I wanted to make sure that a fix also applies for these.

Please don't comment on this issue again. It's disrespectful to the author and subscribers who are here because they are interested in the issue.

I'm here to get a solution for this bug. If you're not interested in offering solutions / fixing bugs or getting feedback from users, then close the issue functionality (you can do so by going into the repository settings => Options => Uncheck "Issues") or let others reply to issues. Finally, let me say that I have the same feeling about you: Your comments are absolutely disrespectful and rude.

[friday]

This is not a conversion about your hurt feelings.

Temporary locking the conversation from further comments. Sorry everyone else for the inconvenience.

[qikkeronline]

Any idea when this will be published? Also running into this issue as we speak :-)

[friday]

@qikkeronline This type of comment is discouraged in the FAQ and contribution guidelines. Anyone has the right to fork Plyr, fix this issue and submit a PR. No one has, and afaik no one is working on it.

I think (at least hope) my previous comment is helpful if someone wants to have a try. Speaking of "try", the simples solution is probably adding a try/catch somewhere in the code or wrapping your destroy() calls.

I'm no longer helping out with Plyr development so I'm going to unsubscribe (so @mention me if you need to reach me).

Update: You can dislike this all you want. Doesn't change anything.

[Blucreation]

Can confirm wrapping destroy calls with a try/catch works.

[Lusinia]

Hello. I still have this issue. I tried try/catch approach but see empty black screen instead of youtube video(after destroy component and init in once again). Maybe you have some other workaround to fix this bug?

[Lusinia]

Ok I found solution. I had to show player only after view init (I dont know why it happened but youtube works only like that. Vimeo and html5 work in both cases).

[oalexdoda]

@sampotts , any idea why this shows as a warning when using import Plyr from "plyr-react"; ?

The YouTube player is not attached to the DOM. API calls should be made after the onReady event. See more: https://developers.google.com/youtube/iframe_api_reference#Events