Improve docs

[puckey]

I would like to work some more on the docs. I thought a good step for this would be to ask a few questions about things that are unclear to me after reading the docs:

API.md -> Starting off

You also need to register a playback service right after you have registered the main component of your app

Is this required? Why does it need to be done after registering the main component?

In this and other examples module.exports is used. Since the import x from 'x' syntax is being used, this could become export default

API.md -> Player information

After a media file has been loaded, you can get information about it (such as retrieving the state)

How can you tell if a track was loaded?

API.md -> Receiving Events -> React components

Works as long as the UI is mounted Should only be used to update the UI state

What is meant above? What works? Why should it only be used to update UI state?

API.md -> Playback Service

The first question I would have here is: what is a playback service?

It is unclear to me what the specifics are of this.. Are only events inside a function passed to registerPlaybackService called when the app is in the background? Others are ignored?

TrackPlayer.registerPlaybackService(() => require('./service.js'));

The example above uses require inside a function – is this necessary? It leads me to believe it would not be able to read anything outside of its own scope – is this the case?

It 'works as long as the player runs' – what does 'works' mean here? What happens when it no longer works? What is the player here? The background service?

API.md -> Notes

Due to the notification not being swipable, we highly recommend you to have a stop button in the notification. The button should destroy() the player.

Is this android only?

[puckey]

Note answers to the questions above were given by @Guichaguri here:

API.md -> Starting off Yes, this is required. I expect someone to be trying it out while they follow the tutorial, and if you try to setup the player without registering the service, Android crashes with No task registered for key TrackPlayer.

Feel free to update module.exports to export default, I don't think there's anything against that.

It feels odd somehow that android needs to crash if you do not do this.. Another way would be to supply something to setupPlayer:

await TrackPlayer.setupPlayer({
    backgroundService: () => require('./service')
});

Then if you do not supply it, we could add a default set of listeners internally. But this would only work if the order as you describe in the API docs is not as strict as it sounds: "You also need to register a playback service right after you have registered the main component of your app"

It makes sense to put it there, because setupPlayer is also only meant to be called once..

One more question I have: in the API docs it states:

It is recommended to set the options right before using any other function (other than setupPlayer)

Why is this recommended?

[puckey]

Yes, the require is necessary. You can see it is wrapped inside a function, which means it will only require (and load the service code into memory) when it actually needs to load. It follows the same lifecycle as Headless JS.

@Guichaguri Here are a few examples where they are not putting require in between:

https://github.com/transistorsoft/react-native-background-geolocation/wiki/Android-Headless-Mode

https://medium.com/@essamtarik2000/introduction-to-react-natives-headless-js-90e87dee0ab4

