-
Notifications
You must be signed in to change notification settings - Fork 776
Crash fix #861
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
base: master
Are you sure you want to change the base?
Crash fix #861
Conversation
This PR is the only way I could get react-native-sound to work with React Native 0.79. I installed it with I also tried the latest released version Thank you very much for this PR. I hope it can get merged and this library can be put in a working state again. |
Hello @EmilJunker & @dhairyasenjaliya Could you please tell us more about why it's not working with the latest master branch? Would be amazing to be able to merge with this branch and make @zmxv update it on npm! 💪 |
@RomualdPercereau Thank you for your interest in getting this issue resolved. Here is an overview of the errors I'm seeing: When using the latest released version installed via
When using the latest tagged version installed via Android build log
When using the latest version from master installed via
I'm using React Native 0.79.1 with the new architechture disabled. |
Hello @dhairyasenjaliya and @EmilJunker, If anyone can update this PR to make it works with the new architecture, it would be wonderful! |
The fact that this PR is 19 commits behind the current master is precisely the reason why it's working. This PR is basically version 0.11.2 of the library with an added fix for the The actual problem is that some of those 19 commits seem to have introduced new errors (see my comment above #861 (comment)). Hence, the priority should be to fix these errors and then release a new and working version of this library on npm. |
Thank you very much for the feedback @EmilJunker. It is indeed strange that this error is occurring on the master branch. The issue with rolling back these 19 commits is that we would lose the compatibility added as part of issue #850 (@SolankiYogesh) for the new architecture. By addressing the new errors introduced in these fix, we could establish a clean baseline and then proceed to publish it on npm. |
Hey @RomualdPercereau its quick fix I will create a new PR for this particular pr we hade some old changes which is not needed for this bug |
Any update on this? |
I just realized the reason why I get the import bell from './../assets/sounds/bell.mp3';
bellSound = new Sound(bell); instead of passing the file path: bellSound = new Sound('./../assets/sounds/bell.mp3'); Therefore, this line of code in However, even if I change my code and pass the file path as |
adding the patch helped me solve the problem react-native-sound+0.11.2.patch
|
Thank you @DK-AC, if you could push a PR, would be amazing! |
No description provided.