search

doublesymmetry / react-native-track-player

A fully fledged audio module created for music apps. Provides audio playback, external media controls, background mode and more!
https://rntp.dev/
Apache License 2.0
3.2k stars 984 forks source link

refactor: `usePlaybackState` without initial `undefined` #2167

Closed kirillzyusko closed 9 months ago

kirillzyusko commented 9 months ago

The idea is to store the data in a singleton and consume it as an initial value for state.

The problem with undefined as an initial value comes in the scenario, when music is playing and hook gets mounted (you open a player screen) -> in this case first state value will be undefined and for play/pause/spinner button most likely it'll be shown as "loading" and only after that will be shown as "playing" (such blink is annoying and this PR attempts to resolve it).

Let me know if you have any concerns with such approach 👀

kirillzyusko commented 9 months ago

@dcvz may I kindly ask you to review this PR? 👀

puckey commented 9 months ago

I am not sure automatically installing a listener like is a good idea, since you are unnecessarily using resources if it goes unused - i.e. when you don't use the hook. If you want to cache this value across unmounts and remounts, another approach would be to lift your usage of the hook up to the root of you app and pass the value down as props / context.. In a any case you would always need to start out with undefined instead of State.NONE

dcvz commented 9 months ago

Agreed @kirillzyusko it seems the issue could be fixed by just setting up state in your app differently. Agreeing with puckey above, I think we shouldn't do unnecessary setups for APIs that may not be used at all.

kirillzyusko commented 9 months ago

Ah, okay @dcvz and @puckey

I understand your point of view and it makes sense to me 👍 Feel free to close this PR then 😊