-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: supporting browfields implementations with a differing react root #1174
base: main
Are you sure you want to change the base?
Conversation
@Debens Thanks for this! What kinds of testing have you done to verify this works correctly? |
@SaeedZhiany You have any feedback on this? |
Apologies for the delayed response. The issue here is that the current implementation relies on the node modules being in a specific location. A folder down from the android app. While this is fine for a greenfields application made from something like |
Related to testing I haven't been able to get much done outside of a personal use case. If you give me some time I can throw a tester app together. But it might be a few days as I have other commitments currently |
@Debens You could try making a PR to https://github.com/msand/react-native-svg-e2e |
I understand now, I didn't experience integrating react-native with an existing android project.
So after installing the dependencies using
it seems this is the most common way for the integration process because it is documented. So the current gradle implementation seems OK and no need for handling complicated cases for two reasons: These changes
So I suggest avoiding adding these changes. @Debens Am I figure out the problem correctly at all? |
Yes you seems to have it! Sadly our integration into our existing app was not the smoothest, and while me agreed it was the right decision the native apps wanted it to be invasive as possible. We are only a single feature within a much larger app and it seems unreasonable to reorganise an already proven structure for a single important feature. It is not necessary to use the |
@msand thanks for pointing me to the repo, here is a PR. |
The ci looks green with the react-native v0.60 setup now at least. Seems this should work out fine. |
Summary
With the change found here react-native-svg lost support for an implementation where any node dependencies are not located specifically at the root of the project.
This PR borrows the gradle implementation from react-native-webview that does a better job a resolving the react root of a project and warns when this cannot be resolved.
See: react-native-webview/react-native-webview@24ec4f7