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

[DEEPLINK][2.1.0] Request to bring back withDeepLink() #639

Closed
extmkv opened this issue May 16, 2024 · 9 comments
Closed

[DEEPLINK][2.1.0] Request to bring back withDeepLink() #639

extmkv opened this issue May 16, 2024 · 9 comments

Comments

@extmkv
Copy link
Contributor

extmkv commented May 16, 2024

Hi,

During the migration to V2 of the library, we noticed that you mentioned that withDeeplink was removed.

These APIs were removed on v2. There are new and better ways to achieve the same thing.

In our case, we can't achieve the same without it. Our app has different URI schemas by flavors that only the parent module has visibility and injects on our DI. We were building our own graphs and adding the deeplinks to the destinations over there by using the provided schema from DI. Not important, but we had an extension for it, and it is reusing the route + schema.

Unfortunately, we can't achieve the same just by using the annotation, our feature modules don't have the visibility of the schemas so using a constant is not an option, since the schema is provided via DI.

Extension:

public fun <T> DestinationSpec.withDefaultDeeplink(scheme: DeeplinkScheme): DestinationSpec {
    return withDeepLink { uriPattern = "${scheme.value}://${this@withDefaultDeeplink.route}" }
}

Example of a graph:

 destinations =
                listOf(
                    ChatSupportScreenDestination.withDefaultDeeplink(schema),
                    ChatRateScreenDestination,
                ),

Any possibility of bringing this back? 🙏🏽

@raamcosta
Copy link
Owner

Hey 👋

So essentially you need to be able to set deep links at runtime. The problem is not where it is set, since there's ways to override things like deep links when importing destinations or graphs. The problem is that you cannot use annotations coz those can only take constants.

Is that it?

@extmkv
Copy link
Contributor Author

extmkv commented May 16, 2024

Yes, that's exactly the problem.

@raamcosta
Copy link
Owner

raamcosta commented May 16, 2024

Ok.
What do you think of an API like this:

DestinationsNavHost(
    //...
) {
    // next to other "runtime" setup we can have with Compose Destinations
    MyDestination addDeepLink { uriPattern = "URI PATTERN" }
}

You could create an extension function too if you prefer:

public fun ManualComposableCallsBuilder.addDeepLink(destination: DestinationSpec, scheme: DeeplinkScheme) {
    return destination addDeepLink { uriPattern = "${scheme.value}://${destination.route}" }
}

But you would need to call it inside the DestinationsNavHost trailing lambda.

Haven't even tried to code this, but just from the top of my head, it should be possible.

@extmkv
Copy link
Contributor Author

extmkv commented May 16, 2024

I think that will solve our problem, but tbh, I would prefer to be able to add it to the destination instead, so everything can be together on the graph.

@extmkv
Copy link
Contributor Author

extmkv commented May 16, 2024

Just realized that by doing your approach we can start generating instead of building our owns.

@raamcosta
Copy link
Owner

So, is that good or bad? I'm not sure I understand what you mean 🤔

@extmkv
Copy link
Contributor Author

extmkv commented May 16, 2024

It's better. We had to have the custom graphs just to include deeplinks, by using that approach it will be not necessary because the deeplinks will be added to the DestinationsNavHost trailing lambda.

@raamcosta
Copy link
Owner

Ahh, yes. With v2 there should be no reason for implementing NavGraphSpec of core library manually.

@raamcosta
Copy link
Owner

Went with above approach. It is included in beta07 that is currently building, so probably up in about 30min or so.
That version will also contain your PR changes (thanks for that! 🙏 )

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

2 participants