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: fixes for new arch on RN 74 #473

Merged

Conversation

WoLewicki
Copy link
Contributor

@WoLewicki WoLewicki commented Feb 22, 2024

Summary

This PR aligns the code with newest version of yoga in RN 0.74.0-rc.0. Made with huge help from @piaskowyk

Test Plan

@jacobp100
Copy link
Collaborator

Thanks for this! What does this do for older versions? Will they break?

@WoLewicki
Copy link
Contributor Author

On new arch it will for sure. On paper I think those files are not compiled so it should not.

@jacobp100
Copy link
Collaborator

I think last time we published this as a pre-release, and then waited until it fully drops to publish in full

@janicduplessis

@janicduplessis
Copy link
Member

Yes, we can do pre-release, I'm fine breaking back compat for new arch still.

@WoLewicki
Copy link
Contributor Author

So let's do it please, it will help with testing it in other libs which use SAC 🚀

@janicduplessis
Copy link
Member

Looks like this will cause issues even with new arch disabled. The ci build on android fails with old arch and rn 0.73. We will need to find a way to make this backwards compatible. Maybe we can just not compile those classes with some preprocessor macros when on old arch.

@WoLewicki
Copy link
Contributor Author

Yeah, it seems not right that you are trying to compile shadow nodes on old arch.

@WoLewicki
Copy link
Contributor Author

Ok it seems that you are building new arch in both flows now:

😄

@WoLewicki
Copy link
Contributor Author

Ok old arch builds correctly now and new arch does not build since it is not compatible with RN 0.73, so it looks ready 🚀

@janicduplessis janicduplessis merged commit 8cf8333 into th3rdwave:main Feb 28, 2024
1 of 2 checks passed
@janicduplessis
Copy link
Member

Thanks! Released as 4.10.0-rc.0

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.

3 participants