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

feat(android)!: specify export mode on BroadcastReceivers, requires compileSdk 33+ #692

Merged
merged 1 commit into from Nov 4, 2023

Conversation

AndyG
Copy link
Contributor

@AndyG AndyG commented Nov 3, 2023

This PR introduces Android targetSdk 34 (Android 14) support.

Without this PR, devices running Android 14 will crash if the app using this dependency is using targetSdk=34.

An alternative approach would be to directly include androidx.core:core as a dependency of this project and use ContextCompat.registerReceiver instead.

More information can be found in this issue: #681

This is the smallest change that I can make and have this work, I think. Locally, in order to get this to build on my machine, I did also need to upgrade the gradle wrapper to work with newer JDKs. But I don't think those changes are necessary for this PR. We'll see what CI says :P

@AndyG AndyG mentioned this pull request Nov 3, 2023
Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Fantastic - a request for your thoughts but leaning towards release as is as noted in comment

@mikehardy mikehardy added awaiting info pending merge A PR that will be merged shortly, waiting for CI or final comment labels Nov 3, 2023
@mikehardy mikehardy changed the title Support android 14 by explicitly not exporting BroadcastReceivers feat(android)!: specify export mode on BroadcastReceivers, requires compileSdk 33+ Nov 3, 2023
@mikehardy
Copy link
Contributor

Looks like we just ran into the issue where CI doesn't actually even attempt to compile the java which is "awesome", so CI has nothing really to say about it. Will pull locally though and give it a run

@AndyG
Copy link
Contributor Author

AndyG commented Nov 3, 2023

let me know what your local results are! to be candid my react native experience is limited and I want a second opinion 🙇

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

works fine - but the irony is that the code modification was required for a code path that is only exercised if Build.VERSION_CODE <= 23 😆

So the new registration style will never actually be triggered, as far as I know? On modern emulators (API34) it isn't used, at least, but for emulator API23 I do see the receiver registration, but it uses the old code path.

I suppose for Fire devices it could trigger but I don't have one of those to test... Either way, looks good and things are working

FWIW - I wasn't able to test this easily at all until I (finally) merged #582 - big improvement for the example app, everything actually works out of the box now for people looking to contribute

@mikehardy mikehardy merged commit a5864cc into react-native-netinfo:master Nov 4, 2023
2 checks passed
@mikehardy mikehardy removed the pending merge A PR that will be merged shortly, waiting for CI or final comment label Nov 4, 2023
github-actions bot pushed a commit that referenced this pull request Nov 4, 2023
# [11.0.0](v10.0.0...v11.0.0) (2023-11-04)

* feat(android)!: specify export mode on BroadcastReceivers, requires compileSdk 33+ (#692) ([a5864cc](a5864cc)), closes [#692](#692)

### BREAKING CHANGES

* - compileSdk minimum is now 33
- windows SDK minimum bumped to 10.0.17763.0 for current react-native-windows
@matt-oakes
Copy link
Collaborator

🎉 This PR is included in version 11.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

3 participants