sveltejs / eslint-plugin-svelte

ESLint plugin for Svelte using AST
https://sveltejs.github.io/eslint-plugin-svelte/
MIT License
297 stars 35 forks source link

Add option to svelte/prefer-class-directive - apply only when one side is the empty string #689

Closed sdarnell closed 7 months ago

sdarnell commented 7 months ago

Description

The examples for prefer-class-directive show a clear benefit for using the directive form. That is:

BAD : class={current === 'foo' ? 'selected' : ''}
GOOD: class:selected={current === 'foo'}

But there are other scenarios where this rule is far less obviously a good thing, for example:

BAD ?: class="btn {isDanger ? 'btn-danger' : 'btn-primary'}" 
GOOD?: class="btn" class:btn-danger={isDanger} class:btn-primary={!isDanger} 

Personally I prefer the class directive form (which is why i have this rule enabled), and the version preferred by the rule avoids direct interpolation into class, but is longer (more verbose), and uses the condition twice, which in this example is a simple boolean, but might be a bit more complex. The option using the ternary also emphasises that we only use one or the other class name, whereas I'd argue it is a bit less clear that they are exclusive when using directives.

So my feature request is to add a configuration option to only enforce class directives when one of the options is the empty string (the rule example). The goal being that the rule can stay enabled, but be less dogmatic for the pretty common case of 'this or that class'. You may find that it could be moved to the recommended set with the proposed option.

Or is there some other justification which makes the ternary approach a bad idea? If so, I'd suggest adding this reasoning to the rule description.

ota-meshi commented 7 months ago

Thank you for suggesting option. Sounds good to me. Pull requests are welcome.

sdarnell commented 7 months ago

@ota-meshi OK, great, thanks. I've prepared PR #690.