Fix - Unable to build React target after a 'pod install'

[Plo4ox]

The following error was thrown:
'string' file not found

To fix the issue, the Yoga's headers importing classes from the STL
have been excluded from the generated umbrella file via the podspec.

Motivation

Integrate React Native into an existing native app.

Test Plan

This has been tested by building a native app using the React pod.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[hramos]

Let's see if we can get someone more familiar with CocoaPods to review this.

[Titozzz]

This PR should be merged

[hramos]

@Titozzz as I said, this PR needs to be reviewed first.

[esakkik]

Any updates on the PR

[floriancargoet]

This fix works for me.

[koenpunt]

more familiar with CocoaPods

I think it's more about someone with familiarity with Yoga, who knows what the necessary headers are.

Modifying the podspec works great for me btw. 👍

[koenpunt]

But apart from that; I believe the podspecs are only used for apps who integrate React Native using CocoaPods, right? And since this fix is confirmed to be working for multiple people, I don't think it can't get any worse with this change, because without it, building doesn't work at all.

[facebook-github-bot]

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[pull-bot]

	Warnings
⚠️	📋 Release Notes - This PR appears to be missing Release Notes.

Generated by 🚫 dangerJS

[grabbou]

CC: @ide @emilsjolander @alloy as you were always active in CocoaPods related issues.

I can see the import did fail for some reason, isn't it because of a mandatory rebase you Hector requested last time?

Anyways, please let me know when this lands and whether it has to be cherry-picked. Looks potentially as a serious candidate.

[alloy]

@koenpunt Is correct that this is more an issue of knowing what Yoga headers are indeed public, but otoh these vendored podspecs are also only meant to be used by React Native, so if builds work with only exposing these headers (and potentially not exporting other headers that are also meant to be public) then that seems fine to me 👍

[mkonicek]

@LukeDurrant @Plo4ox Thank you so much! 💯Fishhook and RCTAnimation was broken for me too.

[alloy]

I don't understand CocoaPods very well, but the yoga podspec says:

source = { :git => ENV['INSTALL_YOGA_FROM_LOCATION'] || 'https://github.com/facebook/react-native.git' }

Does the podspec use the yoga sources present in node_modules (I already have those after yarn add react-native), or fetch the same sources from GitHub?

Because of the way the podspec setup has been chosen in RN’s case (that is, multiple podspecs, but none in a spec repo) there are some chicken and egg problems when linting the specs. That env variable is to make that case work, as such you can ignore it for normal usage.

In your normal usage case, the source is indeed technically fetched from GitHub, but CP doesn’t actually fetch it again, it has a local cache from where it will unpack the same source. So it really doesn’t matter in the end.

Indeed, I meant not using CocoaPods for adding React Native to existing apps, and doing that manually. The advantage would be less moving pieces so less things that can break (it was broken in React Native 0.53 and fixed by this PR). However, the disadvantage of having to add all sources manually might actually be worse for developers?

I think people should either integrate something completely manually and understand all of the build system or leave all of it up to a manager, the combination would lead to more confusion imo.

[alloy]

Thank you so much! 💯Fishhook and RCTAnimation was broken for me too.

I’m still very much in favour of having a RN specific spec-repo with proper podspecs for each of the RN vendored libraries, including the various distinct parts that make up the RN framework (such as RCTAnimation). In naive testing it seemed to work fine to have that spec-repo exist inside the RN repo, so there wouldn’t even be a need for another git repo.

[barbieri]

I'm not using the Animated part, however to build dev menu one needs WebSocket that needs the change:

diff -ruN node_modules/react-native/Libraries/WebSocket/RCTReconnectingWebSocket.m new_node_modules/react-native/Libraries/WebSocket/RCTReconnectingWebSocket.m
--- node_modules/react-native/Libraries/WebSocket/RCTReconnectingWebSocket.m	2018-01-29 08:51:28.000000000 +0100
+++ new_node_modules/react-native/Libraries/WebSocket/RCTReconnectingWebSocket.m	2018-01-29 08:54:01.000000000 +0100
@@ -11,7 +11,7 @@

 #import <React/RCTConvert.h>
 #import <React/RCTDefines.h>
-#import <fishhook/fishhook.h>
+#import <React/fishhook.h>

 #if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && __IPHONE_OS_VERSION_MAX_ALLOWED >= 110000 /* __IPHONE_11_0 */
 #import <os/log.h>

@Plo4ox could you incorporate that as well?

[Plo4ox]

@barbieri I don't think that regrouping all the fixes in the same PR is a good thing to do, knowing that there already is a PR that is fixing the issue.
But I don't know, @hramos @alloy @mkonicek @grabbou @koenpunt @shergin @mhorowitz @Kureev @janicduplessis, what do you think, what is the best way to do here ?

Furthermore, I just wanted to do a rebase, but the latest version of react-native in master didn't work...
I have the following error:
Compile RCTFabricUIManagerWrapper: 'folly/folly-config.h' file not found
And to fix the issue, I had to remove the following line: #include <fabric/FabricUIManager.h> from the following file: RCTFabricUIManagerWrapper.mm
As the last commit was pushed a few minutes ago it may be fixed during the next minutes ^^

[barbieri]

@Plo4ox indeed, they can be handled in separate... however I'm amazed how buggy are the podspecs and the cocoapods support. Compared to Android, gradle works like a charm, just need one single line to say the maven url.

Actually, for me I want to use with Swift... but my project is another library/framework, however I can't use React as a framework (it should be one!), thus I need a -Bridging-Header.h, which prevents my library to be exposed as a framework. Again, for Android using Kotlin is as easy and simple as using Java.

[mkonicek]

In your normal usage case, the source is indeed technically fetched from GitHub, but CP doesn’t actually fetch it again, it has a local cache from where it will unpack the same source.

@alloy This is very cool!

[rizwankce]

@Plo4ox we could close this PR as same change is being merged today with the help of #18492

[Plo4ox]

@rizwankce You are right, it seems that my PR has been deprecated by a new one 😆

[fkgozali]

And to fix the issue, I had to remove the following line: #include <fabric/FabricUIManager.h> from the following file: RCTFabricUIManagerWrapper.mm

@Plo4ox: sorry you got this error. The RCTFabricUIManagerWrapper.mm shouldn't have been linked to the existing project since it's experimental and at the very early stage.

I'll talk to my team to get this fixed. Meanwhile, could you help provide the step to repro the error (preferably as a separate issue, tagging me)?

cc @shergin, @hramos