React-native: [React Fabric] Updating Build Configuration (CocoaPods, .xcodeproj, gradle)

Created on 15 Jan 2019  ·  24Comments  ·  Source: facebook/react-native

For Discussion

Various parts of React Fabric code are available in github, but we need to upgrade a bunch of build configuration to make it build properly on iOS and Android. This will involve reflecting the BUCK targets for various parts of Fabric into the 3 areas:

An initial version of the update could just be manually sync'ed configs for .podspec, .xcodeproj, and .gradle.

But for the future, some ideas include:

  • Have BUCK generate the relevant .podspec, .xcworkspace, or .gradle -- this may require a bunch of work via buck project
  • etc

Requirements

  • Have RNTester built with Fabric symbols correctly (iOS/Android to start)
  • Upgraded version of Folly - see https://github.com/facebook/react-native/issues/20302

    • Note that we're on v2018.10.22 already (https://github.com/facebook/react-native/pull/21976, https://github.com/facebook/react-native/pull/21977)

cc @mdvacca, @shergin

Help Wanted Discussion

Most helpful comment

I believe the issue is due to AppDelegate.m not being an Objective-C++ file

Ah, yeah a bunch of files need to become .mm instead. I think that's an ok requirement though, given the increasing number of C++ we're adding. That said, if AppDelegate.m needs to be AppDelegate.mm, let's switch it over. If you find more .m files to be converted to .mm, let's note them here as well.

All 24 comments

Hey there! I'd like to help, so I started with the podspec, and I'd appreciate any tips, on how to test and make sure everything is working. Thanks.

on how to test and make sure everything is working

First step is to have RNTester podspec able to build Fabric specs (that includes all subspecs from fabric directory).

The next step after that is to actually use Fabric in RNTester. This is still work in progress for our team, so we can tackle that slightly later. cc @shergin

Hey, could you guys please verify if this include is correct? I'm getting an error 'Unknown type name 'CGFloat'' in this file and looking at the code '' is always used with .m files and import instead of include and .cpp files.

Hey, could you guys please verify if this include is correct? I'm getting an error 'Unknown type name 'CGFloat'' in this file and looking at the code '' is always used with .m files and import instead of include and .cpp files.

Does it work if you modify it to #import ? This builds fine using Buck at FB, perhaps there's a missing flag somewhere? Or perhaps you're missing the dependency on CoreGraphics iOS framework in the podspec?

If moving to #import works feel free to send a PR for it. Looking at a similar file, seems like we use #import there: https://github.com/facebook/react-native/blob/94d49e544d10d0039a1178dc97533e96a4354198/ReactCommon/fabric/imagemanager/platform/ios/RCTImagePrimitivesConversions.h#L8

@fkgozali are there any question marks you see as far as using CocoaPods with Fabric/TurboModules? I'd love to hear more about your experience so far.

I can take a look at updating the xcodeproj files.

are there any question marks you see as far as using CocoaPods with Fabric/TurboModules? I'd love to hear more about your experience so far.

We don't use cocoapods/gradle internally, so I'm hoping the unknowns can be covered with the help from community on this issue.

Hey React.podspec is up for grabs again, I'm sorry, I didn't have enough time to finish it up.

I switched gears to updating the podspec (xcodeprojs are a pain). Here is what I have so far: https://github.com/mgriepentrog/react-native/commit/44c56493085e3a204b31d27f51c8c068369e988b. To find a lot of the low-hanging fruit, I'm running pod install inside of RNTester and trying to build the app with the RCTFabric subspec included. Currently encountering react-native/RNTester/Pods/Headers/Private/React/react/graphics/Geometry.h:55:3: Unknown type name 'Float'; did you mean 'float'?, but haven't had much time to investigate.

I switched gears to updating the podspec (xcodeprojs are a pain). Here is what I have so far: mgriepentrog@44c5649. To find a lot of the low-hanging fruit, I'm running pod install inside of RNTester and trying to build the app with the RCTFabric subspec included. Currently encountering react-native/RNTester/Pods/Headers/Private/React/react/graphics/Geometry.h:55:3: Unknown type name 'Float'; did you mean 'float'?, but haven't had much time to investigate.

I got stuck on the same problem, and couldn't get pass it I added same frameworks, changed some files and then same problem again. 😟

I made progress on getting the cocoapods to build and ran in to a few problems:

  • CGFloat in Float.h, same problems as above, it seems that the interop isn't working, I stubbed this with a double to continue.
  • RCTSwitchComponentView makes references to react/components/rncore/, which I could not find, nor could I find something similar.
  • ~Missing x86_64 Symbol Error on getDefaultComponentRegistryFactory, but that might be due to my stubbing?~ <- This is due to missing folly imports.

I am still working on getting it to build a little further, and and still trying to figure out what is going on with Float.h.

But the podfile seems mostly complete: https://github.com/ericlewis/react-native/commit/a6510d9652beab949d944dae923d7881678abcb2. It creates libReact.a.

One thing I noticed, if you look at the Pods project under the Frameworks folder, the CoreGraphics framework is pointing to /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS12.0.sdk/System/Library/Frameworks/CoreGraphics.framework, which doesn't exist (the latest sdk version is 12.1). I'm not sure where that path is getting derived from.

I updated those frameworks manually to point to 12.1 & was no dice.

Here is my WIP: https://github.com/ericlewis/react-native/commits/fabric

  • pod files seem “done”
  • renaming Float.h fixes CGFloat issue. Conflicts somewhere.

Compiling this branch's RNTester with RCTFabric & RCTFabricSample works, it links correctly! But you must remove the fabric switch component refs, bc they ref something called rncore that appears to be missing.

Another issue too is I get a “functional file not found” in relation to what looks like libc++ headers in LayoutPrimitives.h, it also happens in SurfacePresenter with memory, tho that is fixed by a rename to memory.h.

I have not been able to figure out why this is happening, tho it is not happening in older versions of fabric using this same technique.

Another issue too is I get a “functional file not found” in relation to what looks like libc++ headers in LayoutPrimitives.h, it also happens in SurfacePresenter with memory, tho that is fixed by a rename to memory.h.

cc @shergin, @JoshuaGross -- thoughts?

@fkgozali I figured out what the issue was.. I am not sure I am using the library correctly, it happened when importing RCTSurfacePresenter.h, I believe the issue is due to AppDelegate.m not being an Objective-C++ file. It does link and build and run when I use RCTSurfaceHostingProxyRootView, but it is blank due to not having a presenter.

My current progress, open issues, and questions: #23550

I believe the issue is due to AppDelegate.m not being an Objective-C++ file

Ah, yeah a bunch of files need to become .mm instead. I think that's an ok requirement though, given the increasing number of C++ we're adding. That said, if AppDelegate.m needs to be AppDelegate.mm, let's switch it over. If you find more .m files to be converted to .mm, let's note them here as well.

RNTester now works with Fabric, and we have a separate issue dealing with folly so I'm gonna go ahead and close this issue.

@fkgozali feel free to reopen or create a new issue depending on whether you wanted to keep this issue as a meta issue for all Fabric work or just scoped to making things work in RNTester.

Hold on, this is the umbrella ask for fabric build, including android, so let's keep this open. What was done was just for CocoaPods.

Also what's not yet done:

  • component registration, integrated with build system (this doesn't exist in OSS yet)
Was this page helpful?
0 / 5 - 0 ratings