Make `react-native link` play nicely with CocoaPods-based iOS projects. #15460

mironiasty · 2017-08-11T07:08:08Z

The core React Native codebase already has full support for CocoaPods. However, react-native link doesn’t play nicely with CocoaPods, so installing third-party libs from the RN ecosystem is really hard.

This change will allow to link projects that contains its own .podspec file to CocoaPods-based projects. In case link detect Podfile in iOS directory, it will look for related .podspec file in linked project directory, and add it to Podfile. If Podfile and .podspec files are not present, it will fall back to previous implementation.

Test Plan

Build a React Native project where the iOS part uses CocoaPods to manage its dependencies. The most common scenario here is to have React Native be a Pod dependency, among others.
Install a RN-related library, that contains .podspec file, with react-native link (as an example it could be: react-native-maps
Building the resulting iOS workspace should succeed (and there should be new entry in Podfile)

pull-bot · 2017-08-11T07:16:29Z

	Messages
📖	📄 Docs - Thanks for your contribution to the docs!

@facebook-github-bot label Documentation

Attention: @hramos, @grabbou, @Kureev

Generated by 🚫 dangerJS

Kureev

Code looks good to me! I'd even say very good. Documentation, test coverage - everything is there. I only need some time in order to test it before I give my approval.

Kureev · 2017-08-11T08:43:18Z

@mironiasty once again, great job 🙌 Really enjoy reviewing high-quality PRs

mironiasty · 2017-08-14T15:44:58Z

@Kureev Thanks :)

Also another way to test it, is to create Expo project, and then use exp detach on it. This should create iOS project with Podfile. And such project could be updated with modified implementation of react-native link.

grabbou · 2017-08-22T08:20:02Z

This looks really solid. Let me get back to you on that tomorrow latest. Pretty sure will be flawless :)

Kureev

First of all, sorry for taking that much time for review.
I've made a quick test an it works flawless.

After @grabbou double-check this, we're confident to merge it in

grabbou · 2017-08-31T10:44:43Z

local-cli/link/pods/unregisterNativeModule.js

+ * Unregister native module IOS with CocoaPods
+ */
+module.exports = function unregisterNativeModule(dependencyConfig, iOSProject) {
+


I believe empty line here and on line 14 is not intended? (nitpick)

grabbou · 2017-08-31T10:45:07Z

local-cli/link/pods/unregisterNativeModule.js

+module.exports = function unregisterNativeModule(dependencyConfig, iOSProject) {
+
+  const podContent = fs.readFileSync(iOSProject.podfile, 'utf8');
+  const removed = removePodEntry(podContent, dependencyConfig.podspec);


After removing, shall we also call pod install to sync the project? What happens right now after you unlink?

I don't call pod install anywhere. It is up to user to call it later on (it could be also run automatically in build process/Xcode). Right now link/unlink processes only modify configuration files (and with this change only modify Podfile file).

grabbou · 2017-08-31T10:46:48Z

local-cli/link/pods/findLineToAddPod.js

+  const nextTarget = /target (\'|\")\w+(\'|\") do/g;
+  // match line that has only 'end' (if we don't catch new target or function, this would mean this is end of current target)
+  const endOfCurrentTarget = /^\s*end\s*$/g;
+  // match function defeinition, like: post_install do |installer| (some Podfiles have function defined inside main target


nit: typo in definition

grabbou · 2017-08-31T10:46:55Z

local-cli/link/pods/findLineToAddPod.js

+
+module.exports = function findLineToAddPod(podLines, firstTargetLine){
+
+  // match line with new targe: target 'project_name' do (most likely target inside podfile main target)


nit: typo in target

grabbou · 2017-08-31T10:47:07Z

local-cli/link/pods/findLineToAddPod.js

+'use strict';
+
+module.exports = function findLineToAddPod(podLines, firstTargetLine){
+


Same here: no need to include an empty line :)

grabbou · 2017-10-13T14:49:04Z

local-cli/link/pods/addPodEntry.js

+	const newEntry = `pod '${podName}', :path => '../node_modules/${nodePath}'\n`;
+
+	if (!linesToAddEntry) {
+		return;


This file has wrong indentation. Try running prettier for all the files in your PR (not sure if you have it integrated already)

grabbou

Okay, I think this is a good start - in the future ,we might want to handle multiple targets in CocoaPods as well.

Last thing to improve here is to run prettier on all files. Some of the files use tabs for indentation.

Check if your editor has well configured prettier and editor-config.

mironiasty · 2017-10-16T07:57:45Z

Sorry for these tabs, I hope it is fixed now. (I've been working on other project with tabs, and priettier used that config on react-native files)

grabbou · 2017-10-17T15:17:39Z

@facebook-github-bot shipit

facebook-github-bot

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

Summary: The core React Native codebase already has full support for CocoaPods. However, `react-native link` doesn’t play nicely with CocoaPods, so installing third-party libs from the RN ecosystem is really hard. This change will allow to link projects that contains its own `.podspec` file to CocoaPods-based projects. In case `link` detect `Podfile` in `iOS` directory, it will look for related `.podspec` file in linked project directory, and add it to `Podfile`. If `Podfile` and `.podspec` files are not present, it will fall back to previous implementation. **Test Plan** 1. Build a React Native project where the iOS part uses CocoaPods to manage its dependencies. The most common scenario here is to have React Native be a Pod dependency, among others. 2. Install a RN-related library, that contains `.podspec` file, with `react-native link` (as an example it could be: [react-native-maps](https://github.com/airbnb/react-native-maps) 3. Building the resulting iOS workspace should succeed (and there should be new entry in `Podfile`) Closes #15460 Differential Revision: D6078649 Pulled By: hramos fbshipit-source-id: 9651085875892fd66299563ca0e42fb2bcc00825

chrisbianca · 2017-11-22T09:37:17Z

One of the core maintainers of https://github.com/invertase/react-native-firebase here.

We've spent the last year battling with RN's lack of integration with Cocoapods and the pain this causes our users. Having given up on the Cocoapods approach and switching to usage of react-native link as it used to be, we are in the position where we have a .podspec for legacy support, but do not wish this to be used for new projects due to the complications this brings with needing to also add the appropriate React pod and subspecs.

Since this change, our .podspec is picked up automatically, but it seems this PR provides no way for us to opt out of that decision and override the presence of the podspec.

return {
    sourceDir: path.dirname(projectPath),
    folder: folder,
    pbxprojPath: path.join(projectPath, 'project.pbxproj'),
    podfile: findPodfilePath(projectPath),
    podspec: findPodspecName(folder),
    projectPath: projectPath,
    projectName: path.basename(projectPath),
    libraryFolder: userConfig.libraryFolder || 'Libraries',
    sharedLibraries: mapSharedLibaries(userConfig.sharedLibraries || []),
    plist: userConfig.plist || [],
  };

As such, this has fundamentally broken our on-boarding flow for new users and leaves us with only two options:

To remove the .podspec completely which removes backwards compatibility for those devs who have been using the .podspec previously
To switch back to the .podspec approach and introduce the added step of having the users need to set up React and subspecs as pod - something which we know causes issues based on our past experiences

Neither of these are particularly viable alternatives and both involve compromises.

As somebody that has tried to submit PRs to improve the build process customisability in the past (on the Android side), only for the PR to be completely ignored, it's incredibly frustrating to now be affected by a change that hasn't taken into consideration what this might mean for some libraries.

mironiasty · 2017-11-22T13:10:52Z

One thing that might be worth to mention here: this change would only work for users that have Pods configured for their iOS project (so users without Podfile should not be affected). But from react-native-firebase installation documentation, I see that configuring pods is recommended step for users.

I don't know what is exact problem with linking your project using CocoaPods. If problem is related to missing react entries in Podfile, maybe you can recommend creating project with create-react-native-app and detaching project from Expo (this way, user should have fully working Podfile).

As for adding option to opt-out from linking podspecs: I can think of two ways of doing it:

Adding new parameter to link (react-native link react-native-firebase --ignore-pods)
Checking for predefined comment in project .podspec file (like #REACT_NATIVE_IGNORE_FOR_LINKING)

I'm not sure which one would be better. Also adding any of them, would again increase complexity of linking process.

chrisbianca · 2017-11-22T14:57:09Z

@mironiasty thanks for your response.

Our users need to configure pods in order to use the Firebase dependencies, as this is the approach that Google recommend and we don't want to deviate from it.

We also categorically don't want to recommend people creating projects using Expo as a way of getting started. This has also caused us numerous problems down the line and is not a viable alternative given the large dependency that react-native-firebase has on native code.

Regarding your options above:

This would only work when linking just this library, anybody running react-native link without the library would encounter problems
This would work, though feels somewhat hacky

A much better option (and one that keeps in sync with the rest of the rnpm settings) would be to allow this to be overridden as part of the package.json. For example, setting an ignorePodSpec as so:

"rnpm": {
    "ios" {
      "ignorePodSpec": true,
    }
  },

Summary: The core React Native codebase already has full support for CocoaPods. However, `react-native link` doesn’t play nicely with CocoaPods, so installing third-party libs from the RN ecosystem is really hard. This change will allow to link projects that contains its own `.podspec` file to CocoaPods-based projects. In case `link` detect `Podfile` in `iOS` directory, it will look for related `.podspec` file in linked project directory, and add it to `Podfile`. If `Podfile` and `.podspec` files are not present, it will fall back to previous implementation. **Test Plan** 1. Build a React Native project where the iOS part uses CocoaPods to manage its dependencies. The most common scenario here is to have React Native be a Pod dependency, among others. 2. Install a RN-related library, that contains `.podspec` file, with `react-native link` (as an example it could be: [react-native-maps](https://github.com/airbnb/react-native-maps) 3. Building the resulting iOS workspace should succeed (and there should be new entry in `Podfile`) Closes facebook/react-native#15460 Differential Revision: D6078649 Pulled By: hramos fbshipit-source-id: 9651085875892fd66299563ca0e42fb2bcc00825

mironiasty added 12 commits July 7, 2017 15:38

Allow to link libraries that use CocoaPods

d6c79e4

Allow to unregister libraries linked with CocoaPods

db0c191

Add regex to find function header in Podfile

7049f0d

Add podfile and podspec path into config

5c1fce9

Use podspec file name as Pod name

faa3056

Add quick check to see if pod file exist

c6e6add

Add tests to pods part of project configuration reader

d0577db

Add tests to linking pod file files

137c81d

Code cleanup - add missing semicolons and remove license text

93d156c

Add comments for used regular expressions

49fcd10

Add info about linking Podfiles to documentation.

18c738f

Merge branch 'master' into link-with-podfile

bc2773e

mironiasty requested review from grabbou and Kureev as code owners August 11, 2017 07:08

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Aug 11, 2017

Kureev reviewed Aug 11, 2017

View reviewed changes

Kureev approved these changes Aug 23, 2017

View reviewed changes

anp mentioned this pull request Aug 25, 2017

rn-link should work better with CocoaPods expo/expo#254

Closed

Merge branch 'master' into link-with-podfile

82c3fa9

grabbou requested a review from hramos as a code owner August 31, 2017 10:42

grabbou reviewed Aug 31, 2017

View reviewed changes

grabbou reviewed Oct 13, 2017

View reviewed changes

grabbou suggested changes Oct 13, 2017

View reviewed changes

Fix file formatting (remove tabs, add missing semicolons)

7b3574c

grabbou approved these changes Oct 17, 2017

View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Oct 17, 2017

grabbou mentioned this pull request Oct 17, 2017

[0.50] Commits to cherry-pick into stable release #16257

Closed

facebook-github-bot approved these changes Oct 17, 2017

View reviewed changes

facebook-github-bot closed this in 74146cb Oct 18, 2017

0xpatrickdev mentioned this pull request Oct 19, 2017

Warning: Native component for "FastImageView" does not exist DylanVann/react-native-fast-image#51

Closed

chrisbianca mentioned this pull request Nov 22, 2017

react-native-firebase doesn't work with v0.50.0 of react-native invertase/react-native-firebase#636

Closed

This was referenced Nov 22, 2017

"Something went wrong while linking. Error: Cannot read property 'match' of undefined" #16774

Closed

"Something went wrong while linking. Error: Cannot read property 'match' of undefined" microsoft/react-native-code-push#1085

Closed

olivierlesnicki mentioned this pull request Nov 27, 2017

[RN 0.50.0] - Incompatibility between react-native link and Pods #16912

Closed

thedevdavid mentioned this pull request Dec 4, 2017

'GeneratedDotEnv.m' file not found lugg/react-native-config#187

Closed

ericnograles mentioned this pull request Dec 17, 2017

Pull down JB4A-SDK Using CocoaPods ericnograles/react-native-exact-target#16

Open

itinance mentioned this pull request Jan 3, 2018

'React/RCTBridgeModule.h' file not found after Cocoapods installation itinance/react-native-fs#408

Closed

terribleben mentioned this pull request Jan 9, 2018

Adds a podspec for CocoaPods support amazon-archives/amazon-cognito-identity-js#644

Merged

terribleben mentioned this pull request Feb 3, 2018

Problems with compiling ejected ExpoKit app with the Firebase/Firestore Pod. expo/expo#1374

Closed

chrisbianca mentioned this pull request Feb 20, 2018

Move podspec to where "react-native link" expects it to be invertase/react-native-firebase#838

Closed

chrisbianca mentioned this pull request Mar 31, 2018

Argument list too long errors when running 4.0.0-rc.2 on ios invertase/react-native-firebase#917

Closed

olivierlesnicki mentioned this pull request May 27, 2018

[RN 0.55.4] - Incompatibility between react-native link and Pods #19468

Closed

3 tasks

bartolkaruza mentioned this pull request Dec 9, 2018

Make iOS linking to Podfile or directly linking as framework an option for library maintainer and for consumer react-native-community/cli#37

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `react-native link` play nicely with CocoaPods-based iOS projects. #15460

Make `react-native link` play nicely with CocoaPods-based iOS projects. #15460

mironiasty commented Aug 11, 2017

pull-bot commented Aug 11, 2017 •

edited

Loading

Kureev left a comment •

edited

Loading

Kureev commented Aug 11, 2017

mironiasty commented Aug 14, 2017

grabbou commented Aug 22, 2017

Kureev left a comment

grabbou Aug 31, 2017

grabbou Aug 31, 2017

mironiasty Aug 31, 2017

grabbou Aug 31, 2017

grabbou Aug 31, 2017

grabbou Aug 31, 2017

grabbou Oct 13, 2017

grabbou left a comment

mironiasty commented Oct 16, 2017

grabbou commented Oct 17, 2017

facebook-github-bot left a comment

chrisbianca commented Nov 22, 2017

mironiasty commented Nov 22, 2017

chrisbianca commented Nov 22, 2017


		module.exports = function findLineToAddPod(podLines, firstTargetLine){

		// match line with new targe: target 'project_name' do (most likely target inside podfile main target)

		'use strict';

		module.exports = function findLineToAddPod(podLines, firstTargetLine){

Make react-native link play nicely with CocoaPods-based iOS projects. #15460

Make react-native link play nicely with CocoaPods-based iOS projects. #15460

Conversation

mironiasty commented Aug 11, 2017

pull-bot commented Aug 11, 2017 • edited Loading

Kureev left a comment • edited Loading

Choose a reason for hiding this comment

Kureev commented Aug 11, 2017

mironiasty commented Aug 14, 2017

grabbou commented Aug 22, 2017

Kureev left a comment

Choose a reason for hiding this comment

grabbou Aug 31, 2017

Choose a reason for hiding this comment

grabbou Aug 31, 2017

Choose a reason for hiding this comment

mironiasty Aug 31, 2017

Choose a reason for hiding this comment

grabbou Aug 31, 2017

Choose a reason for hiding this comment

grabbou Aug 31, 2017

Choose a reason for hiding this comment

grabbou Aug 31, 2017

Choose a reason for hiding this comment

grabbou Oct 13, 2017

Choose a reason for hiding this comment

grabbou left a comment

Choose a reason for hiding this comment

mironiasty commented Oct 16, 2017

grabbou commented Oct 17, 2017

facebook-github-bot left a comment

Choose a reason for hiding this comment

chrisbianca commented Nov 22, 2017

mironiasty commented Nov 22, 2017

chrisbianca commented Nov 22, 2017

Make `react-native link` play nicely with CocoaPods-based iOS projects. #15460

Make `react-native link` play nicely with CocoaPods-based iOS projects. #15460

pull-bot commented Aug 11, 2017 •

edited

Loading

Kureev left a comment •

edited

Loading