fix: switch to @stylistic/exp-list-style to resolve array edge-cases
#1203
Conversation
susnux
commented
Nov 6, 2025
susnux
commented
- closes [wip] fix(codestyle/array): handle edge case with multi line array #1166
|
IDK, are there options to keep/enforce multiline there, e.g. by line length? |
The old value was not multi line (considered by the fixer) because brackets and first / last item are on the same line. |
|
This? https://eslint.style/rules/list-style#maxitems Looks nice, I'd consider great if limited to 3 items, maybe? |
Pushed a commit |
Maybe exclude functions |
I'm voting for the previous behavior. If a developer makes it multiline — keep multiline. If single-line - keep single line. The decision here doesn't depend on the number of items but on the nature of the array. |
|
In the referenced PR the tests were adjusted to be more real life like, with som explanations |
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
|
@ShGKme I added the tests you created to this PR see the two commits to see how it is changed |
| const LATIN_VOWELS = [ | ||
| 'a', | ||
| 'e', | ||
| 'i', | ||
| 'o', | ||
| 'u', | ||
| ] |
There was a problem hiding this comment.
Do we really need diff-safe here? I don't think new vowels are expected to be added 👀
There was a problem hiding this comment.
IMHO its better readable than one-liner.
But at this point I do not care, either we merge this or keep current behavior or someone propose a different consistent solution. As long as we finally can publish this - IMHO we can also change this later...
There was a problem hiding this comment.
someone propose a different consistent solution
What's wrong with what was already proposed?
- Only check for consistency: either single-line array, or multi-line, forbid mixed
- Check for bracket
[]position if possible - Semantic-related decisions are on the developers. No forced multiline based on syntax without semantics.
The tests copied to this PR were carefully (spending quite much time) developed to follow the proposal: allow developers to decide which format to use to have the readability and simplicity as the priority, keeping the consistency. With the explanation in the comment to make a test not only a snapshot but also a specification.
This is also why vowels in the test where a single line. If we say "multi-line is better for diff-safe", then there is no reason for vowels to be multi-line. Unless we are worried about a case of inventing a new vowel in the English language.
It is not something that can be decided via maxItems. Number 3 has no semantics.
In this test sampleUsers are 3 items, but it is a dynamic array that may have items changed anytime. Same for MUTE_NOTIFICATIONS_USER_STATUSES with a single item.
Let's allow developers to do what is better in a specific case instead of sacrificing anything for the sake of auto-fixed consistency. Is having a diff-safe array every time worth the profit of the diff-safe arrays?
My motivation is to make ESLint a friendly assistant to a developer.
There was a problem hiding this comment.
My motivation is to make ESLint a friendly assistant to a developer.
Same but at the same time enforce consitent style as written in the docs so that if you open any Nextcloud app you do not see 10 different styles but one code style across all apps.
There was a problem hiding this comment.
As said I do not care that much at this moment anymore as we need a release and that best case couple of month ago 😉
So if you have a better solution ready I happily review - or we postpone this rule to v9.0.1 - should not be breaking any already migrated app so weaken the rule after release.
There was a problem hiding this comment.
Same but at the same time enforce consitent style as written in the docs so that if you open any Nextcloud app you do not see 10 different styles but one code style across all apps.
If ESLint breaks my code readability skipping semantics, it doesn't sound as a friendly assistant
Writing a short stable array in a single line doesn't sound like adding 9 more different styles. It is still a consistent array.
We have many other cases where developers can decide if they want the code to be single or multiline, for example, imports.
They can also decide between using .forEach method and for-of statement.
Do you want to specify a single way in every such cases for consistensy?
There was a problem hiding this comment.
They can also decide between using .forEach method and for-of statement.
Do you want to specify a single way in every such cases for consistensy?
No.
While .forEach and for ... of could also be enforces this is different as this can have different use cases.
Also this gets quite opinionated at some point.
I think that's the reason why we only have some general code structuring and let the rest be decided by the developers.
Writing a short stable array in a single line doesn't sound like adding 9 more different styles. It is still a consistent array.
Its consistency within one array vs consistency of all arrays.
I can also reverse the question: What is the problem of having one style everywhere?
I personally also was more in favor of one-line arrays for simple arrays.
But now I am quite used to the multi line arrays because they allow "seeing" the code vs having to "read" the code.
The decision here doesn't depend on the number of items but on the nature of the array.
I do not agree - if the array is quite long its much harder to quickly read all elements.
If ESLint breaks my code readability skipping semantics, it doesn't sound as a friendly assistant
Well but this is not caused by ESLint but by not following code style in the first place.
There was a problem hiding this comment.
But as said - I am fine with leaving this for know.
The biggest problem is having it not released as everything can be changed later if we discover issues with it and someone proposes a working fix.
We already have apps released with it for Vue 3 in Nextcloud 32 but depend on a "rc" version - which should be fixed soon. We can always add new rules in a new minor version or remove rules if we see that it breaks things.
But for now it seems like we had not real problems migrating even big apps like photos.
