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): 🌟 add support for React Native 0.73 #675

Merged
merged 2 commits into from Jun 29, 2023

Conversation

peterlazar1993
Copy link
Contributor

Overview

Starting from React Native v0.73 , all libraries will need to be updated with namespace due to the upgrade to AGP 8
react-native-community/discussions-and-proposals#671

OS Implemented
iOS
Android

Test Plan

I tested the changes on RN 0.71.11 and everything works!

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.

Hi there! Thanks for posting this as a suggestion, part of it is a strict requirement for rn73 yes, but part of it also specifically cuts off support for users on rn < 70, see this: react-native-community/discussions-and-proposals#671 (comment)

I propose to follow that comment - I'm going to pull this locally and leave AndroidManifest.xml alone so that rn<70 works, but leave the build.gradle change so >=73 works

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.

now it is a forwards and backwards compatible change

Yes, users on 71+ will see a warning they can ignore, but that's better than a breaking change

@mikehardy mikehardy merged commit 224fdbb into react-native-netinfo:master Jun 29, 2023
2 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 29, 2023
# [9.4.0](v9.3.11...v9.4.0) (2023-06-29)

### Features

* **android:** add support for React Native 0.73 ([#675](#675)) ([224fdbb](224fdbb))
@matt-oakes
Copy link
Collaborator

🎉 This PR is included in version 9.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@@ -29,6 +29,7 @@ def getExtOrIntegerDefault(name) {
apply plugin: 'com.android.library'

android {
namespace = "com.reactnativecommunity.netinfo"
Copy link
Contributor

Choose a reason for hiding this comment

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

Random comment, you could have also defined something like this:

def agpVersion = com.android.Version.ANDROID_GRADLE_PLUGIN_VERSION
// Check AGP version for backward compatibility reasons
if (agpVersion.tokenize('.')[0].toInteger() >= 7) {
   namespace = "com.reactnativecommunity.netinfo"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

PRs happily accepted :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, one moment :)

Copy link
Contributor

Choose a reason for hiding this comment

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

on second thought, adding the namespace to build.gradle doesn't create any problems for people that don't care about it (so conditionally adding it is...not interesting?) - it's the inability to remove it from AndroidManifest without triggering a breaking change that's the issue, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I understood that this would cause build issues on Gradle 6 or older. I have seen a number of packages adopting the above change instead of just adding "namespace" to the gradle file.

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

Successfully merging this pull request may close these issues.

None yet

4 participants