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

Documentation or nullability annotation of FirebaseDynamicLinks.getDynamicLink() intent parameter wrong #2336

Closed
ubuntudroid opened this issue Jan 18, 2021 · 7 comments

Comments

@ubuntudroid
Copy link

The documentation of

public abstract Task<PendingDynamicLinkData> getDynamicLink(@NonNull Intent intent);

states, that the intent parameter

can be null if the intent does not include the dynamic link. A non-null intent is necessary only when the app is launched directly using the dynamic link, such as when using App Links.

However the parameter is annotated with @NonNull. So either the documentation or the annotation is wrong and needs adaptation.

@google-oss-bot
Copy link
Contributor

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@rlazo
Copy link
Collaborator

rlazo commented Jan 18, 2021

Hi @ubuntudroid, thanks for your report! The documentation is wrong, since the method implementing that interface does in fact expect the value to be non-null

public Task<PendingDynamicLinkData> getDynamicLink(@NonNull final Intent intent) {

I'll follow up here when we have a fix ready.

@rohandandavati
Copy link
Contributor

b/178000308

@davidmigloz
Copy link
Contributor

What is the expected usage when you only care about the captured dynamic link after install? (e.g. because you have your own deep linking mechanism to handle app links that doesn't rely on Firebase Dynamic Links)

It'd good to have a getDynamicLink() without any parameters for that case (or allow to pass null as the documentation was suggesting before).

@eldhosembabu
Copy link
Contributor

Added a fix here: #2629

@mikehardy
Copy link

This just bit us on react-native-firebase. We were coded up to wrap the API per docs, but a user took a null pointer exception because the implementation expects non-null

invertase/react-native-firebase#5662

mikehardy added a commit to mikehardy/flutterfire that referenced this issue Aug 30, 2021
…micLink

firebase-android-sdk requires a `@NonNull` Intent argument to the `getDynamicLink(Intent)`
call, despite the documentation indicating otherwise

Related firebase/firebase-android-sdk#2336
Related invertase/react-native-firebase#5662
@eldhosembabu
Copy link
Contributor

#2629 is merged. Expect the fix to be rolled out in upcoming releases. So closing this issue now.

@firebase firebase locked and limited conversation to collaborators Oct 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants