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

RFC for Improving CocoaPods support #18

Merged
merged 1 commit into from
Jan 21, 2019

Conversation

orta
Copy link
Member

@orta orta commented Aug 15, 2018

This should make CocoaPods support match the Xcode projects currently used, and provides a technique to make sure that all testing can be done offline,


For teams adding React Native to existing apps CocoaPods support let's RN be treated as a dependency like all of the other ones. CocoaPods is built with a lot of escape valves for consumers to fix issues at runtime (you see people passing around `post_install` [fixes](https://github.com/facebook/react-native/issues/20182#issuecomment-409131862) for example, and I built a [CocoaPods plugin](https://github.com/orta/cocoapods-fix-react-native) to try consolidate them all)

I think one of the a key reasons for the drift between the CocoaPods support and the Xcode projects is because they're not mapped together.

Choose a reason for hiding this comment

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

What implications does this have to the existing consumers of the Xcode projects? My understanding is that they'll be unused by CocoaPods. How will this ensure that CocoaPods and the existing Xcode projects stay in sync?

Choose a reason for hiding this comment

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

That is to say, we've had bugs such as this which were a result of an Xcode project being misconfigured. How will changing this CocoaPods structure allow for dependency changes like these to be addressed?

Copy link

@shergin shergin Aug 16, 2018

Choose a reason for hiding this comment

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

I believe man-made Xcode projects in the repo must die, they must be build artifacts of Buck packages (Buck can generate Xcode projects). Nobody should configure builds in more than one place.


## Motivation

For teams adding React Native to existing apps CocoaPods support let's RN be treated as a dependency like all of the other ones. CocoaPods is built with a lot of escape valves for consumers to fix issues at runtime (you see people passing around `post_install` [fixes](https://github.com/facebook/react-native/issues/20182#issuecomment-409131862) for example, and I built a [CocoaPods plugin](https://github.com/orta/cocoapods-fix-react-native) to try consolidate them all)

Choose a reason for hiding this comment

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

s/support let's/support, this lets

@axe-fb
Copy link
Collaborator

axe-fb commented Aug 15, 2018

cc @dehlen for the issue created at #15

@shergin
Copy link

shergin commented Aug 16, 2018

I love the idea of decoupling dependencies and slicing existing huge CocoaPod file!

Do we have a strategy to address the fact that some deps of RN does not have Cocoapod version?

Let's consider Folly, for example. We are starting to use Folly more and more in the our new efforts and we have to have ability to depend on its the latest version available. Even we if can hack a Cocoapod for Folly (we actually tried and failed), we cannot expect that Folly contributors will fix the Cocoapod-based configuration every time they change something. So, I believe, we have to find a way to produce a Folly Cocoapod automatically from existing Buck (or cmake) configuration.

Same story with Yoga.

So, I would start the slicing from decoupling those.


## Basic example

CocoaPods support in React Native is a tad tricky, because it’s trying to map a complex set of Xcode projects into a single dependency. The Podspec for React Native does a lot of work under the hood at ~300 LOC. This is, in part, because we use a CocoaPods abstraction called subspecs to consolidate out the React Native library into one single library.
Copy link

Choose a reason for hiding this comment

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

I would also start from killing those hand-written projects. We have to figure out a way to build them automatically from Buck files which should be (they actually are) source of truth of the build configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

I read up some of the buck docs - this might be possible by having buck output the compiler options into JSON and then having the pod specs capture that, I need to try prototype it to be sure though

date: 2018-08-15
---

# RFC0003: CocoaPods Support Improvements
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is RFC0005.

@cpojer
Copy link
Member

cpojer commented Jan 21, 2019

@orta seems like people are in favor of this. What's the hold-up? Can you just go ahead and make this happen?

@orta
Copy link
Member Author

orta commented Jan 21, 2019

Yep, it can happen.

It's mainly been blocked on us upgrading to the latest versions of React Native at Artsy ( artsy/emission#1210 ) so that we can use/test it in-house. Which is on-going, and sporadically worked on by a few of us in our spare time. So, it might take some time for me to get around to it

@cpojer cpojer merged commit 08733ee into react-native-community:master Jan 21, 2019
@cpojer
Copy link
Member

cpojer commented Jan 21, 2019

I merged this RFC then (feel free to follow-up with small changes, given that you were still iterating on it a bit). I'll let you do this whenever you get around to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Proposal This label identifies a proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants