(fix): clear video src to prevent old video from continue loading #1360
Conversation
ptgamr
changed the title
|
Thanks for fixing it. we had the same issue that the player keeps loading old url. |
|
@novyQ cheers, now just waiting for someone to merge it, hopefully |
noticed the CI error. is that why the PR is not being merged? |
src/players/FilePlayer.js
Outdated
| @@ -38,11 +38,13 @@ export default class FilePlayer extends Component { | |||
| this.props.url !== prevProps.url && | |||
| !isMediaStream(this.props.url) | |||
| ) { | |||
| this.player.src = '' | |||
There was a problem hiding this comment.
I don’t think this is needed? This runs when the url has changed and the new one is not a MediaStream, we don’t necessarily want to clear the src here, especially if the new url is also a file path?
There was a problem hiding this comment.
@cookpete I have tested it without this line and it doesn't really fix the problem (old video still load when I change the URL)
I don't see any downside of clearing src here, as it'll be set again in the next render.
There was a problem hiding this comment.
I don't see any downside of clearing src here, as it'll be set again in the next render.
Will it? This is in componentDidUpdate which fires after everything has finished rendering. When does the "next render" happen?
There was a problem hiding this comment.
@cookpete Oh you're right. Sorry, I was blind and somehow thought it is componentWillUpdate... Now, I don't even know why it's still working when I have this.
I undo this change, and looks like reseting the src inside componentWillUnmount is enough in my case (as I actually unmount the videoPlayer)
Thanks for looking into this!
|
Published in |
Fix #1359
Inspired by prior-art from videogular:
https://github.com/videogular/videogular/blob/master/app/scripts/com/2fdevs/videogular/controllers/vg-controller.js#L698