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

Fabric: add podspecs #23802

Closed
wants to merge 2 commits into from
Closed

Conversation

ericlewis
Copy link
Contributor

Summary

This adds up to date podspecs for compiling fabric. Special thanks to @kmagiera which did the original libevent & folly header changes.

Changelog

[iOS] [Added] - Fabric podspecs

Test Plan

Added the following to RNTester and ran pod install then compiled.

pod 'React-Fabric', :path => '../ReactCommon'
pod 'React-graphics', :path => '../ReactCommon/fabric/graphics'
pod 'React-RCTFabric', :path => '../React'
pod 'libevent', :podspec => '../third-party-podspecs/libevent.podspec'

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 7, 2019
@ericlewis
Copy link
Contributor Author

This has a breaking change for Folly FYI, you will be required to include libevent in your podspec along side the other 3rd party libs.

@ericlewis
Copy link
Contributor Author

ericlewis commented Mar 7, 2019

Note: this will have trouble compiling now, due to the fact I am pulling in codegen headers again. A prerequisite would be to properly run the expected codegen before pod install.

@ericlewis ericlewis closed this Mar 13, 2019
@ericlewis
Copy link
Contributor Author

ericlewis commented Mar 13, 2019

Closing in favor of #23803.

facebook-github-bot pushed a commit that referenced this pull request Mar 16, 2019
Summary:
This is the couple of hacks I used after I finished #23802 in order to get fabric working on RNTester. This is inspired from prior work by kmagiera.

The goal of this PR is to show others what I’m struggling with, and to eventually merge it sans hacks.

- Yarn Install
- Uncomment the commented out pods in RNTester's pod file
- Open RNTesterPods workspace
- Run App

- this is only for pods, the non-pod RNTester will no longer work until updated with fabric too.
- `SurfaceHostingView` & `SurfaceHostingProxyRootView` both try to start the surface immediately, this leads to a race condition due to the javascript not having loaded yet, the hack here is:
   1. Swizzle the `start` method on `RCTFabricSurface` to no-op when called.
   2. Add observer for `RCTJavaScriptDidLoadNotification`
   3. Call private method `_startAllSurfaces` on `_surfacePresenter` in AppDelegate when we receive `RCTJavaScriptDidLoadNotification`.

[General] [Added] - Use Fabric in RNTester
Pull Request resolved: #23803

Reviewed By: shergin, mdvacca

Differential Revision: D14450726

Pulled By: fkgozali

fbshipit-source-id: 8ae2d48634fecb60db539aaf0a2c89ba1f572c27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants