Skip to content
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

Better interop with ESLint? #122

Closed
wmertens opened this issue Jun 7, 2016 · 24 comments
Closed

Better interop with ESLint? #122

wmertens opened this issue Jun 7, 2016 · 24 comments

Comments

@wmertens
Copy link
Contributor

wmertens commented Jun 7, 2016

Ok, this is hard to explain. I really like XO, but it is just a very thin wrapper around eslint nowadays, with eslint config in package.json and the various editor plugins for eslint having also discovered the fix capabilities of eslint.

Furthermore, eslint plugins have npm dependencies for eslint, but XO pulls in eslint so it is missing from the project's package.json, leading to warnings, or else you have to manage eslint anyway.

It is really nice that XO plays the role of curator, pulling in useful eslint configs and plugins as they become available (:heart: AVA) .

So I am wondering if it would not be better if XO was only a CLI tool to run eslint and manage the eslint config and dependencies in package.json, but as far as npm and the editors are concerned, there is only eslint… I would like to run xo update-config and it would do all the things that would have happened with xo as a direct dependency, except that the rest of the eslint ecosystem understands what's going on…

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

…so maybe all that is needed is a command that destructively sets the equivalent eslint config in package.json based on the xo config. That allows all modes of use.

@sindresorhus
Copy link
Member

Why? Is there anything missing that you can only get by using ESLint? There are XO plugins (IMHO better than the ESLint ones) for most editors, so not really sure what you would gain.

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

Well, the editor plugin for VSCode is a bit lacking, and it seems that if an editor officially supports a linter, it is likely to be eslint.

Then there is also the npm thing, where it's a bit unclear what eslint deps to declare. If you use the extra config for React, you get a warning for missing deps. The only way around that would be if all eslint plugins/configs would be wrapped as being xo plugins/configs…

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

Ok, so basically, consider this a feature request for a command that dumps the current eslint config as defined by xo into .eslintrc with the comment at the top saying autogenerated by XO. Do NOT edit. See package.json for configuration. That would be best of both worlds for me…

@sindresorhus
Copy link
Member

Well, the editor plugin for VSCode is a bit lacking

Lacking how? → https://github.com/SamVerschueren/vscode-linter-xo/issues/new // @SamVerschueren

and it seems that if an editor officially supports a linter, it is likely to be eslint.

Don't like discussing hypotheticals. Any decent editor would have plugin support.

Then there is also the npm thing, where it's a bit unclear what eslint deps to declare.

I'm not following. You never declare ESLint as a dependency. That's bundled for you in XO.

If you use the extra config for React, you get a warning for missing deps. The only way around that would be if all eslint plugins/configs would be wrapped as being xo plugins/configs…

That's the first time I'm hearing about this. Can you elaborate?

Ok, so basically, consider this a feature request for a command that dumps the current eslint config as defined by xo into .eslintrc with the comment at the top saying autogenerated by XO. See package.json for configuration

I'm open to it if there's an actual use-case. That's what I'm trying to investigate above.

@jamestalmage
Copy link
Contributor

Isn't this exactly what eslint-config-xo is?

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

  • Lacking: XO Fix => empty file vscode-linter-xo#7 (no slight intended, I would help fix it if I had time, I understand that OSS is developed in spare time)
  • Hypothetical: VSCode release notes specifically mention ESLint, webstorm has built-in support
  • npm: Hmm, I definitely had issues with this in the past (maybe with npm v2) but now it seems fine. I removed some eslint deps from a project and npm is not complaining any more. Great :)
  • eslint-config-xo is still more annoying to set up than xo…

@SamVerschueren
Copy link
Contributor

SamVerschueren commented Jun 7, 2016

Well, the editor plugin for VSCode is a bit lacking

Probably referring to this one xojs/vscode-linter-xo#7. Currently don't really have much time to reimplement the entire plugin. But PR is always more then welcome ;). I actually rarely use the fix option, but that's probably me :).

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

  • npm: scratch that, I forgot to prune. This is what I get when I prune npm and added eslint-config-xo-react to the xo config:
$ npm i
/Users/wmertens/Documents/Projects/app_agent
├── autoprefixer@6.3.6 
├── UNMET PEER DEPENDENCY eslint@>=2
├── UNMET PEER DEPENDENCY eslint-plugin-react@>=5
├── postcss-flexbugs-fixes@2.0.0 
└── postcss-loader@0.9.1 

npm WARN eslint-config-xo@0.14.1 requires a peer of eslint@>=2.9.0 but none was installed.
npm WARN eslint-config-xo-react@0.7.0 requires a peer of eslint@>=2 but none was installed.
npm WARN eslint-config-xo-react@0.7.0 requires a peer of eslint-plugin-react@>=5 but none was installed.
npm WARN eslint-plugin-ava@2.4.0 requires a peer of eslint@>=2 but none was installed.
npm WARN eslint-plugin-babel@3.2.0 requires a peer of eslint@>=1.0.0 but none was installed.
npm WARN eslint-plugin-import-order@2.1.4 requires a peer of eslint@>=2 but none was installed.
npm WARN eslint-plugin-xo@0.3.1 requires a peer of eslint@>=2 but none was installed.
npm WARN app_agent No repository field.
npm WARN app_agent No license field.

@jamestalmage
Copy link
Contributor

I think this idea is sort of interesting. He is right that there is no XO config for WebStorm/IDEA, and the existing eslint plugin can't be forced to work with XO.

I think this would probably only work for npm@3, since our dependencies would be nested in npm@2, and we can't manipulate require paths without the XO runtime.

Still, it would be great if I could add .eslintrc to .gitignore and just create it to help out WebStorm.

(Or I could finally get around to writing a WebStorm plugin. But that means Java, which slows my productivity to a crawl).

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

I think this would probably only work for npm@3, since our dependencies would be nested in npm@2, and we can't manipulate require paths without the XO runtime.

Can you elaborate? A flat node_modules dir is not guaranteed with npm3, only if it was freshly installed. There are alternatives that don't do this. I think the only real fix is adding eslint deps into devDependencies…

@jamestalmage
Copy link
Contributor

A flat node_modules dir is not guaranteed with npm3

I'm aware, but if you're installing eslint plugins directly into devDependencies, they should probably take precedence anyways.

I think the only real fix is adding eslint deps into devDependencies…

Perhaps that's the most robust solution, but I wouldn't want it to do that. I'm more interested in a band-aid that works to provide editor support in most projects until native XO plugins are more ubiquitous.

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

Perhaps that's the most robust solution, but I wouldn't want it to do that. I'm more interested in a band-aid that works to provide editor support in most projects until native XO plugins are more ubiquitous.

Yes, I hate it too, I am sad that we can't have comments in package.json to indicate what a dep is used for.

So how would you solve the problem of eslint-config-xo-react peer-requiring eslint and the plugin? That will occur in the only-xo situation as well.

@wmertens
Copy link
Contributor Author

wmertens commented Jun 7, 2016

I wonder if eslint can use relative paths as config and plugin names…

@wmertens
Copy link
Contributor Author

So, for now, installing XO with any external config means also installing
eslint, correct? I can live with that, but indeed it would be nice to dump
the eslint config as used by XO to a file.

On Tue, Jun 7, 2016 at 6:24 PM James Talmage notifications@github.com
wrote:

A flat node_modules dir is not guaranteed with npm3

I'm aware, but if you're installing eslint plugins directly into
devDependencies, they should probably take precedence anyways.

I think the only real fix is adding eslint deps into devDependencies…

Perhaps that's the most robust solution, but I wouldn't want it to do
that. I'm more interested in a band-aid that works to provide editor
support in most projects until native XO plugins are more ubiquitous.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#122 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AADWlr4h98fiqPy8uMpq4wQ-nwJ70lWYks5qJZtHgaJpZM4IwCZU
.

Wout.
(typed on mobile, excuse terseness)

@sindresorhus
Copy link
Member

@wmertens We can probably make use of the --print-config flag. I'm just waiting for it to stabilize as it seems a bit buggy at the moment.

@wmertens
Copy link
Contributor Author

I just found another use for this: codeclimate.com uses eslint as part of their analysis, and they need an eslintrc…

@wmertens
Copy link
Contributor Author

Funny, on the day you made the comment about print-config being buggy, they merged this: eslint/eslint#6385

The initial code is https://github.com/eslint/eslint/pull/5145/files btw, it uses engine.getConfigForFile(...). There's apparently also Config.getConfig(dir) (https://github.com/eslint/eslint/pull/6385/files#diff-6ba37f425b9251290009f67e414218a5R492).

@wmertens
Copy link
Contributor Author

Since XO can best be described as an eslint wrapper, but you can't tell npm that, how about making XO edit the package.json to require eslint and all its dependencies?

That way you would edit the XO config, and it would automatically install all the required plugins.

@fregante
Copy link
Member

fregante commented Aug 13, 2016

This sounds like the eject command of https://github.com/facebookincubator/create-react-app

That command expands the wrapper's dependencies and config into the project, and then disappears.

XO (or more likely a secondary tool for it) could apply the config to eslintrc and package.json but still stay around to be the "source of truth" of the configuration. This way if you only have ESLint (for example in a non-xo-supporting editor), it should automatically behave the same as XO.

@jeffreywescott
Copy link

I agree with @wmertens -- CodeClimate integration for XO would be awesome.

More to the point, however, is that when <insert random project here> wants to integrate with a JS linter, they're going to integrate with eslint, if for no other reason than it has the most distribution. So, a command that would auto-generate a .eslintrc from the XO config would be amazing.

@wmertens
Copy link
Contributor Author

wmertens commented Mar 5, 2017

Here's another example: https://github.com/relekang/lint-filter only shows you linter errors from your changes since you branched from master, so that if your codebase is not clean yet, at least new changesets will be clean.

However, it needs to read the eslint configuration…

@xojs xojs deleted a comment from ForsakenHarmony Nov 28, 2017
@sindresorhus sindresorhus changed the title Better interop with eslint? Better interop with ESLint? Nov 28, 2017
@sindresorhus
Copy link
Member

Continued in #272

@fregante
Copy link
Member

fregante commented Jan 4, 2023

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants