Comments (22)
any news7
from eslint-plugin-prettier.
Thanks for clarifying. This is actually a bug in eslint-plugin-prettier
and is unrelated to the issue that I mentioned in #65 (comment). The problem is that eslint-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.
from eslint-plugin-prettier.
Not necessarily - it could just be sure to specifically replace entire arrow functions at once, no?
from eslint-plugin-prettier.
What's the differences in user experience and why do you consider them to be worse?
Thanks for calling me out on that, I knew I was being lazy with that handwaving :)
Currently this plugin does the following:
- Grab the entire contents of a JS file and push it into prettier
- Compare the original input with prettier's output. For each block of differences then mark them as changes in eslint.
This means that if you have a file like:
const a = 0;
const b=0;
const c = 0;
const d=0;
const e = 0;
it will report changes (adding the spacing around the equals) on lines 2 and 4.
const a = 0;
-const b=0;
+const b = 0;
const c = 0;
-const d=0;
+const d = 0;
const e = 0;
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:
const a = 0;
-const b=0;
-const c = 0;
-const d=0;
+const b = 0;
+const c = 0;
+const d = 0;
const e = 0;
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 :)
from eslint-plugin-prettier.
Are there any plans to fix this?
from eslint-plugin-prettier.
I wonder if
arrow-body-style
andprefer-arrow-callback
extend the fix range will fix the issue?
Or maybe we can introduce a prettier/arrow-body-style
and prettier/prefer-arrow-callback
rule?
from eslint-plugin-prettier.
Is the underlying issue from the
prefer-arrow-callback
fixer or the prettier plugin fixer?
It could also be how ESLint applies fixes.
from eslint-plugin-prettier.
Related: eslint/eslint#7928
tl;dr: Autofixes from rules can, on rare occasions, conflict with each other. Rules can work around the issue by coarsening their fixes to ensure that the "fix range" contains the entire range of text that caused the particular problem to be reported, rather than just the actual text replacement.
For convenience, could you paste the output of running ESLint without --fix
when those two rules are enabled? I think this might be a problem with the core prefer-arrow-callback
rule, but I'm not sure.
from eslint-plugin-prettier.
Here is the output of running ESLint
without --fix
on issue2.js
.
╰─$ yarn run lint
yarn run v1.1.0
$ eslint issue2.js --max-warnings 0
/Users/ismailsyed/test/prettier-eslint-config-invalid-code/issue2.js
2:10 error Replace `isTrue·&&·[0,1,` with `(⏎····isTrue·&&⏎····[0,·1,·` prettier/prettier
2:32 error Unexpected function expression prefer-arrow-callback
3:5 error Insert `··` prettier/prettier 4:1 error Replace `··}` with `····})⏎··` prettier/prettier
✖ 4 problems (4 errors, 0 warnings)
4 errors, 0 warnings potentially fixable with the `--fix` option.
With --fix
, I see parsing errors:
from eslint-plugin-prettier.
I believe the only way to fix this - changing the plugin so every change is batched into a single "fix" - results in a much worse user experience than the current behaviour of multiple smaller fixes in the common case of where you have multiple small and independent fixes. I don't think the tradeoff is worth it so I think this will end up remaining unfixable.
from eslint-plugin-prettier.
I think there’s one more way to fix this – taking a really deep dive into the ESLint autofix code and trying to fix the problem at its core. Sounds harder though.
from eslint-plugin-prettier.
I believe the only way to fix this - changing the plugin so every change is batched into a single "fix" - results in a much worse user experience than the current behaviour of multiple smaller fixes in the common case of where you have multiple small and independent fixes. I don't think the tradeoff is worth it so I think this will end up remaining unfixable.
What's the differences in user experience and why do you consider them to be worse?
from eslint-plugin-prettier.
I opened a similar issue in React Boilerplate.
from eslint-plugin-prettier.
Is it possible to know when we are running in auto fix mode and decide to create a single fix instead of individual fixes? This way the DX of showing the smallest diffs will still exist when running without --fix for example with the eslint vscode plugin, but when it comes to auto fixing all rules there will only be one fix to apply so it can't conflict with other fixes.
from eslint-plugin-prettier.
I wonder if arrow-body-style
and prefer-arrow-callback
extend the fix range will fix the issue?
from eslint-plugin-prettier.
That's what I was thinking, but I don't know if it works.
from eslint-plugin-prettier.
I think I found another rule that can cause problems:
getInitialState is a React lifecycle method, and should not be an arrow function or in a class field. Use an instance method instead.eslintreact/no-arrow-function-lifecycle
export default class Root extends Component {
getInitialState = () => ({
errorImporting: null,
errorParsing: null,
errorUploading: null,
file: null,
fromExtension: false,
importSuccess: false,
isImporting: false,
isParsing: false,
isUploading: false,
parsedResults: null,
showLongRunningMessage: false,
});
}
Here's eslintrc and package.json:
https://gist.github.com/devinrhode2/9dc0e0debee7d2dcdf1afbb4af62fee3
from eslint-plugin-prettier.
Why would that rule cause problems? Prettier is about formatting; that rule is about actual runtime semantics.
from eslint-plugin-prettier.
Produces this code:
export default class Root extends Component {
getInitialState() { return {
errorImporting: null,
errorParsing: null,
errorUploading: null,
file: null,
fromExtension: false,
importSuccess: false,
isImporting: false,
isParsing: false,
isUploading: false,
parsedResults: null,
showLongRunningMessage: false,
}; }); // Extra trailing paren
}
from eslint-plugin-prettier.
Probably this is what it should produce:
export default class Root extends Component {
getInitialState() {
return {
errorImporting: null,
errorParsing: null,
errorUploading: null,
file: null,
fromExtension: false,
importSuccess: false,
isImporting: false,
isParsing: false,
isUploading: false,
parsedResults: null,
showLongRunningMessage: false,
};
}
}
I had to manually delete trailing paren to get it to format correctly
from eslint-plugin-prettier.
@devinrhode2 that sounds like a bug in the rule's autofix; i'd appreciate you filing an issue on eslint-plugin-react :-)
from eslint-plugin-prettier.
yeah it actually seems totally unrelated to prettier. Tried disabling all the prettier stuff, run just eslint autofix, and still happening.
from eslint-plugin-prettier.
Related Issues (20)
- Synckit seems incompatible with Node 14.18.1 HOT 2
- use es6's import syntax to import eslint-plugin-prettier/recommended
- [feat] fallback `ignorePath` to `.eslintignore` if `.prettierignore` not found HOT 3
- Setting "trailingComma" to "all" does not work HOT 4
- Importing `eslint-plugin-prettier/recommended` in ESM syntax results in a lint error. HOT 1
- case study: `eslint-plugin-prettier-vue`
- Removal of eslint-config-prettier from peerDependencies HOT 1
- Prettier enforces adding unnecessary parentheses HOT 1
- Linebreak error when using with `nest new` project HOT 13
- Unable to Deploy App Due to ESLint/Prettier Errors HOT 1
- eslint-plugin-prettier version `3.1.1` and newest prettier version `3.2.5` mismatch causing issues HOT 2
- TypeError: prettier.resolveConfig is not a function HOT 2
- Delete `0` eslint(prettier/prettier) HOT 1
- 本地部署报错 3:17 error Delete `⏎⏎` prettier/prettier ✖ 1 problem (1 error, 0 warnings) 1 error and 0 warnings potentially fixable with the `--fix` option. HOT 1
- feat: support other HTML related ESLint plugins HOT 1
- Customize flat config
- Does not consider overrides defined in .prettierrc
- issue with typed configs from typescript-eslint & eslint9 --fix
- Error: Unexpected key "meta" found. HOT 1
- cause endless eslint running HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from eslint-plugin-prettier.