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

TopLevelComposableFunctions and intefaces #29

Open
BraisGabin opened this issue Aug 7, 2023 · 2 comments
Open

TopLevelComposableFunctions and intefaces #29

BraisGabin opened this issue Aug 7, 2023 · 2 comments

Comments

@BraisGabin
Copy link
Contributor

I like the idea of TopLevelComposableFunctions. The composable function shouldn't be declared inside an Activity or anything like that.

But some times I have a Module were I want to display a Component that can't be defined in my module. For example I have a ProductDetail and I want to show Offers. Offers is a different feature so what I do is create an interface like this:

interface OffersProvider {
  @Composable
  fun Offers(id: String, modifier: Modifier = Modifier)
}

Then with the magic of Dagger everything works. The problem is that TopLevelComposableFunctions complains about that @Composable inside an interface.

I see different ways to fix this:

interface OffersProvider {
  fun offers(): @Composable (id: String, modifier: Modifier = Modifier) -> Unit
}

Or don't use OffersProvider and just inject the lambda @Composable (id: String, modifier: Modifier = Modifier) -> Unit.

But I'm not 100% sure if any of those solutions are better than the original one.

What do you think? Should TopLevelComposableFunctions have a flag to allow interfaces? Am I missing other solution?

Sorry if this issue is too philosophical.

@dimsuz
Copy link
Collaborator

dimsuz commented Aug 7, 2023

Hi! 👋

I like the idea of TopLevelComposableFunctions.

Me too! And it even was you who suggested it :)

This way may have some issues:

 fun offers(): @Composable (id: String, modifier: Modifier = Modifier) -> Unit

The thing about this solution is that it would potentially cause unnecessary recompositions as this lambda would be re-created on each call. I got myself puzzled over this recently even asked on StackOverflow, but no one helped me, so I did a little debugging and proved that recompose happens more with this "lambda factory" variant. Still can't wrap my head over what actually happens in my examples.

I have checked my own code and in the current project I also use @Composable functions inside interfaces, but also inside objects (mainly to group them in some scope: I tend to define several slots for "common" components under such groups, e.g MyComponent(header = { MyComponent.Header(...) }, text = { MyComponent.Text(...) }).

So it seems that this tends to be a common thing and you're right about adding allowInterfaces flag and maybe turning it on by default. Maybe also a separate flag for objects (with default == false).

And classes should still be disallowed, because if you do want composables in them, that effectively means turning off this rule completely.

@BraisGabin
Copy link
Contributor Author

Me too! And it even was you who suggested it :)

🤦🤦🤦 I'm getting old. I didn't recall that.

I agree, a configuration to allow interfaces sound good.

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