I am not sure the memory considerations of passing a require should trump the questions the use of require raises (like 'is this code being evauated in a seperate environment?)

Another question: isn't the async function in the service.js example resolving straight away, causing React Native to allow itself to go into paused mode?

From the docs:

You can do anything in your task such as network requests, timers and so on, as long as it doesn't touch UI. Once your task completes (i.e. the promise is resolved), React Native will go into "paused" mode (unless there are other tasks running, or there is a foreground app).

[puckey]

I worked on the getting started docs: https://github.com/react-native-kit/react-native-track-player/pull/506/commits/2e404a0bc2ab988250b8a505d87a4aa17f87aa36:

• Split up the events section into two and linked the remote events to the Playback Service
• Simplified some of the code sections which were acting like extensive docs – while that is what Documentation.md is for.
• Incorporated answers to my questions

[Guichaguri]

API.md -> Starting off It feels odd somehow that android needs to crash if you do not do this.

Yes, but this is how Headless JS works, unfortunately. In order to run a Headless JS service, you need to register a JS task for it, since the whole music service is based on that, it won't run until you register it.

We can't use setupPlayer for that. When React Native needs to initialize a headless JS, it ONLY runs the index.js file, looks into the list of tasks registered and finds the TrackPlayer one, so we can't add the registration to a context-based code. That's why you also have to register the main component of your app, React Native may or may not use it when index.js runs.

It is recommended to set the options right before using any other function (other than setupPlayer) Why is this recommended?

Mostly because you want consistency in your app, and you want set the options before using anything that might use them. It's not a requirement.

Yes, the require is necessary. You can see it is wrapped inside a function, which means it will only require (and load the service code into memory) when it actually needs to load. It follows the same lifecycle as Headless JS.

Here are a few examples where they are not putting require in between [...]

Sorry, I phrased that badly. We used to do that (the old TrackPlayer.registerEventHandler(...)), but we figured that we are initializing unnecessary code that won't be used. In order to improve the initialization performance and follow RN's design closely, we decided to add the require back.

isn't the async function in the service.js example resolving straight away, causing React Native to allow itself to go into paused mode?

Yes, it is, but no, it doesn't go into paused mode. Since we wanted to use the service as a place to add event handlers, we decided to keep the JS engine running as long as the playback runs.

I'm looking into changing the playback service once again, we mostly had success with this new design, but it still has some flaws. (See #433)

[puckey]

API.md -> Starting off It feels odd somehow that android needs to crash if you do not do this.

Yes, but this is how Headless JS works, unfortunately. In order to run a Headless JS service, you need to register a JS task for it, since the whole music service is based on that, it won't run until you register it.

We can't use setupPlayer for that. When React Native needs to initialize a headless JS, it ONLY runs the index.js file, looks into the list of tasks registered and finds the TrackPlayer one, so we can't add the registration to a context-based code. That's why you also have to register the main component of your app, React Native may or may not use it when index.js runs.

This is very useful information – I see how best incorporate it in the docs.

It is recommended to set the options right before using any other function (other than setupPlayer) Why is this recommended?

Mostly because you want consistency in your app, and you want set the options before using anything that might use them. It's not a requirement.

Okay, I think I will remove this from the docs. I think it is self explanatory that if you want certain options to be set, you should set them directly.. I can't image someone calling setOptions in some kind of onClick handler within a component for example.

Yes, the require is necessary. You can see it is wrapped inside a function, which means it will only require (and load the service code into memory) when it actually needs to load. It follows the same lifecycle as Headless JS. Here are a few examples where they are not putting require in between [...]

Sorry, I phrased that badly. We used to do that (the old TrackPlayer.registerEventHandler(...)), but we figured that we are initializing unnecessary code that won't be used. In order to improve the initialization performance and follow RN's design closely, we decided to add the require back.

In combination with your explanation above, I understand now why this would be the case. Would the promise version of import also work here? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Dynamic_Imports

isn't the async function in the service.js example resolving straight away, causing React Native to allow itself to go into paused mode?

Yes, it is, but no, it doesn't go into paused mode. Since we wanted to use the service as a place to add event handlers, we decided to keep the JS engine running as long as the playback runs.

I'm looking into changing the playback service once again, we mostly had success with this new design, but it still has some flaws. (See #433)

[Guichaguri]

In combination with your explanation above, I understand now why this would be the case. Would the promise version of import also work here?

No, and I'm pretty sure there is no benefit in using a dynamic import in this case.

[puckey]

Would it be an idea to include code in the example app for the remote-duck event?

On Android it is essential for an app to behave well between other apps and there are quite few details on how it might best be implemented in the docs: https://react-native-kit.github.io/react-native-track-player/documentation/#remote-duck

service.js file:

/**
 * This is the code that will run tied to the player.
 *
 * The code here might keep running in the background.
 *
 * You should put everything here that should be tied to the playback but not the UI
 * such as processing media buttons or analytics
 */

import TrackPlayer from 'react-native-track-player';

module.exports = async function() {

  TrackPlayer.addEventListener('remote-play', () => {
    TrackPlayer.play()
  })

  TrackPlayer.addEventListener('remote-pause', () => {
    TrackPlayer.pause()
  });

  TrackPlayer.addEventListener('remote-next', () => {
    TrackPlayer.skipToNext()
  });

  TrackPlayer.addEventListener('remote-previous', () => {
    TrackPlayer.skipToPrevious()
  });

  TrackPlayer.addEventListener('remote-stop', () => {
    TrackPlayer.destroy()
  });

  let playingBeforeDuck;
  let volumeBeforeDuck;
  const DUCKED_VOLUME = 0.2;
  TrackPlayer.addEventListener(
    'remote-duck',
    async ({ paused, permanent, ducking }) => {
      if (permanent) {
        TrackPlayer.stop();
        return;
      }

      if (paused) {
        const playerState = await TrackPlayer.getState();
        playingBeforeDuck = playerState === TrackPlayer.STATE_PLAYING;
        TrackPlayer.pause();
        return;
      }

      if (ducking) {
        const volume = await TrackPlayer.getVolume();
        if (volume > DUCKED_VOLUME) {
          volumeBeforeDuck = volume;
          TrackPlayer.setVolume(DUCKED_VOLUME);
        }
        return;
      }

      if (playingBeforeDuck) {
        TrackPlayer.play();
      }

      const playerVolume = await TrackPlayer.getVolume();
      if (volumeBeforeDuck > playerVolume) {
        TrackPlayer.setVolume(volumeBeforeDuck || 1);
      }

      volumeBeforeDuck = playingBeforeDuck = null;
    }
  );
};

Speaking of the service.js file. At the top is written "The code here might keep running in the background.". Is this a warning somehow? What is meant by the word 'might'?

[Guichaguri]

Would it be an idea to include code in the example app for the remote-duck event?

Absolutely!

Speaking of the service.js file. At the top is written "The code here might keep running in the background.". Is this a warning somehow? What is meant by the word 'might'?

An app developer might only test the module when the app is running, merging UI-related code to the service, and they will not realize that the service can also run in the background. This is kind of a warning to be careful with the code you run in the service, as the UI may or may not be running as well.

[puckey]

I just wanted to come back to the note about the android notification:

Because you can not stop the player service by swiping the Android notification, we highly recommend you to have a stop button in the notification. The button should destroy() the player.

While the stop button destroys the notification, the app stays running in the background.. When coming back to the app, you would then need to tell somehow that the app was destroyed and set it up again, correct? Recreating queue etc etc to match the app state. Should this be added to the docs somehow?