jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.97k stars 2.77k forks source link

`jsx-newline`: support for checking newline at the start and end of children #3678

Open burtek opened 8 months ago

burtek commented 8 months ago

Fixes #3663

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (3730edb) 97.65% compared to head (1b3be40) 97.74%. Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3678 +/- ## ========================================== + Coverage 97.65% 97.74% +0.08% ========================================== Files 132 132 Lines 9397 9405 +8 Branches 3445 3446 +1 ========================================== + Hits 9177 9193 +16 + Misses 220 212 -8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ljharb commented 8 months ago

prevent: true or false obv need to continue working.

I like the idea of effectively deprecating the boolean config, and instead allowing an enum of 'ignore' (false), 'siblings' (true), 'adjacent' (lexically adjacent, ie everything) - that way if someone wants different options we can easily add them later, but for now we're only adding adjacent to cover the likely reading of the existing docs. Thoughts?

burtek commented 8 months ago

Kinda feel like deprecating (while still supporting) old config, but making new config be:

{
    between: 'force' | 'force-one' | 'prevent' | 'ignore' // between children
    adjecent: 'force' | 'force-one' | 'prevent' | 'ignore' // before first child and after last child
}

force - must be at least 1 empty line force-one - must be exactly 1 empty line prevent - must be 0 empty lines ignore - ignore case

Obviously if both are ignore then rule does nothing.

Those properties might probably be called something else (better?).

ljharb commented 8 months ago

"between" seems weird for a parent/child relationship. Are you just trying to think of possible combinations, or do you have use cases?

A linter saying "adjacent" usually means lexically, not like "siblings" in terms of the AST graph.

burtek commented 8 months ago

So I want to distinguish all four options I saw in different projects:

<parent>
    <childA />
    <childB />
</parent>
<parent>
    <childA />

    <childB />
</parent>
<parent>

    <childA />

    <childB />

</parent>
<parent>

    <childA />
    <childB />

</parent>

Easiest way would be allow configuring the rule for newlines between siblings separately from newlines before first and after last sibling. between and adjecent are names that came to my mind, but as non-native speaker I'm not sure what those names should actually be.

And the choice between force, force-one, prevent and ignore is just all the possible options that came to my mind, based on current config scheme

ljharb commented 8 months ago

OK, let's aim for those 4 options, with an eye for future extensibility.

That means two keys, one for siblings and one for directly inside of a parent, each of which a "must have a newline" value, and a "must not have a newline" value. I suppose we could also have "ignore", too.

What about betweenSiblings and aroundChildren?

(allowMultilines would apply to all permutations)

burtek commented 8 months ago

Sounds good