-
-
Notifications
You must be signed in to change notification settings - Fork 285
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
Slow when linting a single file #212
Comments
I noticed running Maybe there's an issue with the way I'm using Here's the package.json I'm using.
|
I got This means the problem is somewhere between these:
|
Not sure if this is related but after the last update for |
Ignore that it seems somewhere in my testing I removed xo from the deps section. It'd be nice if xo ran as long as there was an "xo" field in the |
|
Could it not check if "xo" is a field though? If there's a "xo" field in the |
@OmgImAlexis Not sure I want to support that. There's a very good reason to have XO locally in your devDependencies; You can have different XO versions for different projects. This is really the beauty of the npm dependency system. |
That's why I was suggesting to possibly show a warning that xo is missing in the deps if they have a xo field. For example I can still run the xo cli if I don't add the dep but have the xo field yet the atom plugin doesn't do anything and I can see others having the same issue without knowing the cause. |
Or maybe even warn in the xo cli tool that the field was found but the dep was missing. |
I am having insane issues using https://github.com/avajs/ava and XO. Both packages use https://github.com/sindresorhus/globby, not sure if this in any way related. I encounterd this problem in BIG projects. the number of targeted files doesn't matter but the size of the project itself.
|
After doing some testing @pixelass you're definitely right about the source of the problem. @sindresorhus it looks like globby is causing the slow downs. For example using the atom plugin if I move my file into a new directory with nothing but a |
I also worked around the issue by moving my testing and linting suite into the specific folder. It comes with drawbacks but currently the only solution that works. |
Also incredibly slow on my end to lint a single file that is 2987 lines long. It freezes on me, and I have to resort to using eslint directly instead. |
XO is still incredibly slow, I have a file that is 548 lines of code and XO hits 100% CPU and doesn't even finish running on it. |
@niftylettuce Just saying "things are slow" doesn't help anything. Please give us more information. Can you give us a repository to test with? What is your system information? Which version of Node? Can you dump the module tree you have? We can only help you as much as you help us. |
@Qix- here you go - this file does not lint and |
Thanks for the details. Out of curiosity did you test that code with eslint? |
I believe it's xo/prettier causing it, eslint runs fine on the file |
Ok. If you have already done the analysis and figured out it's Prettier, can you open an issue in the Prettier repo then? |
I just ran it with prettier by itself and it runs fine as well. Here's my relevant xo config from
|
Just to confirm |
|
So from my tests the problematic rule is |
@pvdlg Interesting find. Correct me if I'm wrong or missing the right config, here are contents of
|
Looks like this is filed here already eslint/eslint#10893 |
It was release today in I just tested after doing a |
Should we also version bump here https://github.com/xojs/xo/blob/master/package.json#L55? Not sure if we need a deprecation notice to alert users of bug. |
No, it's defined as a range ( We are not going to update each dependency definition every time there is a minor or patch release. That's the point of using npm and dependency range, you get the fix automatically (again, unless you use a lock file) |
I just upgraded and tried to |
Yep I tested and it works fine. I created My XO config is: "xo": {
"prettier": true,
"space": true
} Maybe you have a different config, but as you didn't provide it cannot test in the same exact circumstances. |
@pvdlg I provided my config above here - can you try it out? #212 (comment) |
You could omit the |
Oops missed that one. I'm checking |
Same, it runs in around 1 second. Which version of XO are you using? |
@pvdlg here is a narrowed down xo config that fails consistently for me:
here are versions: xo --version
0.23.0
eslint --version
v5.6.1 |
Same result. Works in 1s. Do you have a lock file? |
I did a clean of npm/yarn cache, completely removed yarn.lock and rm -rf node_modules and it works 🎉 That was painful - thank you so much @pvdlg for your time and patience here. Do you have a Patreon or way to donate for your help? |
That was probably due to
Sindre the creator of XO has a Patreon. I personally don't need money for my OSS work. |
12% of all NPM downloads are of @sindresorhus's code. You can find his well-deserved Patreon here. |
@OmgImAlexis please close issue |
Why? Still an issue for me. |
This seems to be fixed on my side (after updating) |
xo
seems to load really slowly when linting a single file. The main issue from what I can see relies aroundeslint:glob-util
and loadingplugin:unicorn/recommended
because of these two the lint can take upwards of 2 seconds to lint a 19 line file. This doesn't sound so bad until you take into account it takes the same time when running in atom with theatom-linter-xo
package.I used this to run xo
DEBUG=* xo js/helpers/config.js
.This is the file I was linting.
The text was updated successfully, but these errors were encountered: