Replacing iOS Shimmer submodule with a cocoapods dependency on Shimmer

chaseholland commented 5 years ago

Resolving an issue where npm was sometimes not downloading iOS shimmer on npm install (presumably since Shimmer was a submodule). This updates iOS Shimmer to be a pod dependency rather than a submodule, also resolving issues with name spacing in apps that already have a dependency on Shimmer.

Additionally:

cleaned up dependency specifiers in package.json and the podspec
bumped the sample app dependency to RN 0.57.1
added a Podfile to the sample app
bumped version to 0.5.0 to reflect the change in how ios shimmer is consumed

oblador commented 5 years ago

Thanks for your PR! The issue was due to a faulty local checkout of the repo after migrating to a new computer. I don't have the releasing part automated yet due to the infrequency of updates, but this is a pretty good argument for fixing that.

Regarding your PR: This would be a breaking change and force people to use CocoaPods. While I agree that your solution is much better and I personally use CocoaPods to handle my iOS dependencies, I know that most in the RN community don't. We could also allow a gradual CocoaPods adoption that's a bit more common, just for the Shimmer dependency itself.

What are your thoughts?

chaseholland commented 5 years ago

Regarding the move to Shimmer as a pod dependency vs a submodule -- This would, indeed, be a breaking change for some folks. The existing approach with a submodule will cause namespacing issues for people already using Shimmer, forcing them to delete whatever copy of Shimmer they already have. So -- there is that to consider. Although it will cause short term pain, I think eliminating the submodule is a good way to go.

Would it make sense to update the integration guide to give alternate directions for those not using cocoapods? We could say something like:

iOS Integration (with cocoapods)

Simply the following to your Podfile:

pod 'react-native-shimmer', :podspec => 'path/to/node_modules/react-native-shimmer'

iOS integration (not using cocoapods)

react-native-shimmer relies on Shimmer from Facebook. You need to add the Shimmer source from Facebook to your Xcode project. Shimmer source can be found here..

Specifically, add FBShimmering.h, FBShimmeringView.h, FBShimmeringView.m, FBShimmeringLayer.h, and FBShimmeringLayer.m to your Xcode project.

Admittedly, my background is in native iOS development -- I've only recently started with react -- so I don't know what the larger community's integration expectation will be.

How do react-native-only developers usually pull in dependencies? All the guides I've seen for RN online reference cocoapods. Do you think most folks would find an integration guide update acceptable?

oblador commented 5 years ago

Most people expect react-native link to fix their problems that will link the Xcode project, but I guess many are also comfortable using CocoaPods for "pure native" dependencies. Fewer are using CocoaPods for all their dependencies tho. I think the challenge here would be how to explain to users what the different types of CocoaPods setups are relevant to them.

oblador commented 5 years ago

We could be doing two separate approaches, one for cocoapods and one for regular linking. Not saying that's not a terrible solution tho.

chaseholland commented 5 years ago

I tried doing some research on react-native link to see what the best alternate solution is here. Looks like some people (like lottie from AirBnB) recommend opening Xcode and embedding the lottie binary. However, FBShimmer doesn't publish a static framework.

Others (like react-native-fs) recommend cocoapods first, but they also have similar manual link directions that involve manually copying files in Xcode. https://github.com/itinance/react-native-fs/#adding-manually-in-xcode

From RNFS's docs:

Adding Manually in XCode

In XCode, in the project navigator, right click Libraries ➜ Add Files to [your project's name] Go to node_modules ➜ react-native-fs and add the .xcodeproj file

In XCode, in the project navigator, select your project. Add the lib*.a from the RNFS project to your project's Build Phases ➜ Link Binary With Libraries. Click the .xcodeproj file you added before in the project navigator and go the Build Settings tab. Make sure 'All' is toggled on (instead of 'Basic'). Look for Header Search Paths and make sure it contains both $(SRCROOT)/../react-native/React and $(SRCROOT)/../../React - mark both as recursive.

Run your project (Cmd+R)

I couldn't really find anyone with a "good" non-cocoapods solution. There seems to always be manual copying involved. :/

emusgrave commented 5 years ago

@chaseholland @oblador I'm one of those people that has avoided using Cocoapods for react-native development. I don't believe that your solution of manually adding the FBShimmering project is working, because the RNShimmer project is directly referencing the FBShimmering.h/m files, expecting them to exist within the RNShimmer directory structure.

I could probably get it to work by manually adding those files under ./node_modules/react-native-shimmer/ios/Shimmer/FBShimmering, but then each time I do an npm install I'll have to copy those same files into the directory again.

For now I'm just sticking with 0.4.1 since it's working fine.

oblador / react-native-shimmer