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

@DoNotStripAny annotation is missing in the Proguard Setup for React Native Android #33008

Closed
cortinico opened this issue Jan 31, 2022 · 3 comments
Assignees
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.

Comments

@cortinico
Copy link
Contributor

Description

I'm opening this public issue if someone from the community wants to pick this up.

We do have several classes that are using the @DoNotStripAny annotation:

$ rg DoNotStripAny

ReactAndroid/src/main/java/com/facebook/react/uimanager/ComponentNameResolver.java
10:import com.facebook.proguard.annotations.DoNotStripAny;
12:@DoNotStripAny

ReactAndroid/src/main/java/com/facebook/react/uimanager/ComponentNameResolverManager.java
12:import com.facebook.proguard.annotations.DoNotStripAny;
16:@DoNotStripAny

ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
37:import com.facebook.proguard.annotations.DoNotStripAny;
97:@DoNotStripAny

ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
10:import com.facebook.proguard.annotations.DoNotStripAny;
19:@DoNotStripAny

This annotation is actually not included in the Proguard file that we ship alongside ReactAndroid: https://github.com/facebook/react-native/blob/main/ReactAndroid/proguard-rules.pro

It's configuration it's only available in this file which is consumed by Buck:
https://github.com/facebook/react-native/blob/main/ReactAndroid/src/main/java/com/facebook/proguard/annotations/proguard_annotations.pro

We should adapt the former proguard file to include the Proguard config for @DoNotStripAny.
Ideally also doing a run with Proguard and verify that the classes are also not stripped would be great.

Happy to review a PR addressing this, which should be also easy to jump on.

Version

0.67.1

Output of npx react-native info

N/A

Steps to reproduce

N/A

Snack, code example, screenshot, or link to a repository

N/A

@cortinico cortinico added Needs: Triage 🔍 Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. and removed Needs: Triage 🔍 labels Jan 31, 2022
@lunaleaps
Copy link
Contributor

@cortinico Does this qualify as a blocking issue for 0.68?

@cortinico
Copy link
Contributor Author

@cortinico Does this qualify as a blocking issue for 0.68?

Nope is not a blocker. Users on Android + New Architecture (which is opt-in) + Building Release + Using Proguard will face problems. They can still provide extra rules locally to circumvent the problem. Still something we want to offer (so quite important, thought not a hard blocker).

@ShikaSD
Copy link
Contributor

ShikaSD commented Feb 2, 2022

Fixed by 48318b1

@ShikaSD ShikaSD closed this as completed Feb 2, 2022
@facebook facebook locked as resolved and limited conversation to collaborators Feb 2, 2023
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants