Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(guides): add an Expo guide #2296

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Conversation

dctalbot
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jspizziri jspizziri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dctalbot , thanks for taking the time to write this up. I think this could be a great addition! Left some feedback for you.

docs/docs/guides/with-expo.md Outdated Show resolved Hide resolved
docs/docs/guides/with-expo.md Outdated Show resolved Hide resolved
docs/docs/guides/with-expo.md Show resolved Hide resolved
@isilher
Copy link

isilher commented Apr 22, 2024

Hey, just chiming in here because we have been using this library in our Expo based app for a while. We needed to figure some things out ourselves so I definitely think adding documentation will be very helpful to others!

I have some comments and suggestions:

  1. Adding this dependency (or any native dependency that is not included in Expo) will not work in the default "Expo Go" shell. To still be able to develop and/or test build with the track player functionality you will need to implement a strategy they call "development builds". (See: https://docs.expo.dev/develop/development-builds/create-a-build/). I think this will be the most common gotcha that people run into.
  2. I don't think it's necessary to change the entry point just to include the audio service. We made it work simply by adding it to our index.js
import { PlaybackService } from './src/services/PlaybackService'

// ...

registerRootComponent(App)

// We need to register a playback service right after registering the main component of the app 
TrackPlayer.registerPlaybackService(() => PlaybackService)
  1. If you wish to be able to play audio in the background, you need to add iOS permission exactly like you do with expo-av: https://docs.expo.dev/versions/latest/sdk/audio/#playing-or-recording-audio-in-background

@jspizziri
Copy link
Collaborator

@isilher , thanks for the great feedback, I think that's all quite helpful.

@dctalbot , would you be able to confirm item #2 and incorporate @isilher's other suggestions?

If you're not able to get to all of this LMK.

@dctalbot
Copy link
Contributor Author

Points 1 and 3 are good 👍🏻

@isilher On point 2, I'm not sure I'm seeing the full picture in that example. The default entry point already calls registerRootComponent(App). Are you calling it a second time in this index.js file?

Regardless, you're probably right. It probably is not strictly necessary to call TrackPlayer.registerPlaybackService() immediately after registerRootComponent(App) in the same entrypoint file. However, if an expo user wants to follow the RNTP documentation as written, they will need a custom entry point.

I will follow-up with another commit soon.

BTW, I have an example open source project here - https://github.com/dctalbot/spinitron-mobile-app
Maybe that would be a useful reference to include. Is there an example projects page in the docs?

@isilher
Copy link

isilher commented Apr 23, 2024

@dctalbot you are correct! I was checking our git history for including this library and assumed the pre-existing index.js in our project to be conventional (at least for development builds). However, we already included that as part of our Detox setup earlier, exactly like you specified (changing the main entry in package.json).

@jamsch
Copy link

jamsch commented Apr 24, 2024

Another note: on iOS, you'll need to update app.json / app.config.js to include the following for background audio playback:

{
    ios: {
      infoPlist: {
        UIBackgroundModes: ["audio"],
      },
    },
}

@dctalbot dctalbot requested a review from jspizziri April 25, 2024 21:47
@andordavoti
Copy link

I had an issue on Expo web because shaka-player is a dynamic import. Might be worth mentioning in this guide. My current workaround is to create two files in a shaka-player folder in my project, index.ts (empty file), index.web.ts (has an import of shaka-player with this line "import 'shaka-player/dist/shaka-player.ui';"). This works as it forces the shaka-player import on web, but would be nice to see first party Expo web support.

@jspizziri jspizziri changed the title [docs] Add an Expo guide docs(guides): add an Expo guide Apr 29, 2024
@jspizziri
Copy link
Collaborator

@andordavoti , definitely would love for Expo web to work. I'm curious if this PR addresses your issue. Would you be able to take it for a spin and let me know? Apart from that, I'd be happy to work with you separately on Expo web support. But it's definitely something I'll need assistance on due to my ignorance surrounding Expo.

Why don't we move the conversation to the linked PR for now.

@jspizziri jspizziri enabled auto-merge (squash) April 29, 2024 12:33
@jspizziri
Copy link
Collaborator

@dctalbot , thanks a ton for your work on this.

@isilher @jamsch thank you for reviewing and providing feedback.

It's great to see the community come together like this!

@jspizziri jspizziri disabled auto-merge April 29, 2024 12:39
@jspizziri
Copy link
Collaborator

@dctalbot sorry, I have it set to auto-merge, but it looks like the docs build is failing to an invalid link. Would you be able to sort that out?

@9mzodiac
Copy link

9mzodiac commented Apr 29, 2024

@jspizziri I just moved away from Solito / NextJS to Expo router for my app today. I had gotten RNTP working perfect in Next but instantly hit the issue @andordavoti was having when I moved to Expo web. Just checked this PR and saw you talking about expo web support today. Couldn't be better timing haha, thanks @andordavoti for sharing and @jspizziri for keeping this project going!

@jspizziri jspizziri merged commit 7cb43bb into doublesymmetry:main Apr 30, 2024
5 checks passed
@ajarian
Copy link

ajarian commented Jul 25, 2024

@isilher thanks for sharing your example project–that was really helpful 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants