Fix: Support prettier v1.6.0 config (#46) #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kombucha
commented
Sep 17, 2017
| @@ -342,12 +337,31 @@ module.exports = { | |||
| } | |||
| } | |||
|
|
|||
| if (prettier) { | |||
| prettier.clearConfigCache(); | |||
There was a problem hiding this comment.
Not too sure about this but it proved necessary when using vscode, otherwise changes to the .prettierrc were not getting picked up.
There was a problem hiding this comment.
This makes ESLint a lot slower. I have a partial fix here, but it would be good to figure out a better way to do this: #303
lydell
reviewed
Sep 17, 2017
README.md
Outdated
| @@ -55,7 +55,7 @@ Then, in your `.eslintrc.json`: | |||
|
|
|||
| * The first option: | |||
|
|
|||
| - Objects are passed directly to Prettier as [options](https://github.com/prettier/prettier#api). Example: | |||
| - Objects are passed directly to Prettier as [options](https://github.com/prettier/prettier#api). This plugin will merge and override any config set with a `.prettierrc` file (only for prettier >= 0.17). Example: | |||
There was a problem hiding this comment.
- Here's a better link for options: https://github.com/prettier/prettier/#options
- You could change "prettier" to "Prettier" for consistency :)
- Typo:
0.17→1.7.0 - I think it is clearer to say
[Prettier configuration file](https://github.com/prettier/prettier/#configuration-file)instead of`.prettierrc` file. Not sure if we need to mention the version at all. - The sentence about config file support applies both to the object option and the
"fb"option. I suggest moving it outside of all the bullet lists to the very start of the "Options" section.
There was a problem hiding this comment.
I fixed the typos, changed the link and put the sentence about config files outside the sublist, but at the end. WDYT?
kombucha
force-pushed
the
prettierrc-support
branch
from
77799d5
to
2aa0cd3
Compare
not-an-aardvark
requested changes
Sep 17, 2017
There was a problem hiding this comment.
Thanks for the PR! This implementation looks good to me, but can you add tests?
One way to do that would be to put .prettierrc files in a tests/fixtures directory, then add a filename property to an object like this which is also in the same subdirectory, so that the fixture config will be found.
kombucha
force-pushed
the
prettierrc-support
branch
from
2aa0cd3
to
d09f3f1
Compare
kombucha
force-pushed
the
prettierrc-support
branch
from
d09f3f1
to
34114f4
Compare
|
@not-an-aardvark I added three tests :
I had to update the yarn.lock, otherwise tests are failing because Hope that's ok ! |
not-an-aardvark
approved these changes
Sep 18, 2017
This was referenced Sep 22, 2017
lencioni
added a commit
to lencioni/eslint-plugin-prettier
that referenced
this pull request
Jun 11, 2020
At Airbnb we've noticed that the Prettier ESLint rule is extraordinarily slow. When running ESLint with `TIMING=1`, Prettier is two orders of magnitude slower than our other slowest rules (which come from eslint-plugin-import mostly). I spent some time investigating this today. I ran the Chrome DevTools profiler while running ESLint on a directory with 480 files in it. Looking at the flame charts, I noticed that this plugin ended up calling Prettier's pluginSearchDirs.map.pluginSearchDir for every file that was being linted, even though it was being memoized. Through some logging, I determined that the Prettier cache was being cleared between every file, and narrowed it down to this line, which was added here: prettier#55 (comment) It looks like this cache busting was added to make it so long-running ESLint processes (e.g. for vscode-eslint) would be able to pick up changes to prettierrc files without having to reload the process. Thankfully, we don't use a prettierrc file at Airbnb right now, in favor of putting our Prettier config inline with our ESLint config. So the quick performance fix for us is to simply skip the cache busting when this option is not enabled. In my profiling, this reduces the time spent in ESLint's verifyAndFix when running on the same 480 files from 34 seconds to 26 seconds, for a total savings of 8 seconds. For folks who are using prettierrc files, this is still pretty crappy, so it would be great to find a better way to do this. Unfortunately my knowledge of the inner workings of vscode-eslint and ESLint are not enough to know if there might be a better way to do this--e.g. maybe there's some ESLint option that we can respect here?
BPScott
pushed a commit
that referenced
this pull request
Jun 14, 2020
At Airbnb we've noticed that the Prettier ESLint rule is extraordinarily slow. When running ESLint with `TIMING=1`, Prettier is two orders of magnitude slower than our other slowest rules (which come from eslint-plugin-import mostly). I spent some time investigating this today. I ran the Chrome DevTools profiler while running ESLint on a directory with 480 files in it. Looking at the flame charts, I noticed that this plugin ended up calling Prettier's pluginSearchDirs.map.pluginSearchDir for every file that was being linted, even though it was being memoized. Through some logging, I determined that the Prettier cache was being cleared between every file, and narrowed it down to this line, which was added here: #55 (comment) It looks like this cache busting was added to make it so long-running ESLint processes (e.g. for vscode-eslint) would be able to pick up changes to prettierrc files without having to reload the process. Thankfully, we don't use a prettierrc file at Airbnb right now, in favor of putting our Prettier config inline with our ESLint config. So the quick performance fix for us is to simply skip the cache busting when this option is not enabled. In my profiling, this reduces the time spent in ESLint's verifyAndFix when running on the same 480 files from 34 seconds to 26 seconds, for a total savings of 8 seconds. For folks who are using prettierrc files, this is still pretty crappy, so it would be great to find a better way to do this. Unfortunately my knowledge of the inner workings of vscode-eslint and ESLint are not enough to know if there might be a better way to do this--e.g. maybe there's some ESLint option that we can respect here?
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm not sure if anybody was working on this issue so I made this PR !
Tests are passing, and I tested it on a small example project.
Cheers