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

KSP option for single destination invoke() function #602

Open
stanleyguevara opened this issue Mar 22, 2024 · 3 comments
Open

KSP option for single destination invoke() function #602

stanleyguevara opened this issue Mar 22, 2024 · 3 comments
Labels
enhancement New feature or request feedback needed Extra attention is needed

Comments

@stanleyguevara
Copy link

stanleyguevara commented Mar 22, 2024

Hello, this is rather just a loose idea for improvement, maybe something for 2.0

I think having two equivalent operator fun invoke to navigate to the same destination can be somewhat undesired.
When having navArgsDeletage this is generated:

    override fun invoke(navArgs: SyncScreenNavArgs): Direction = with(navArgs) {
        invoke(title)
    }
     
    public operator fun invoke(
		title: String,
    ): Direction {
        return Direction(
            route = "$baseRoute" + "/${DestinationsStringNavType.serializeValue("title", title)}"
        )
    }

I've found our team is using almost exclusively navArgsDelegate version because click-through-ability is way better.
If I need to see "who navigates to this screen?" I can just ctrl+clik its arguments, no need to go through generated class.
It still happens once i a while somebody uses the wrong option, would be nice to be able to block that.

It could be done by something like:

ksp {
    arg("compose.destinations.singleInvokeOperator", "true")
}

Which would simply make the basic invoke private, if there's navArgsDelegate used.
Let me know what you think. I can do a PR on it if you see no issues with such approach :)

@raamcosta
Copy link
Owner

raamcosta commented Mar 29, 2024

Hi 👋

So you mean you'd want the lib to not generate this?

public operator fun invoke(
		title: String,
    ): Direction {
        return Direction(
            route = "$baseRoute" + "/${DestinationsStringNavType.serializeValue("title", title)}"
        )
    }

@raamcosta raamcosta added the feedback needed Extra attention is needed label Mar 29, 2024
@stanleyguevara
Copy link
Author

Rather to make it private, if navArgsDeletage is defined.

@raamcosta
Copy link
Owner

raamcosta commented Jul 15, 2024

Hi again, sorry for the big delay 🙏

I accept PRs if you want :)
Given this is low priority, I'd really appreciate it!

HINT:
NavArgumentBridgeCodeBuilder#invokeMethodsCode

You can check this config and make the second method private if it is set.
For how configs are handled, check ConfigParser.

And finally you may need to add CodeGenConfig to the constructor of NavArgumentBridgeCodeBuilder to be able to check it.

@raamcosta raamcosta added the enhancement New feature or request label Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feedback needed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants