-
Notifications
You must be signed in to change notification settings - Fork 1
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
MHP-2029: Upgrade packages #570
Conversation
Why are there so many files changed??? Lol |
I committed the Pods folder... https://guides.cocoapods.org/using/using-cocoapods.html#should-i-check-the-pods-directory-into-source-control suggested it. We could make everyone run |
I think I started down the podfile route cuz I was getting errors with the Facebook SDK. I don't remember what they were. Idk if moving it to pods was the only solution but seemed like it would be an improvement. Also that's what the new Facebook docs say in their install instructions. |
That's a great idea. I'm glad you did that. Will it increase the size of our app? Also, could perhaps add that change in a separate commit so it's easier to review? |
7feb2ce
to
dd49981
Compare
I moved the Podfile stuff to a separate commit. I didn't really want to reconcile the updates to the ios project (the giant project.pbxproj) from both the pods stuff and the react native upgrade so it's still in the second commit. The Podfile commit won't work on its own (although we could work on configuring it separately if needed). I meant to check and see if all of these pods are needed. I guess facebook has split it into separate packages. I was definitely getting errors about FBSDKShareKit being missing. Maybe the react native package needs them all.
Idk that it'll change the size of our app since this should be the same FB library as before, just imported differently. But I haven't checked it. |
- Remove dependency on having to manually downloaded the Facebook SDK to your Documents folder - Commit Pods directory per documentation suggestion and allow other developers to not have to run pod install locally - Integration of Podfile into ios project happens in next commit
- Upgrade to latest React, React Native, Babel 7 - Upgrade all other packages - Integrate Podfile added in last commit into ios project - Get running with Xcode 10
…e react-test-renderer
dd49981
to
cc5e3b3
Compare
Can we close this? |
I started down this rabbit hole and it took a lot longer than I expected... I got so close. Everything works except the tests. Enzyme doesn't seem to like the new React context API yet... enzymejs/enzyme#1647 There doesn't seem to be much progress on that issue. Guess this might sit for a little while. I didn't commit the snapshot updates yet cuz they'll probably change
The latest React Native requires react@16.6.0-alpha.8af6728. We could wait until that lands in a stable release. But I think the newest React Native is needed to support the Xcode 10 build process.
With Xcode 10, for now you may need to run the commands at:
facebook/react-native#20774 (comment)
Otherwise during the iOS build there will be missing file errors.
Do any of you guys feel comfortable reviewing the iOS stuff? Is there anyone I can ask? I guess I mainly want to see if the podfile setup is correct.