Skip to content
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

[DynamicLinks] Update Docs for v6 #235

Open
builtbyproxy opened this issue Nov 22, 2019 · 15 comments
Open

[DynamicLinks] Update Docs for v6 #235

builtbyproxy opened this issue Nov 22, 2019 · 15 comments

Comments

@builtbyproxy
Copy link

builtbyproxy commented Nov 22, 2019

Hello

Could someone who knows how, please update step 5 of the appDelegate.m stage for DynamicLinks to fit v6?

Update your AppDelegate.m file: as seen here

If this has been deprecated, could someone please point me to where I may go to see how to do this in V6?

Thank You!

@mikehardy
Copy link
Collaborator

@builtbyproxy there's an edit button on the top right of each documentation page and github does walk you through the process of proposing on update? I haven't been through what you're talking about so someone that has been through it is best to propose a change - give it a shot 💪

@builtbyproxy
Copy link
Author

builtbyproxy commented Nov 22, 2019

Ah there's been a miscommunication. I don't actually know what to put here haha, I was hoping someone who does know what to put there could edit it, specifically the react-native-firebase team if they see this

Thanks for the super quick response though!

@mikehardy
Copy link
Collaborator

Oh I see sorry :-) - I think the whole document will likely still apply except the "add the pod" part. v6 takes care of the Pod dependencies for you under the covers

@builtbyproxy
Copy link
Author

builtbyproxy commented Nov 22, 2019

The pod linking is working great, just having some troubles with importing. Running @react-native-firebase/dynamic-links: 6.0.4 I no longer have RNFirebaseLinks.h for Step 5, i.

Then it breaks down in iii. with return [[RNFirebaseLinks instance]

The 'fix' is to change RNFirebaseLinks to

#import "RNFBDynamicLinksModule.h"

however if I replace the return with RNFBDynamicLinksModule it gives me errors


Edit: It seems if I restart my computer the error no longer comes up...
Super weird, cleared my Pods, Build, everything haha

Oh well, leave this here for somone else to come across

Thanks for the rubberduck @mikehardy

@mikehardy
Copy link
Collaborator

You may have been bitten by stale DerivedData - I swear by npx react-native-clean-project and cleaning the iOS build as it wipes DerivedData as well. If I could have all those hours back....

Glad you're moving forward

@builtbyproxy
Copy link
Author

builtbyproxy commented Nov 22, 2019

Ah yikes actually the errors are back :( It had to re-index in xcode
error: no known class method for selector 'instance'

Any ideas?

I'll give this react-native-clean-project a go 👍

@mikehardy
Copy link
Collaborator

I'm not sure any of that is necessary anymore? https://invertase.io/oss/react-native-firebase/v6/dynamic-links/ios does not mention it, and normally I take inspiration from the test project and I likewise see nothing in there like that - it may be all auto-linked in now? https://github.com/invertase/react-native-firebase/blob/master/tests/ios/testing/AppDelegate.m

@builtbyproxy
Copy link
Author

builtbyproxy commented Nov 22, 2019

Yeah im a bit lost on all of this because it isn't mentioned linking automatically the use cases like we have defined in our AppDelegate.m

I also can't find anywhere they mention to explicitly remove it though.

In this use case of mine it seems pretty important
image

@mikehardy
Copy link
Collaborator

Agreed there - I think the "happy path" through tests for dynamic links (that works for the react-native-firebase module in it's tests, that is) is a default openURL that needs no override. But as I'm integrating google sign-in now (and FB next) that doesn't fly, no.

So - realizing that you've got multiple possible listeners for the URL open - here's where I am unfortunately less prescriptive. At the same time, I'm still not sure it's necessary - I'm not an ObjC wizard but it looks like it handles things well in getInitialLink here? https://github.com/invertase/react-native-firebase/blob/master/packages/dynamic-links/ios/RNFBDynamicLinks/RNFBDynamicLinksModule.m

@builtbyproxy
Copy link
Author

Ah very interesting! I was wondering where this suggestion of FIRDynamicLinks comes from...

So you suggest just remove the above code I showed and see how it goes?

I'll give it a go haha

@harrisrobin
Copy link

Also having trouble with this, @builtbyproxy 's solution did not work for me:

image

Would be great if we get an example or updated docs on how Appdelegate.m should look like

@mikehardy
Copy link
Collaborator

I can't promise a timeline but I am nearly done integrating social auth and apple sign in and dynamic links is next. I'll be happy to collaborate with everyone here and in the related issues to come up with a great solution, fuzzbust through any of the issues and update the docs. During the apple sign in work, it helped to share code via gists, so if people post what they currently have, and which cases work (or don't work) in gists and link them in we can maybe get something together that handles all the cases?

And for concretness, I think the cases are:

For each of android and ios:

  • if app not installed, link results in installed app and initialLink() containing information?
  • if app is installed but not running, initialLink() will contain information if link is clicked? and there is a sub-case for facebook right now as that may be independently broken?
  • if app is installed and running, onLink() is called?

apologies if the API names aren't perfect but those are the general cases I'm aware of?

@builtbyproxy
Copy link
Author

builtbyproxy commented Dec 8, 2019

@harrisrobin I've fixed that import error before
Head into the RNFBSharedUtils.h and change the import from #import RCTBridgeModule.h to #import <React/RCTBridgeModule.h>

I then ran a patch-package to ensure posterity across CI and such

Hope that helped

@harrisrobin
Copy link

thank you @builtbyproxy , I'll try that.

@builtbyproxy
Copy link
Author

builtbyproxy commented Dec 9, 2019

Let me know! I'm online for another hour or so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants