fix(android)!: share function not working on Android 14

[mlazari]

Overview

This fixes #1463 ("One of RECEIVER_EXPORTED or RECEIVER_NOT_EXPORTED should be specified when a receiver isn't being registered exclusively for system broadcasts")

On Android 14 a security exception is thrown when not specifying the RECEIVER_EXPORTED / RECEIVER_NOT_EXPORTED flag: https://stackoverflow.com/a/77276774/3337236

BREAKING CHANGE: compileSdk minimum is now 33

Test Plan

Verified Share.open(...) works on Android 13 and Android 14 with this change. Without it, it crashes the app on Android 14.

[MateusAndrade]

We might need to update the Android build with CircleCI, as it's failing: https://app.circleci.com/pipelines/github/react-native-share/react-native-share/1219/workflows/36b92415-a309-4874-8186-90c50433527a/jobs/4585. Should we publish this as a breaking-change?

[mikehardy]

These are difficult as they require API33 compileSdk in order to have the required symbols and that is definitely a breaking change.

However, as of August 31, 2023 all updates to the platforms react-native successfully targets already had a minimum targetSdk of 33, so while it is breaking, everyone should already be doing it unless their app needs updates anyway --> https://developer.android.com/google/play/requirements/target-sdk

So, definitely a breaking change but it's mandatory already, I'm issuing these in other repos for the same reason and my note is like "BREAKING CHANGE: bump compileSdk to 33 minimum, aligns with existing Play Store requirements"

That said, you've got a bigger problem here. You're not just using the new symbols sometimes, this change uses them everywhere even on non-API34 devices, which will crash I think?

Here's how we handled it in react-native-netinfo - avoiding an androidx.core dependency and making our own little compat function (like androidx.core does...) that uses the correct API depending on platform availability and target API needs:

https://github.com/react-native-netinfo/react-native-netinfo/pull/692/files#diff-1ca9eff43d42bc2b19e7e75f5a24027ed616359b14adee89a2f25dacc36e7e4cR41-R49

[mikehardy]

I believe this will crash on non-API33/34 devices because it does not check if the API is available on the specific device, and I don't believe the API was available until 34?

I may not have time to review this again but I've linked to a solution that appears to be working in react-native-netinfo

[mlazari]

It looks like a breaking change indeed, since it requires API level 33 minimum (which react-native updated to in version 0.71.0) and the example project uses API level 30 (which is why the CircleCI job fails).

BTW I just tested this change on a device with Android 10 and works just fine.

[mikehardy]

BTW I just tested this change on a device with Android 10 and works just fine

That's surprising! I see why though, It wasn't API33 where it was first added but Android 8 / API26 --> https://developer.android.com/reference/android/content/Context#registerReceiver(android.content.BroadcastReceiver,%20android.content.IntentFilter,%20int)

It will break on older devices that are still within minSdkVersion range of this module

[mlazari]

Ok, I'll add the same verification as in react-native-netinfo.

[mikehardy]

@MateusAndrade for what it's worth, I just merged the switch proposed by @tido64 from a custom example app to react-native-test-app in react-native-netinfo and it was nice.

I built on that success and switched react-native-apple-authentication to react-native-test-app using the guide here https://github.com/microsoft/react-native-test-app/wiki/Migrate-an-Existing-Test-App and it was about the same time as it took me to update an example app before (maybe 30 minutes?). That gave me access to modern react-native, modern Java (this will require JDK 17 also...) and modern gradle/gradle plugin (those need to be up to date for compileSdk 34...) And now it should be easy to manage in the future...

[MateusAndrade]

@MateusAndrade for what it's worth, I just merged the switch proposed by @tido64 from a custom example app to react-native-test-app in react-native-netinfo and it was nice.

I built on that success and switched react-native-apple-authentication to react-native-test-app using the guide here https://github.com/microsoft/react-native-test-app/wiki/Migrate-an-Existing-Test-App and it was about the same time as it took me to update an example app before (maybe 30 minutes?). That gave me access to modern react-native, modern Java (this will require JDK 17 also...) and modern gradle/gradle plugin (those need to be up to date for compileSdk 34...) And now it should be easy to manage in the future...

Niceee, it would be nice to do the same for rn-share. I will try to do it, but this week might be difficult for me. Tks for sharing! 🙉

[mikehardy]

Wow! @mlazari you're working through the example app update process :-) - I was curious so I pulled it and checked it, on yarn start:android I see this when I check it now 🤔

C:\Users\micro\work\react-random\react-native-share\node_modules\flow-parser\flow_parser.js:818
throw a}function
^

UnsupportedTypeAnnotationParserError: Module NativeRNShare: TypeScript type annotation 'TSObjectKeyword' is unsupported in NativeModule specs.

[mlazari]

@mikehardy I see it, I was getting 2 errors on yarn lint so I replaced "Object" to "object" per the suggestion, but didn't observe it broke the build.

mihai@Mihais-Mac-mini react-native-share % yarn lint        
yarn run v1.22.19
$ eslint "src/**/*.{js,ts,tsx}" "codegenSpec/**/*.{js,ts,tsx}" --max-warnings=0

/Users/mihai/Desktop/Repos/react-native-share/codegenSpec/NativeRNShare.ts
  29:19  error  Don't use `Object` as a type. The `Object` type actually means "any non-nullish value", so it is marginally better than `unknown`.
- If you want a type meaning "any object", you probably want `object` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead  @typescript-eslint/ban-types
  30:26  error  Don't use `Object` as a type. The `Object` type actually means "any non-nullish value", so it is marginally better than `unknown`.
- If you want a type meaning "any object", you probably want `object` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead  @typescript-eslint/ban-types

✖ 2 problems (2 errors, 0 warnings)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[mlazari]

I changed it back from "object" to "Object" and turned off the @typescript-eslint/ban-types rule for now.

The example builds seem to work fine for me, just need to figure out the CircleCI errors.

[mikehardy]

I would either try to type it well (whatever seems right ?) or // @ts-ignore that specific line with a note and maybe a related issue peeled off from it so it doesn't hang up this PR. Getting stuck on a typing thing that worked before and only triggers lint now because of tightened restrictions seems like losing sight of the forest by looking too closely at a tree :)

My personal opinion anyway

[mlazari]

I think this is ready for review now.

I managed to fix the CircleCI build by using the cimg/android image instead of the old reactnativecommunity/react-native-android:5.1 image version used by the linux_android executor in the react-native CircleCI orb in which the react-native/android_build runs. Also for iOS I had to replace the react-native/pod_install command with a bundle exec pod install command because that was not using the cocoapods and other dependency versions from example/Gemfile and it was causing issues.

[MateusAndrade]

I think this is ready for review now.

I managed to fix the CircleCI build by using the cimg/android image instead of the old reactnativecommunity/react-native-android:5.1 image version used by the linux_android executor in the react-native CircleCI orb in which the react-native/android_build runs. Also for iOS I had to replace the react-native/pod_install command with a bundle exec pod install command because that was not using the cocoapods and other dependency versions from example/Gemfile and it was causing issues.

WOW! Nice work @mlazari , thanks! 🚀 🚀 🚀

[mlazari]

Sorry, I had to do one more change to fix yarn start:ios.

[MateusAndrade]

Should we publish this as a breaking-change @mikehardy , @mlazari ?

[mlazari]

@MateusAndrade I think yes, since the compileSdk minimum is now 33 with these changes (this is the Android API Level in which RECEIVER_EXPORTED was added).

react-native-netinfo also published it as a breaking change: https://github.com/react-native-netinfo/react-native-netinfo/releases/tag/v11.0.0

[mikehardy]

Yeah - this is a breaking change, even if it is one that most people should not feel as "difficult" since they should be on API33 already - the ones that have not moved to API33 will suffer through lots of infrastructure work like @mlazari just did here

And @mlazari - amazing work, powering through all the example + CI stuff to prove this out, that was a beast! Kudos

[MateusAndrade]

🎉 This PR is included in version 10.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[thuongtv-vn]

Hi All,

Version 10.0.0 is working on Debug only, i got the crash message when i test share.open function on Release mode
java.lang.RuntimeException: com.facebook.react.devsupport.JSException: JS Functions are not convertible to dynamic

Do you have any ideas?

[mlazari]

@thuongtv-vn Hi, can you share the Share.open(...) snippet you're using (the parameters you're passing to it)? I just re-tested in my app with 10.0.0 in release mode and it didn't crash. My guess is that the error you see might have something to do with some options you're passing to it. Not sure how the changes in this PR could cause that.

[thuongtv-vn]

Hi @mikehardy

My share.open verify simple that share a text:

Share.open({
            message: selectedText
          }).catch(() => {
            // do not handle error
          });

My react-native version:
"react-native": "0.71.13"

[mikehardy]

Why am I tagged? please do not tag random people for unrelated issues, thank you

[mlazari]

@thuongtv-vn Can you try to hardcode a string there to see if it still crashes? Something like:

Share.open({
            message: 'hardcoded text'
          }).catch(() => {
            // do not handle error
          });

Just to make sure it's not happening because selectedText is a function or something else rather than a string.