Forward port to facebook-ios-sdk v12

[mikehardy]

Looks like a sizable batch of changes, at the least I think the initialization code will change, some of the other removals/deprecations will likely be hit as well

https://github.com/facebook/facebook-ios-sdk/blob/master/CHANGELOG.md#1200

[danielmark0116]

Hi @mikehardy - you simply want to migrate to the latest SDK v? (11th)? 🧐🙂

[mikehardy]

In general, support for the current stable version of the underlying sdks is always the goal

[perrosnk]

I am also interested in this because of a compatibility issue with react-native-applovin-max.

[TheMakerOleguch]

Hello everyone, do we have in roadmap upgrade to v11?

[mikehardy]

@TheMakerOleguch no one has taken it up. Do you want to propose a PR? It would be most welcome!

[tranducduong1994]

I think we can fix the issues below once the library us upgraded to v11.

1. fetchDefferedAppLink didn't throw an error In IOS 14 ( upper 14 version ), If user didn't grant Tracking Transparency Permission.
2. fetchDefferedAppLink doesn't work on IpadOS, this function doesn't throw any error also. :(

[mikehardy]

The SDK will not successfully operate with android targetSdk of 31 until this is done

[mikehardy]

@ArsenyYankovsky has done a fantastic job with #125 so we are up to facebook-ios-sdk v11-current now. V12 is still a big step away, but we are closer!

Here is my comment related to android, from that PR:

This looks fantastic, to me, thank you very much for doing this! After a quick scan of the changelogs it seems like doing Android to v11 might not be so bad, and to v12 on android also not so bad, but the v12 jump for iOS looks pretty big unfortunately. Any thoughts on those?

I do not have time to do it myself but I am happy to collaborate with anyone trying to make a PR and I can release results if PRs come in! 🙌

[mikehardy]

I actually had a spare moment and I've done android up to v12 666ce0e

I'll release the iOS v11 / android v12 changes when that PR is ready (may take a while)

I will not have time to move forward to iOS v12, we really need contributors, if anyone has the time to do that forward port it would be great. I just cleaned up the example app so it's easier to work with in order to do the development, it has a README

cheers

[mikehardy]

Lots of noise in the iOS console log until we can adopt facebook-ios-sdk v12.1, for what it's worth: facebook/facebook-ios-sdk#1887 will be great when ever we can adopt it :)

[CodeToTee]

@mikehardy I spent some time today working on the iOS port to v12. Basically, all I had to do was change the .podspec to specify iOS 10 as the minimum supported version, then update FBSDK versions to 12.1.0. I was able to build and test the example app (which only does a login). I reviewed the FBSDK release notes for v12, and perhaps I'm missing something obvious, but I don't see that any of those changes are relevant. Am I missing something? I can put together a PR with the .podspec changes if you like. But...I feel like I'm missing something here. Thoughts/comments appreciated.

[mikehardy]

@NiwotSmitty I'm going to go out on a limb here and guess no one would make a joke with a subject of updating the version of a wrapped library on some random module ;-) but my first thought was you must be joking given the release notes for the v12 semver in the upstream changelog. I was sure it was going to be a pain. But hey, if it works it works - I simply hadn't tried it so it must be that is missing something. I'm literally putting the finishing touches on the example refresh so I've got this all open on my workbench right now. I'll bump the SDK version in the podspec and thanks to your comment I've got high hopes. Thanks!

[mikehardy]

Nope - I had no such luck. There are at least 3 breaking changes to handle - two that seem just naming changes, but one is that getting the advertiser id appears to have disappeared, it did not look horrific but I won't have the time to chase it down, I need to finish up the current work here and turn attention elsewhere for a while

[mikehardy]

Note that the reason changing just the podspec resulted in a successful compile is because of the way the example app works / used to work. Part of what I did in the android v12 forward port was spend a lot of time cleaning up the example app instructions and adding some developer-experience run scripts to make things clear. Most likely the change to the podspec never actually "took" and it was still running against facebook-ios-sdk v11 resulting in a false positive test

This commit in particular should explain it - just a few lines but code shows it better than text:

639535d#diff-58283fff676e13def427e281bce09e853a41c15c1b24bbc141f4da742baf5097R28-R39

...and here is that in action now, in CI making sure CI actually builds against dev code (it did not used to!!) a9506bd#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR112

[CodeToTee]

Thanks for that background. I realized that I failed to mention I had also updated Info.plist to add CFBundleURL configuration stuff to hit the first crash. I'm updating the refresh-example.sh to ensure it really does work, and I'll also verify the correct FDSDK libraries are being loaded. Once I have it cleaned up and I think its working, I'll submit a PR so you can have a look. I'll also review the changes you just sent.

I like the way the refresh-example.sh script works. Using sed is always a little cumbersome to maintain, but it's far easier than updating the example every time React Native is updated.

Thanks for the pointers!

[mikehardy]

Will be excited to see the PR! I've been playing with https://github.com/microsoft/react-native-test-app/ which should handle all of this but haven't had success yet - perhaps in the future. Until then sed it is...

[CodeToTee]

@mikehardy Are the commit links you sent in your repository? GitHub says't they're not in the main repository. Assuming yes, should I base my PR off your fork, or will you be pushing those to the main fork? Thanks.

[mikehardy]

They are in my fork, I usually do fork-based development. I'm just waiting on this CI action to go green then I merge the whole show and release it: https://github.com/thebergamo/react-native-fbsdk-next/runs/4177703478?check_suite_focus=true - so it will be on the main fork / main branch in just a few minutes I hope

[CodeToTee]

Thanks. I'll be out for a few hours, but will be back it it this afternoon. I'll merge those changes into my fork before sending a PR.

[CodeToTee]

@mikehardy Thanks for pushing your updates. You are right that I was actually using 11.2.1 of the FBSDK (due to the package.json file in the example directory). I've addressed that and am working through the build issues now.

[mikehardy]

for what it's worth I was really confounded by the getAdvertiserId thing - that symbol appears to have gone to an internal header that is no longer in the pod at all and I'm not sure it will even be possible to expose that function now, but now I could be missing something 🤷 - it's what really had me stumped about v12. Maybe they can offer guidance if you post a question in the upstream repo, or perhaps you already have it figured out :-)

[CodeToTee]

Re the advertiserID, it's no longer exposed by FBSDK. That field is now kept internal to the module, so I'm going to have to return a null value. I'm also fixing some deprecation warnings. Additionally, there is a type conflict warning in the RCTFBSDKGraphRequestManager module which needs to be addressed, so I'm fixing that one too. I'm going to test with my app, as the example project doesn't use the GraphRequestManager.

[CodeToTee]

BTW...you can assign this one to me if you like.

[mikehardy]

I think it's probably best to just remove the advertising id API then, my philosophy on major changes is "don't be afraid of them, just use semantic versioning and be really really clear in the release notes about what exactly changed and how to handle it". To leave a method in that never returns useful information is false hope I think. What do you think?

(example, the last release: https://github.com/thebergamo/react-native-fbsdk-next/releases)

Thank you so much for working through it - I hate out of date dependencies and this one has been nagging on me for long time personally

[mikehardy]

Sorry for the comment spam: if the android SDK still returns it, then removing the API is a bit brutal, and then sure returning null for the platform is fine. Would still be a breaking change I think and need a docs update to note the platform difference.

[CodeToTee]

Yea, we can certainly remove the API, but what about Android? I don't know how widely it's used in the field. Thoughts?

[CodeToTee]

Ok...I'll leave it in, and I agree its a breaking change that we'll cover in the docs.

[CodeToTee]

@mikehardy I'm reviewing all the changes I've made, and it occurred to me that we can (should?) nuke everything in the RNFBExample directory except the README.md and App.js files. When one runs refresh-example.sh, it deletes the directory then reinitializes it. It also saves android/local.properties, but it looks like that never gets restored, so I suspect its stale. Thoughts? If you agree, I'll incorporate in the PR I'll be sending.

[mikehardy]

I see what you mean, but it is a tradeoff vs out of the box experience, GitHub browsable for people just looking for an example and CI needs it to be there. I think the committed files should stay

Maybe adding the local.properties to git ignore though in parent dir makes sense 🤔

[CodeToTee]

You're right...that's a good point re GitHub browsing; I'll leave the files as-is. I am updating the README to give a little more detail on how to setup the example folder.

That is local.properties used for? It's not in the repository today.

[mikehardy]

local.properties is occasionally useful for what I think of as "poorly configured macOS android dev environments" --> https://github.com/mikehardy/rnfbdemo/blob/218f551c831c3df4d0034b9783459e833ec880e5/make-demo.sh#L198

...you can force the sdk.dir there if you need to (or configure it at all, if things are very poorly configured). Some people have it in there but it is best if it is not there

I appreciate any help updating the example README, the more people can just get in there and be productive, the easier it will be for anyone to help

Thanks!

[github-actions]

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