search

benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
185 stars 28 forks source link

Don't remove listeners if media element isn't created #342

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

In some cases, media element isn't created, e.g. old browsers don't have MutationObserver and so creating it throws an error. If you use an ErrorBoundary you can catch this and render a fallback, but then you also get additional errors when the PlayerContextProvider unmounts, because it tries to remove event listeners of a null media elem.

benwiley4000 commented 5 years ago

I'd like a bit more information about the use case. What browser are you trying to support / what sort of fallback are you using? (We could support slightly older browsers if we fallback to the webkit prefix: https://caniuse.com/#feat=mutationobserver). My initial reaction is that supporting a case where the video element doesn't exist is odd since I don't understand how Cassette could work at all without it.

danielr18 commented 5 years ago

I don't think it should support browsers without MutationObserver, but the idea is the user can have an ErrorBoundary to render a simple fallback component with an error message. In order to do that, Cassette has to be unmounted, and that causes additional errors, because event listeners can't be removed of a null element.

This PR makes sure of not running that code if media doesn't exist, which is how it should be, there aren't any events attached.

benwiley4000 commented 5 years ago

Alright, this makes sense to me. Thanks! Please leave comments at each place explaining the Error boundary use case - otherwise someone else reading the code might be confused why the media element isn't guaranteed to exist.

danielr18 commented 5 years ago

Done, feel free to edit the comment if you'd like

benwiley4000 commented 5 years ago

Thanks! Sorry for the delay - I was going to merge immediately but my computer's charger stopped working all of a sudden. It's now released in v2.0.0-alpha.18.