-
-
Notifications
You must be signed in to change notification settings - Fork 34
fix: 🐛 Check if a path(basePath) exists on Android, RN >= 0.80 #117
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
base: main
Are you sure you want to change the base?
Conversation
Because RN >= 0.80 has extracted the example out of the core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should do a check before, if the previous path are not available, then you use the new path. Otherwise it will be compatible only with the newer version of RN.
But the check just above was already working with RN versions below 0.80, wasn’t it? |
@tconns your fix seems wrong it should be
|
i also agree with @ildfreelancer |
@dougg0k I sent you an invite on GitHub, can you check? |
Thanks @numandev1 but I am good as collab / contrib only, when I can. |
The current impl are weird, even though the first are being used for the project, a different path is used for the example project. Then it's checked again for the actual node_modules. It seem to contain duplication in there. https://github.com/numandev1/react-native-keys/blob/main/android/build.gradle#L85 Perhaps it should be.
Removing
Since the example project should have no ties to the project. But it's own android folder. |
Just as an interesting note: when I approved this PR #102, I made a mistake and applied my own patch to the exact same lines that are changed in this current PR. Now that I've updated rn-keys to 0.7.12 and removed my patch, it's broken again. So it turns out that this diff is actually correct and functional:
And the previous PR (#102) should be rolled back. |
functional are not necessarily correct, but just functional. that is functional because before that, it is considering the supposed example project, but it should have no need. |
What is probably missing here due to the example project is another check of the same, done in my PR for newer RN versions. https://github.com/numandev1/react-native-keys/blob/main/android/build.gradle#L96 |
Summary
Because RN >= 0.80 has extracted the example out of the core
Changelog
Test Plan