mrmans0n / compose-rules

Lint rules for ktlint/detekt aimed to contribute to a healthier usage of Compose. Actively maintained and evolved fork of the Twitter Compose rules.
https://mrmans0n.github.io/compose-rules
Other
543 stars 20 forks source link

Using `@Composable` for Custom Modifiers is now the recommended method based on official documents #203

Closed amirkazemzade closed 5 months ago

amirkazemzade commented 6 months ago

As you can see in the official document of Custom Modifiers, the lint for not using @Composable for custom modifiers is now removed:

Note: In previous versions of Compose, we recommended against this approach and suggested using composed {} instead via a lint rule. Now that composed {} is not recommended, the lint rule has been removed.

So I think it is better to also remove it from this ruleset.

Reference of the rule in the repository's website

Reference of the rule in the repository

t-regbs commented 6 months ago

@amirkazemzade I'd argue that the recommended approach for creating performant custom modifiers is Modifier.Node api and hence the ModifierComposable rule is correct and should remain. See official docs here

rafaeltonholo commented 5 months ago

I think this issue is partially correct. I wouldn't say to remove the ruleset, but change it.

The current ruleset states:

In instances where Modifier.Node is not fit for the job, you can still rely on the composed modifier.

However, as @amirkazemzade stated, that has changed in the new official documentation. I agree with you, @t-regbs, that implementing a custom modifier with Modifier.Node API is more performant and should be recommended, however, it also brings some complexity that we often want to avoid, and I believe that is the reason for the current ruleset allows the usage of the composed API.

As the official documentation for implementing custom Modifiers states that the usage of composed should be replaced with Composable Modifier Factory, I'd argue to change the current ruleset to the same enabling the usage of it.

mrmans0n commented 5 months ago

I think It would be cool to add a new rule:

It's completely ok to disable a rule (for a file, or altogether) if you know best. This project is aimed to point people to the "golden path" wherever possible, but sometimes some things are just not practical. That's why we have so much configuration available for many rules, and both ktlint/detekt allow for disabling them if needed.