thebergamo / react-native-fbsdk-next

MIT License
633 stars 165 forks source link

fix(expo): update scheme injection #504

Closed andrejpavlovic closed 3 weeks ago

andrejpavlovic commented 1 month ago

Currently expo plugin injects scheme into all intent filters causing auto verify to break for deep links. See #329 issue for more info.

The fix is to follow latest instructions for configuring Facebook Login for Android essentially removing existing injection of scheme into intent filters and adding <activity android:name="com.facebook.FacebookActivity" .. />

Test Plan: Updated unit test to check that new <activity android:name="com.facebook.FacebookActivity" .. /> has been added to the manifest.

Fixes #329

What this really does is remove support for Android WebViews as per Deprecating Facebook Login support on Android WebViews which should be fine since we are well beyond Facebook SDK 8.2.0 at this point.

JuanRdBO commented 1 month ago

@mikehardy please merge 🙏

andrejpavlovic commented 1 month ago

I haven't tested this with a fresh expo project and I'm not sure if custom tabs need extra configuration as per https://developer.chrome.com/docs/android/custom-tabs/guide-get-started . It's working for me in an existing project just fine though.

JuanRdBO commented 1 month ago

I haven't tested this with a fresh expo project and I'm not sure if custom tabs need extra configuration as per https://developer.chrome.com/docs/android/custom-tabs/guide-get-started . It's working for me in an existing project just fine though.

works for me too

andrejpavlovic commented 1 month ago

I tried it with a new app and it's working fine there as well - so it should be fine to merge this at some point.

mikehardy commented 3 weeks ago

Hey @andrejpavlovic 👋 - thanks for your patience

This looks close to working but I was unable to push to your branch (looks like you didn't grant maintainers rights to push?) so I can't fix the trivial problems

Two things:

With those two items done it should clear CI then I can merge this - both are trivial / mechanical - the PR itself looks fine and since I don't use Expo myself I'll trust you - if you test it and it works, that works for me

Thanks!

mikehardy commented 3 weeks ago

Hope you don't mind - I didn't want to delay further since I'd prefer to get all the work done in this repo in a single batch then let it sit again for a long while - so I opened #512 that contains this as well as the rebase + format fix I mentioned

Should do the trick!

github-actions[bot] commented 3 weeks ago

:tada: This issue has been resolved in version 12.1.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: