Enforce curly braces in eslint config #26886
Conversation
talldan
added
[Type] Bug
talldan
requested review from
ajitbohra,
gziolo,
nerrad and
ntwb
as code owners
|
Size Change: 0 B Total Size: 1.19 MB ℹ️ View Unchanged
|
|
Looks like a few to fix in the codebase as well. I can sort those out tomorrow. |
|
I think we decided to get rid of all overrides to let Prettier do all formatting work. How is Prettier behaving in the setup proposed, will it reformat code to omit curly braces for short lines? |
|
@gziolo Thanks for the context, I wasn't aware that we had some overrides previously. It worked fine in my testing with a long line, no issues with the automated fix. |
|
It's a bit inconvenient for me as a developer that when I have a line like: if ( ! onIndexChange ) return;ESLint errors complaining about the stylistic issue, but running Prettier doesn't fix it. We can discuss it with a wider group of folks on WordPress Slack in |
|
@gziolo Is that using a prettier plugin for your editor? I use the eslint one and it handles both linting fixes and prettier fixes, including the curly braces. |
|
|
I see what you did. I have the regular Prettier integration with VSC. I also tried |
|
I looked into this further, as an option could be to contribute upstream to
With an item in that list being:
As this is a WordPress project, I do think we should be doing our best to enforce WordPress coding standards, which is why I think we should improve this. Is the issue that you feel this is too strict for users of |
|
For the full context, let's include the link to the current coding standards: I raised this issue for discussion during the next weekly WordPress Core JS chat. I doubt that the Prettier project would accept any new flags. We tried several times with WordPress spacing rules and were forced to use |
|
We discussed this PR during in the This PR reports 55 violations that are fixable with the ESLint rule propose:
We didn't come up with any conclusion yet but people expressed that they are fine deferring the formatting to the tool - Prettier. It would be great to stay aligned with PHP coding standards but it feels like they are diverging anyway. |
|
Closing based on feedback. |
Description
On a few PRs recently I've noticed the eslint 'curly' rule that requires curly braces for if, else, for, while, or do statements isn't resulting in errors.
The cause is prettier's eslint config which purposely disabled some rules that could cause a conflict with prettier's formatting.
However, the docs mention that some of these rules can be re-enabled when configured in a particular way:
https://github.com/prettier/eslint-config-prettier#curly
Curly is one of those rules if configured using the 'all' option, which just happens to be what WordPress also requires.
How has this been tested?
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: