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

Integrate Flipper in RNTester on iOS #27762

Open
wants to merge 1 commit into
base: master
from

Conversation

@priteshrnandgaonkar
Copy link
Contributor

priteshrnandgaonkar commented Jan 14, 2020

Summary: Integrates Flipper in RNTester application and fixes the integration issues with use_frameworks! keyword.

Differential Revision: D19391739

@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Jan 14, 2020

This pull request was exported from Phabricator. Differential Revision: D19391739

@pull-bot

This comment has been minimized.

Copy link

pull-bot commented Jan 14, 2020

Messages
📖

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

📖 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.
📖 📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.

Generated by 🚫 dangerJS against eef0942

Summary:
Pull Request resolved: #27762

Integrates Flipper in RNTester application and fixes the integration issues with `use_frameworks!` keyword.

Changelog: Flipper integration in RNTester application.

Differential Revision: D19391739

fbshipit-source-id: 99e2fa1435b5a3d6959b35133e1d391f315a953a
@priteshrnandgaonkar priteshrnandgaonkar force-pushed the priteshrnandgaonkar:export-D19391739 branch from dc5f854 to eef0942 Jan 14, 2020
@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Jan 14, 2020

This pull request was exported from Phabricator. Differential Revision: D19391739

@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Jan 14, 2020

Hi priteshrnandgaonkar! Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but we do not have a signature on file.In order for us to review and merge your code, please sign 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 to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

end
end
end
end

This comment has been minimized.

Copy link
@alloy

alloy Jan 16, 2020

Collaborator

@priteshrnandgaonkar Could you link me to a write-up, if one exists, on the need for these deps to be linked statically?

This comment has been minimized.

Copy link
@priteshrnandgaonkar

priteshrnandgaonkar Jan 16, 2020

Author Contributor

Go through the iOS setup mentioned here

This comment has been minimized.

Copy link
@alloy

alloy Jan 17, 2020

Collaborator

Thanks. I’ve seen those before, but they do not describe why these dependencies need to be linked statically and why a monkey-patch of CocoaPods is required. I would like to understand those technical reasons, as I would like to see if there’s a way to avoid the need to monkey-patch.

This comment has been minimized.

Copy link
@priteshrnandgaonkar

priteshrnandgaonkar Jan 17, 2020

Author Contributor

The reason being, Flipper relies on CocoaLibEvent which has bunch of statically linked binaries libevent.a, libevevnt_core.a etc. due to which the SubPods relying on it has to be built as a Static Framework, which is Flipper-Folly. So ideally speaking building just CocoaLibEvent, Flipper-Folly as a static framework should work, but for some reason the dynamic framework failed to link Flipper-Folly, which I was not able to solve. Thus I built all the Flipper and Flipper related deps as a Static Framework and it gave no errors.

Even I feel that we shouldn't have a monkey-patch, if you can solve this problem, then it would be great.

This comment has been minimized.

Copy link
@alloy

alloy Jan 20, 2020

Collaborator

Ok, when I’m back from vacation I’ll take a look-see 👍

@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Jan 16, 2020

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

@rickhanlonii rickhanlonii changed the title Integrate Flipper in RNTester application Integrate Flipper in RNTester on iOS Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.