-
-
Notifications
You must be signed in to change notification settings - Fork 565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing formatting on higher-order function with template string loses indentation #241
Comments
Have you tried reproducing this with a straight up Which version of |
Standard itself is at v17.0.0 as well. |
From instrumenting the code a bit, I can see that {
token: Token {
type: 'String',
value: "'I want this to go on a new line so I am making it long, it is true'",
start: 360,
end: 428,
loc: SourceLocation { start: [Position], end: [Position] },
range: [ 360, 428 ]
},
numSpaces: 6,
actualIndent: [ ' ', ' ', ' ', ' ', ' ', ' ' ]
} But with a template string, there's no suggested fix. I assume that's why it inherits the indentation level of the ... but interestingly, it does seem connected to the fact that there's a closure, and that its argument list is so long that it hits the next line. As soon as one of those conditions is no longer true, the template string gets indented. |
So I went further down the rabbit hole and found out that it is an option specific to Standard that's triggering the issue: eslint-config-standard/.eslintrc.json Line 77 in bcd0199
Ignoring This ignore was originally released in Standard v14.0.0 for standard/standard#1176, in an attempt to fix a bug in ESLint. It was subsequently relaxed in v14.0.2 with standard/standard#1385. However, it apparently has undesired side effects. @feross You originally added the exclusion. Do you have an opinion on this? |
Just documenting my ugly workaround for posterity: // .eslintrc.cjs
const standard = require('eslint-config-standard')
const indent = [...standard.rules.indent]
indent[2] = {
...indent[2],
ignoredNodes: indent[2].ignoredNodes.filter(
(expr) => !expr.startsWith('Template')
)
}
module.exports = {
extends: 'standard',
rules: {
indent
}
} |
What version of this package are you using?
v17.0.0
What operating system, Node.js, and npm version?
Node v18.6.0, npm v8.13.2, ESLint v8.20.0, prettier-eslint-cli v6.0.1
What happened?
I apologize in advance if I'm in the wrong place. My (current) best guess is that there's a bug with how this package interacts with ESLint.
I'm using prettier-eslint-cli to format my code. I'm using the config provided in this package's readme and calling
prettier-eslint --write index.js
. At first, I thought this was a prettier-eslint issue, but I found out that it happens when prettier-eslint calls ESLint with the Standard config (and{ fix: true }
).So here goes! When I try to format this code:
(which is already properly formatted), the argument to
console.info
loses its indentation:What did you expect to happen?
The argument should be one nesting level deeper.
Interestingly, it appears to be related to template strings. If I use a regular string, the indentation is as intended. Additionally, ESLint itself complains about indentation if I use a regular string but not if I use a template string.
I assume the issue is either with one of the underlying rules or with ESLint itself, but I haven't dug that deep. What I do know is that ESLint with defaults formats the code more sanely:
This is why I'm filing it as an issue with the Standard configuration.
My best guess is that it's related to a difference in how higher-order (arrow) functions are handled, since by default, ESLint doesn't indent those as much.
I went through the names of all the rules that Standard applies but couldn't identify the one that governs this. I did try to apply the config for the
indent
rule separately but it didn't seem relevant.Are you willing to submit a pull request to fix this bug?
If you tell me where to look. 🙂
The text was updated successfully, but these errors were encountered: