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

Fix/fix duplicate c++ symbols jsi #2530

Merged
merged 4 commits into from
Oct 13, 2021
Merged

Conversation

piaskowyk
Copy link
Member

@piaskowyk piaskowyk commented Oct 12, 2021

Description

By compiling jsi.cpp, we will have duplicate C++ symbols in the user's app because jsi.cpp is compiled multiple times. (once for RN, once for REA, and maybe also for other third party libs)

This PR fixes this by re-using the shared/dynamic C++ library libjsi.so.

Based on: #2488
Authors: @mrousavy @piaskowyk

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

🖤

@mrousavy
Copy link
Contributor

btw the build.gradle + CMakeLists is a bit messy already, I think we can do a few cleanups here and there to reduce it's complexity

@piaskowyk piaskowyk merged commit a48b24f into master Oct 13, 2021
@piaskowyk piaskowyk deleted the fix/fix-duplicate-c++-symbols-jsi branch October 13, 2021 09:10
@radex
Copy link
Contributor

radex commented Oct 18, 2021

Awesome, thanks for this work! I will base an improvement to WatermelonDB on this

@mrousavy
Copy link
Contributor

@radex 💪

osdnk pushed a commit to osdnk/react-native-reanimated that referenced this pull request Oct 18, 2021
## Description

By compiling `jsi.cpp`, we will have duplicate C++ symbols in the user's app because `jsi.cpp` is compiled multiple times. (once for RN, once for REA, and maybe also for other third party libs)

This PR fixes this by re-using the shared/dynamic C++ library `libjsi.so`.

Based on: software-mansion#2488
Authors: @mrousavy @piaskowyk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants