eslint-plugin-prettier: prefer-arrow-callback fix conflicting with prettier fix
Edit by @lydell: TL;DR We recommend turning off these rules for the time being:
{
"rules": {
"arrow-body-style": "off",
"prefer-arrow-callback": "off"
}
}
What version of eslint
are you using?
v4.9.0
What version of prettier
are you using?
v1.7.4
What version of eslint-plugin-prettier
are you using?
v2.3.1
Please paste any applicable config files that you’re using (e.g. .prettierrc
or .eslintrc
files)
https://github.com/ismail-syed/prettier-eslint-config-invalid-code
What source code are you linting?
function foo() {
return isTrue && [0,1,2].map(function(num) {
return num * 2;
});
}
What did you expect to happen?
The code above should be formatted as per prettiers config and also should adhere to that prefer-arrow-callback
fix
What actually happened? Invalid code was generated, closing parenthesis is missing on the return statement.
function foo() {
return (
isTrue &&
[0, 1, 2].map((num) => {
return num * 2;
});
}
Is the underlying issue from the prefer-arrow-callback
fixer or the prettier plugin fixer?
About this issue
- Original URL
- State: open
- Created 7 years ago
- Reactions: 5
- Comments: 22 (9 by maintainers)
Commits related to this issue
- (patch): Resolves Prettier and eslint integration: https://github.com/prettier/eslint-plugin-prettier/issues/65 — committed to jamiemagique/eslint-config-pavo by magique 6 years ago
- fix(prettier): fix prettier eslint autofix conflict https://github.com/prettier/eslint-plugin-prettier#arrow-body-style-and-prefer-arrow-callback-issue https://github.com/prettier/eslint-plugin-prett... — committed to Yoctol/eslint-config-yoctol-base by tw0517tw 3 years ago
- fix(prettier): fix prettier eslint autofix conflict https://github.com/prettier/eslint-plugin-prettier#arrow-body-style-and-prefer-arrow-callback-issue https://github.com/prettier/eslint-plugin-prett... — committed to Yoctol/eslint-config-yoctol-base by tw0517tw 3 years ago
any news7
Thanks for clarifying. This is actually a bug in
eslint-plugin-prettier
and is unrelated to the issue that I mentioned in https://github.com/prettier/eslint-plugin-prettier/issues/65#issuecomment-337017965. The problem is thateslint-plugin-prettier
is providing separate “fixes” for each problem it reports, and the fixes are dependent on each other (in this case, one fix adds an opening paren, and another fix adds a closing paren). However, ESLint doesn’t guarantee that it will be able to apply any given fix (e.g. if a fix overlaps with a fix from another rule).It would probably be pretty difficult to determine which fixes need to be applied together as part of
eslint-plugin-prettier
. A simpler solution might be to just provide one “fix” which replaces the entire source text.Not necessarily - it could just be sure to specifically replace entire arrow functions at once, no?
Thanks for calling me out on that, I knew I was being lazy with that handwaving 😃
Currently this plugin does the following:
This means that if you have a file like:
it will report changes (adding the spacing around the equals) on lines 2 and 4.
This “find the smallest block of changes” logic gives a nice DX experience as it means the smallest change a user needs to make is shown.
The naive shotgun approach that would solve the problem, but give a crappy DX is to not try and split out changes into atomic parts, and instead batch all the changes into one big one. In this case the suggested fix for the above code would be:
Note that the
c
line is caught up in the change too despite it not requiring any modifications. This seems kinda reasonable in a small example but what if you had a 100 line file and the first change requested by prettier was on line 2 and the last change requested was on like 70. This algorithm would say that lines 2-70 would all need to change, despite most of that content being identical. Making two mistakes and large swaths of your file getting a red wavy line of whoops underneath it is a bad experience as it makes it very hard to tell what actually got changed.While this approach is simple to implement the potential for making hard to understand suggestions that make it untenable IMO.
The 3rd approach is @ljharb’s - a diff mechanism that understands JS and can block changes within a function into a single change. This would solve the problem but a very quick search came up with no drop-in solution available at the moment, and my gut thinks implementing a diffing algorithm that understands JS logic would be complex and much slower than the current algorithm.
If there’s a way that this can happen without adding too much complexity to our codebase and doesn’t have a major performance impact then I’d be interested in reviewing a PR but I don’t have much interest in attempting to solve this personally as my gut thinks it’s going to be a lot of effort and a lot of complexity for worse overall performance for a problem that only crops up occasionally. Happy to be proved wrong though 😃
Or maybe we can introduce a
prettier/arrow-body-style
andprettier/prefer-arrow-callback
rule?Are there any plans to fix this?