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

getTrackSources for undefined track #320

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

https://github.com/benwiley4000/cassette/blob/da848c2d42c471343622d25262f6ef1cc23be63e/packages/core/src/PlayerContextProvider.js#L348

I think I found another case when this can happen, we discussed it on #318. If you have a playlist with 3 tracks, and activeIndex: 2, and the playlist is changed to only have one track, it would try to access the activeIndex of the new playlist, which would be undefined and throw an error.

benwiley4000 commented 5 years ago

Good catch! Thank you.

Check out this part of static getDerivedStateFromProps:

    // the sources if we stay on the same track index
    const currentSources = getTrackSources(
      newPlaylist,
      prevState.activeTrackIndex
    );
    // non-comprehensive but probably accurate check
    if (prevSources[0].src === currentSources[0].src) {
      // our active track index already matches
      return baseNewState;
    }

I think we can wrap all of that in an if statement and if it's false then we automatically find a new activeTrackIndex afterward:

    if (newPlaylist[prevState.activeTrackIndex]) {
        // the sources if we stay on the same track index
        const currentSources = getTrackSources(
          newPlaylist,
          prevState.activeTrackIndex
        );
        // non-comprehensive but probably accurate check
        if (prevSources[0].src === currentSources[0].src) {
          // our active track index already matches
          return baseNewState;
        }
    }

I haven't tested this though so it's hard to be sure.

I will probably have time to deal with this sometime later this week, unless you want to submit a PR. 🙂

danielr18 commented 5 years ago

I'll create the PR, and test it.