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

[RCTImageLoader] Make C++ requirement opt-in #27731

Conversation

alloy
Copy link
Contributor

@alloy alloy commented Jan 10, 2020

Note that this is the back ported version of #27730, so the description is the same.

Summary

When building as a framework these headers get automatically added to the framework umbrella header for React-Core. Instead of converting all the React sources to ObjC++ files and still forcing external users that build native source (and link against a framework build) to also compile as ObjC++, this makes the attribution related methods that were added in fdcdca4 opt-in to ObjC++ builds.

This is also the reason for the current failure of the CI test_ios_frameworks run.

Changelog

I’m unsure if this change really warrants an entry in the CHANGELOG, as it’s more of an amendment of the (afaik) unreleased change.

[iOS] [Fixed] - Make framework builds work again by making RCTImageLoader C++ requirement opt-in

Test Plan

I tested static and dynamic (framework) builds and ran the test suite.

This change should make the test_ios_frameworks CI run build again, but it may still fail overall as in my local testing one of the tests leads to a segfault (which I will try to address separately).

When building as a framework these headers get automatically added to
the framework umbrella header for React-Core. Instead of converting all
the React sources to ObjC++ files and still forcing external users that
build native source (and link against a framework build) to also
compile as ObjC++, this makes the attribution related methods opt-in to
ObjC++ builds.
@alloy alloy requested a review from fkgozali January 10, 2020 12:38
@alloy alloy requested a review from shergin as a code owner January 10, 2020 12:38
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 10, 2020
@pull-bot
Copy link

Warnings
⚠️

❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

Generated by 🚫 dangerJS against 81bf0b7

@alloy
Copy link
Contributor Author

alloy commented Jan 10, 2020

Going to move this to react-native-community/releases#145

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants