Race condition causes player to play even when playing prop is false

[MattSkala]

Current Behavior

I am using ReactPlayer to play HLS audio using the following configuration:

<ReactPlayer
        url={song.webPlaybackPath}
        config={{ file: { forceHLS: true } }}
        playing={playing}
/>

The issue occurs when playing prop toggles too quickly between false - true - false values. The player usually keeps playing even when the final value of playing is false.

Expected Behavior

The player should always stop playing when playing prop is false.

Environment

• Browser: Electron 8
• Operating system: macOS

Other Information

After checking the source code, I think the issue is in the logic present in Player.componentDidUpdate:

react-player/src/Player.js

Line 60 in c9421f7

if (prevProps.playing && !playing && this.isPlaying) {

The issue occurs when the following sequence of events occurs:

1. playing prop is changed to true => player.play() is invoked
2. playing prop is change to false. this.isPlaying is still false, so player.pause() is not invoked
3. The underlying Player.onPlay callback is received, which is handled by handlePlay, which sets this.isPlaying = true, but player.pause() from the previous prop change never gets invoked.

What is the reason for the existence of this.isPlaying? According to the comment, it should "prevent bugs", but in this case it causes a bug. :|

[cookpete]

Historically there have been a few bugs when calling play() on players that are already playing, or pause() on paused players. We also don’t want to fire a false onPlay or onPause callback and potentially run a bunch of app listeners for no reason.

If an app doesn’t update its playing state correctly on play/pause events, then the internal logic can get really messy. isPlaying is a way for the player to know if it's playing or not regardless of the playing prop being passed in.

This does expose the logic to problems like this, but my suggestion would be to simply try and avoid toggling playing too fast, either with a debounce or throttle, or some other logic.

Some interesting discussion from a similar problem here: #85