Skip to content

Conversation

@hoangvvo
Copy link
Collaborator

@hoangvvo hoangvvo commented Oct 2, 2021

I updated the submodule to the latest commits.

I also updated the build.gradle following the latest setup from https://github.com/callstack/react-native-builder-bob (recommended by RN docs)

Fixed #161

@hoangvvo hoangvvo marked this pull request as ready for review October 3, 2021 00:18
@hoangvvo
Copy link
Collaborator Author

hoangvvo commented Oct 3, 2021

I did a round of testing in my RN app (v0.65.1, android sdk 30) and everything seemed ok.

@cjam
Copy link
Owner

cjam commented Oct 3, 2021

Awesome thanks for the contribution! I'll try to get this merged and released ASAP

@cjam cjam merged commit 3c5e339 into cjam:master Oct 5, 2021
@hoangvvo
Copy link
Collaborator Author

@cjam Sorry for pinging but is there any chance for an NPM release? Thanks!

@cjam
Copy link
Owner

cjam commented Oct 10, 2021

Yes @hoangvvo definitely. I wanted to get it done last week but unfortunately broke my leg at soccer. So I'm hoping to get it done this week.

@hoangvvo
Copy link
Collaborator Author

Yes @hoangvvo definitely. I wanted to get it done last week but unfortunately broke my leg at soccer. So I'm hoping to get it done this week.

Thanks for the confirmation. Hope your leg gets better soon!

@cjam
Copy link
Owner

cjam commented Oct 13, 2021

hey @hoangvvo. I wanted to go through and make sure I could run the examples and everything still, but didn't get a chance. So I just released a pre-release version that 0.3.11-0 that you can install via react-native-spotify-remote@next. Let me know if it fixes your problem or if you have any issues.

@hoangvvo
Copy link
Collaborator Author

hey @hoangvvo. I wanted to go through and make sure I could run the examples and everything still, but didn't get a chance. So I just released a pre-release version that 0.3.11-0 that you can install via react-native-spotify-remote@next. Let me know if it fixes your problem or if you have any issues.

I tried the release but apparently something is wrong with the submodule init. (maybe I broke something 😢). In the NPM package, running the command git submodule update --init --recursive (via postinstall yarn submodules hook) no longer add the files into android and ios.

~/dev/mobile/node_modules/react-native-spotify-remote$ yarn submodules
yarn run v1.22.10
$ git submodule update --init --recursive
Done in 0.06s.

However trying with my branch hoangvvo:bump-android-sdk, it can still:

~/dev/react-native-spotify-remote$ yarn submodules
yarn run v1.22.10
$ rm -rf ios/external/* && rm -rf android/external/* && git submodule update --init --recursive
Submodule path 'android/external/SpotifySDK': checked out 'cfd6b68a47440a7db8afac1983d92d324a1c0015'
Submodule path 'ios/external/SpotifySDK': checked out 'f9a7d53967de5ea633845c2387b7fc8f90b96265'
Done in 0.18s.

I will investigate this a little bit and let you know. Sorry for the broken release! :(

@cjam
Copy link
Owner

cjam commented Oct 16, 2021

Ah all good it happens. Glad I did it as a pre-release. Keep me posted.

@hoangvvo
Copy link
Collaborator Author

Ah all good it happens. Glad I did it as a pre-release. Keep me posted.

I think the simplest fix to this is to remove the postinstall hook. After all it is not good to be there since it assumes the user of the package use yarn (to call yarn submodules) and on a Linux machine (since it uses "rm" command inside).

The published tarball already contains the submodule (https://unpkg.com/browse/react-native-spotify-remote@0.3.11-0/) so I don't think we should try to run "rm -rf ios/external/* && rm -rf android/external/* && git submodule update --init --recursive" after the user install the package.

@hoangvvo hoangvvo mentioned this pull request Oct 16, 2021
@hoangvvo
Copy link
Collaborator Author

So I tried to freshly clone your repo and running git submodule status shows the following:

~/dev/react-native-spotify-remote-cjam$ git submodule status
 cf06a97e2a707fadf1f70e7e570145bad71476af android/external/SpotifySDK (v0.6.2-appremote_v1.1.0-auth-55-gcf06a97)
 cad6ba74d5160bafa71ae9ea8d4e060bb0bf595c ios/external/SpotifySDK (v1.0.1-7-gcad6ba7)

which is incorrect. For example the commit on android/external/SpotifySDK should have been cfd6b68a47440a7db8afac1983d92d324a1c0015 like on my branch:

~/dev/react-native-spotify-remote$ git checkout bump-android-sdk && git submodule status
Already on 'bump-android-sdk'
Your branch is up to date with 'origin/bump-android-sdk'.
 cfd6b68a47440a7db8afac1983d92d324a1c0015 android/external/SpotifySDK (v0.6.2-appremote_v1.1.0-auth-80-gcfd6b68)
 f9a7d53967de5ea633845c2387b7fc8f90b96265 ios/external/SpotifySDK (v1.0.1-12-gf9a7d53)

@hoangvvo
Copy link
Collaborator Author

hoangvvo commented Oct 16, 2021

@cjam Okay, so I was looking at the wrong spot but apparently it turns out this commit fffa45e from you accidentally undo my change for some reason, which seem to be the issue.

You can manually reapply my change a push to master with the below:

git submodule update --remote --merge
git add .
git commit -m "Update submodules"
git push

Feel free to do it whenever you are available. I have already published a fork to use in the meantime so no rush!

@cjam
Copy link
Owner

cjam commented Oct 16, 2021

Gotcha. Will look into it soon. Thanks for investigating. 👍

@cjam
Copy link
Owner

cjam commented Oct 17, 2021

@hoangvvo I've pushed a new pre-release which should have the updated versions. Also updated some docs and added some badges to the readme for the SDK versions, https://www.npmjs.com/package/react-native-spotify-remote/v/0.3.11-1

You can try it out using react-native-spotify-remote@next

@hoangvvo
Copy link
Collaborator Author

hoangvvo commented Oct 18, 2021

@hoangvvo I've pushed a new pre-release which should have the updated versions. Also updated some docs and added some badges to the readme for the SDK versions, https://www.npmjs.com/package/react-native-spotify-remote/v/0.3.11-1

You can try it out using react-native-spotify-remote@next

Awesome! I will test it out and let you know. Love the additional touches on the README.

@hoangvvo
Copy link
Collaborator Author

hoangvvo commented Nov 3, 2021

The NPM hook that triggers git submodules cmd still causes the external git modules failing to be pulled unfortunately. Only after patching with this PR #164 that it can work normally in my case.

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.

Does not work when targeting Android SDK v30 (React Native 0.65+)

2 participants