react-native-track-player icon indicating copy to clipboard operation
react-native-track-player copied to clipboard

Fix broken events in Bridgeless mode (React Native New Architecture)

Open robik opened this issue 1 year ago • 17 comments

This PR fixes broken event emitting in React Native new architecture.

EDIT: This is breaking change, and mostly acts as a preview. I will try to come up with a solution for it if time allows.

This is a partial solution, and requires:

  • #2290 for this package
  • https://github.com/facebook/react-native/pull/46480 for react native (note you'll need to build from source)

robik avatar Sep 13 '24 10:09 robik

I have test this patch with new arch react native 0.75.3. But i got this kotlin error : MusicService.kt error: unresolved reference: reactContext

pigeonmal avatar Sep 15 '24 21:09 pigeonmal

I have test this patch with new arch react native 0.75.3. But i got this kotlin error : MusicService.kt error: unresolved reference: reactContext

This probably is caused by the fact that the old RN is used (RN patch was not applied or it was not built from source)

robik avatar Sep 16 '24 10:09 robik

Thanks for your reply, and how can i fix that ?

My steps :

  • Install 0.75.3 RN
  • install rntp ( "react-native-track-player": "git+https://[email protected]/behenate/react-native-track-player.git#@behenate/improve-bridgeless-support )
  • Add patchs of here
  • install and build I checked node_modules source code and patched are working

pigeonmal avatar Sep 16 '24 11:09 pigeonmal

see https://github.com/lovegaoshi/RNTPExampleNewArch/tree/fix ur either missing https://github.com/lovegaoshi/RNTPExampleNewArch/blob/830428365aac927e2f3c984fe414469b6d4c88af/android/settings.gradle#L9 or https://github.com/lovegaoshi/RNTPExampleNewArch/blob/fix/patches/react-native%2B0.75.3.patch

lovegaoshi avatar Sep 16 '24 19:09 lovegaoshi

Yes the only file i not have the same as you is settings.gradle (i have the default one). If add your modifications, i have an other error :

`Could not determine the dependencies of task ':react-native:packages:react-native:ReactAndroid:hermes-engine:configureBuildForHermes'.

Could not create task ':react-native:packages:react-native:ReactAndroid:hermes-engine:installCMake'. Could not find sdkmanager executable.`

pigeonmal avatar Sep 17 '24 17:09 pigeonmal

so u r having compilation issues. if u dont add that in, ie. https://reactnative.dev/contributing/how-to-build-from-source, ur not compiling RN and not applying any changes. this is outside the scope of this PR and u should seek help in the general RN community. im not going to pretend compiling RN is easy. its a PITA and im too sane now to troubleshoot someone elses env setup.

lovegaoshi avatar Sep 17 '24 17:09 lovegaoshi

Now that 0.76 has been released, should we pick up again on this PR?

Ref. https://github.com/doublesymmetry/react-native-track-player/pull/2290

Noitham avatar Oct 29 '24 13:10 Noitham

with 0.76.1 released these prs can finally be tested. for the lazy folks my example is now upgraded to newarch + 0.76.1. im still waiting for expo 52 to actually test newarch myself, but with possibly many of my used libs breaking, im not spending a whole lot of effort here.

lovegaoshi avatar Oct 29 '24 19:10 lovegaoshi

Now that 0.76 has been released, should we pick up again on this PR?

Ref. #2290

in new architecture the app crashes while setupPlayer so will this PR covered that issue as well?

syed-shoaib-ali avatar Oct 31 '24 18:10 syed-shoaib-ali

like i said my example app runs. so presumably yes.

On Thu, Oct 31, 2024, 11:59 AM Syed Shoaib Ali @.***> wrote:

Now that 0.76 has been released, should we pick up again on this PR?

Ref. #2290 https://github.com/doublesymmetry/react-native-track-player/pull/2290

in new architecture the app crashes while setupPlayer so will this PR covered that issue as well?

— Reply to this email directly, view it on GitHub https://github.com/doublesymmetry/react-native-track-player/pull/2370#issuecomment-2450614958, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZMOVVTUY5Q3D56O6YSFBJ3Z6J4XXAVCNFSM6AAAAABOFEDBAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJQGYYTIOJVHA . You are receiving this because you commented.Message ID: @.*** com>

lovegaoshi avatar Oct 31 '24 19:10 lovegaoshi

Merge that for god sake!

ducatti007 avatar Dec 12 '24 11:12 ducatti007

I patched this along w/ the required other changes (from 1st comment), and I'm seeing NPEs on reactContext in prod. Does anyone have any guesses as to why this might happen?

Some context:

  • it's pretty rare (1/300 sessions)
  • it seems to happen across all devices manufacturers / all OS versions / foreground & background states
  • it happens after users have already been interacting w/ TrackPlayer for a bit (1-5 minutes, so in my case that means dozens of short tracks played)
  • I'm using the new arch

My thoughts:

  • the old impl was anticipating context potentially being undefined/null (ie reactNativeHost.reactInstanceManager.currentReactContext?.)
  • easiest thing seems to be to just add optional operator (?), but I don't really know anything about what's going on in this file, so maybe that's naive?
Fatal Exception: java.lang.NullPointerException
Attempt to invoke virtual method 'void com.facebook.react.bridge.ReactContext.emitDeviceEvent(java.lang.String, java.lang.Object)' on a null object reference
com.doublesymmetry.trackplayer.service.MusicService.emit (MusicService.kt:744)

mihaibulic2 avatar Feb 06 '25 14:02 mihaibulic2

thats an interesting report. i actually use the null safe call myself. I do see from time to time that event emitting becomes unresponsive until the Activity is brought back to the foreground, which explains the reactContext loss and could have the same root as ur report. Though I then deemed the newarch to be unusable and didnt pursue further.

Id start from headlessJsTaskService's getReactContext and find out the status of the reactHost and reactContext (u do need to build ur own RN for this). With the difficulty to reproduce this I wouldnt have high hope though.

lovegaoshi avatar Feb 06 '25 17:02 lovegaoshi

@mihaibulic2 apologies for the ping, did you perhaps see with new arch android the app becomes unresponsive (from being frozen for several seconds, to a whilte screen) when the app is left running at the background for a long period of time (5+ minutes)? I found the longer I left it, the longer it froze/shows a whit screen of death instead. I'm suspecting its my app's issue and about to test the example app, but also curious to see if this is somethign cmmon.

also i dumped more time to testing my android new arch app since react 19 is likely to dump old arch for good, and i'm actually yet to see the js events not being responsive. every event emitted via the notification panel works, despite the view actually crashes out.

lovegaoshi avatar Feb 20 '25 23:02 lovegaoshi

I haven't gotten any reports of that (so far I've had ~100k user sessions in prod across Android and iOS on the new arch)

Message ID: @.*** com>

mihaibulic2 avatar Feb 21 '25 07:02 mihaibulic2

thx for the reply! i indeed could not reproduce with the example app. back to the grind.

I narrowed down to anything using useProgress for anyone interested. My current hypothesis and experiments point to useProgress updating states even when the app is in background, causing a lag in state updates when the app is resumed in the new architecture.


a simple if app is background check in useProgress will solve this issue

lovegaoshi avatar Feb 24 '25 21:02 lovegaoshi

Thanks, I did my local changes and it's now working on my one

SachinRajyaguru avatar Mar 24 '25 21:03 SachinRajyaguru

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jun 23 '25 02:06 github-actions[bot]

Closing this in favor of https://github.com/doublesymmetry/react-native-track-player/pull/2478 which migrates to the [email protected] and migrates the library to a turbomodule library.

jspizziri avatar Jun 25 '25 13:06 jspizziri