Make all headers public and add #ifdef __cplusplus#1150
Make all headers public and add #ifdef __cplusplus#1150janicduplessis wants to merge 1 commit intofacebook:mainfrom
Conversation
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Hi @janicduplessis, can we revert facebook/react-native#33381 as the next step? |
|
Yes, please. But I think the change to react_native_pods.rb don’t need to be reverted. I can create a pr for this later today if it works. |
Summary: This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See facebook/react-native#33381. The most reliable fix is to include all headers as public headers, and add `#ifdef __cplusplus` to those that include c++. This is already what we do for other headers, this applies this to all headers. Tested in the YogaKitSample, and also in a react-native app. Changelog: [iOS] [Changed] - Make all Yoga headers public and add #ifdef __cplusplus X-link: facebook/yoga#1150 Reviewed By: dmitryrykun Differential Revision: D36966687 Pulled By: cortinico fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
Summary: This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See #33381. The most reliable fix is to include all headers as public headers, and add `#ifdef __cplusplus` to those that include c++. This is already what we do for other headers, this applies this to all headers. Tested in the YogaKitSample, and also in a react-native app. Changelog: [iOS] [Changed] - Make all Yoga headers public and add #ifdef __cplusplus X-link: facebook/yoga#1150 Reviewed By: dmitryrykun Differential Revision: D36966687 Pulled By: cortinico fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
|
Yea we don't want to revert the whole thing. Sounds good @Kudo you can do the follow up PR in react-native. |
|
What's the process now to get this version of Yoga with the fix to be consumed by RN? (yoga is actually missing from https://reactnative.dev/contributing/release-dependencies 😅) |
Yoga is dir-synced inside |
|
ok so we just need to have (basically) a sync commit land on master, and that one could potentially be even cherry-picked? |
|
@kelset Correct. This landed on |
|
@Kudo @brentvatne you were saying that you wanted to fix in 0.69 right for Expo next SDK version, right? |
|
the revert pr is in facebook/react-native#33973. we want to have both facebook/react-native@43f831b and facebook/react-native#33973 in 0.69. thanks for making it happens. |
|
@Kudo , sounds good - can you add those requests here? reactwg/react-native-releases#21 since they are about Yoga I feel we'd need a new RC to ensure everything goes smoothly 😅 |
|
thanks @kelset! i'll add the commits after facebook/react-native#33973 landed. |
Summary: facebook/yoga#1150 is better than the tricky #33381 and fix the build error on react-native 0.69 with swift clang module. as facebook/yoga#1150 is landed as 43f831b, i'm reverting the previous change, only leaving the necessary react_native_pods.rb change. ## Changelog [iOS] [Changed] - Better fix for yoga + swift clang module build error Pull Request resolved: #33973 Test Plan: ci passed Reviewed By: cortinico, cipolleschi Differential Revision: D36998007 Pulled By: dmitryrykun fbshipit-source-id: fa11bd950e2a1be6396f286086f4e7941ad2ff5b
Summary: This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See #33381. The most reliable fix is to include all headers as public headers, and add `#ifdef __cplusplus` to those that include c++. This is already what we do for other headers, this applies this to all headers. Tested in the YogaKitSample, and also in a react-native app. Changelog: [iOS] [Changed] - Make all Yoga headers public and add #ifdef __cplusplus X-link: facebook/yoga#1150 Reviewed By: dmitryrykun Differential Revision: D36966687 Pulled By: cortinico fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
Summary: facebook/yoga#1150 is better than the tricky #33381 and fix the build error on react-native 0.69 with swift clang module. as facebook/yoga#1150 is landed as 43f831b, i'm reverting the previous change, only leaving the necessary react_native_pods.rb change. ## Changelog [iOS] [Changed] - Better fix for yoga + swift clang module build error Pull Request resolved: #33973 Test Plan: ci passed Reviewed By: cortinico, cipolleschi Differential Revision: D36998007 Pulled By: dmitryrykun fbshipit-source-id: fa11bd950e2a1be6396f286086f4e7941ad2ff5b
Summary: This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See facebook/react-native#33381. The most reliable fix is to include all headers as public headers, and add `#ifdef __cplusplus` to those that include c++. This is already what we do for other headers, this applies this to all headers. Tested in the YogaKitSample, and also in a react-native app. Changelog: [iOS] [Changed] - Make all Yoga headers public and add #ifdef __cplusplus Pull Request resolved: facebook/yoga#1150 Reviewed By: dmitryrykun Differential Revision: D36966687 Pulled By: cortinico fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
Summary: This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See facebook/react-native#33381. The most reliable fix is to include all headers as public headers, and add `#ifdef __cplusplus` to those that include c++. This is already what we do for other headers, this applies this to all headers. Tested in the YogaKitSample, and also in a react-native app. Changelog: [iOS] [Changed] - Make all Yoga headers public and add #ifdef __cplusplus Pull Request resolved: facebook/yoga#1150 Reviewed By: dmitryrykun Differential Revision: D36966687 Pulled By: cortinico fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See facebook/react-native#33381.
The most reliable fix is to include all headers as public headers, and add
#ifdef __cplusplusto those that include c++. This is already what we do for other headers, this applies this to all headers.Tested in the YogaKitSample, and also in a react-native app.