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

"parser" config not supported #31

Closed
CrabDude opened this issue Mar 11, 2015 · 10 comments
Closed

"parser" config not supported #31

CrabDude opened this issue Mar 11, 2015 · 10 comments

Comments

@CrabDude
Copy link

Currently, SublimeLinter-eslint does not seem to support the ESLint "parser" config option. When installing and using Babel-eslint from the CLI, everything works, but it fails in SublimeLinter-eslint (e.g., async/await).

@CrabDude
Copy link
Author

It seems this does work, but I had to restart Sublime (or maybe it was because I reinstalled eslint and babel-eslint) for it to work for some reason, and I have no idea why.

@roadhump
Copy link
Collaborator

It seems that package.json is required to work with local node modules and to find babel-eslint there. Or it should work if babel-eslint is installed globally.

@roadhump
Copy link
Collaborator

Yes, I suppose it is an issue with SublimeLinter, I see python errors in console when create/delete package.json and it starts work again only after restart.

@mderazon
Copy link

mderazon commented Jul 8, 2015

This is happening to me as well, was about to pull my hair out.

I have eslint and babel-eslint installed globally. I don't have a direct dependency on eslint in my package.json but it seems that other packages I have are using it so it ends up in my node_modules/.bin folder.

When running eslint from shell, it uses the global eslint package and all works well. But when running in Sublime via Sublime-eslint it fails and tells me Error - Cannot find module 'babel-eslint' because it's looking for the local one in bin folder.

Just for try, when I delete package.json Sublime-eslint uses the global one and all works okay. But when I bring back package.json it tries to use the local one again.

Any idea why it happens ?
Anything to do about it ? (other than unnecessarily installing babel-eslint as a local dep)

@mderazon
Copy link

mderazon commented Jul 8, 2015

It seems that the problem comes from NodeLinter extension
https://github.com/SublimeLinter/SublimeLinter3/blob/ce29335059a0aecf7f8ef8c2b6b5e3b07a70542b/lint/node_linter.py

It searches for package.json and then looks for binaries in the bin folder. Unfortunately, it doesn't take into account plugins such as babel-eslint.

I think that it's better if this linter wouldn't use NodeLinter and instead always require global installation. This is already very common requirement on most of js linters that work with js.

@roadhump what do you think ?

@andrewdeandrade
Copy link

@mderazon It's highly recommended that you don't do something like install eslint globally. The reason is that eslint versions aren't entirely compatible with any .eslintrc files. For an example of this type of incompatibility, see this open issue I filed for eslint that is still unresolved: eslint/eslint#1549

That said, if you really want this to work, I need to know more about why SublimeLinter needs to know about the eslint plugins at all. Theoretically, SublimeLinter and the NodeLinter class should only have to know about the linter (eslint in this case) and any configuration file to load. Beyond that it should be entirely eslint's responsibility to to know about any eslint plugins and how to load them.

When NodeLinter doesn't find a binary in the local ./node_modules/.bin/ folder it should default back to the globally installed linter, which was the behavior prior to creating NodeLinter. I don't see why this should bork plugin loading. Then again, it's been a while since I dug into the eslint code, so I'm not familiar with how it handles plugin loading in terms of path resolution (I would imagine that it just relies on how node resolves modules, which is to load from the local node_modules and climb the file hierarchy checking parent node_modules until it gets to checking the user's global node_modules folder and then the system global node_modules folder)

Do you have stack traces and errors to share?

@mderazon
Copy link

mderazon commented Jul 9, 2015

Plugins in eslint have peer dependency with eslint and are installed separately. Eslint plugins doc says

A globally-installed instance of ESLint can only use globally-installed ESLint plugins. A locally-installed ESLint can make sure of both locally- and globally- installed ESLint plugins.

Eslint is the one looking for the plugin in the path and not NodeLinter and when eslint is run locally by this linter with the help of NodeLint it can't find the plugin since it was installed globally (along with the global version of eslint) and I get:

SublimeLinter: eslint: counters.js ['/Users/me/proj-src/proj-common/node_modules/.bin/eslint', '--format', 'compact', '--stdin', '--stdin-filename', '__RELATIVE_TO_FOLDER__'] 
SublimeLinter: eslint output:
lib/counters.js: line 0, col 0, Error - Cannot find module 'babel-eslint'

Eslint was "randomly" installed in my local bin folder by on of my dependencies and that's the only reason that it is run locally.

Maybe a better solution, instead of looking in the bin folder for the linter binary, was if NodeLint could analyze the package.json to see if the linter is listed as a dependency. If it does, go take it from the bin folder. If it doesn't, look for it globally in the path

@andrewdeandrade
Copy link

@mderazon unfortunately, this is very squarely in the territory of a wontfix. Globals are pretty much always a bad idea in almost every situation except when you absolutely need a singleton. The entire purpose of NodeLinter was to be able to move away from globally installed linters. If a project relies on eslint to maintain correct style, then it should be a development dependency every time. 99% of the time you want a module installed locally. The only time you should want a module installed globally is when the module is a cli and that CLI is a general utility and not something meant to be used as a dev dependency of a specific project. You should never rely on globally installed modules as a library.

Imagine the following scenario: you create an open-source module with the husky module to lint the project pre-commit with eslint to make sure all pull requests conform to the project style. By relying on a global eslint, another user that clones the project and npm installs will encounter an error when they try to commit. Requiring people to globally install stuff in order to contribute to a project is bad etiquette. Bundling all dependencies into a project results in less friction for all future contributors.

As I pointed out in my last comment, not all versions of eslint are compatible with every rc file. There is no guarantee that your globally installed eslint or the globally installed eslint of another contributor will be compatible with the rc file in your project.

Lastly, there is no reason that SublimeLinter should be implementing features that are specific to the internal semantics of one specific linter. If many linters all had this same issue, it would make sense as a feature, but since this is pretty much isolated to eslint, it's an internal concern of eslint. To add something to SublimeLinter here would unnecessarily increase the complexity of SublimeLinter and would tightly couple SublimeLinter to eslint.

I'm open to listening to additional arguments making a case for this feature, but I also ask that you think carefully about separation of concerns and why avoiding global state is a bad thing.

@CrabDude
Copy link
Author

@malandrew Your POV is reasonable, but ignores the primary use case: as an editor-level plug-in. Inheriting project settings like a local .eslintrc or a local eslint install is a convenience, but frequently you will be working on projects that either don't use eslint, or don't have devDependencies installed, and for those cases you are concluding "wontfix". While I appreciate the rationale, having linting only work when the specific project supports it is just not very useful compared to the typical editor with linter plug-ins and project-based rc-file inheritance.

As someone who regularly trains new node.js developers, I see these conversations often, "I installed everything like I was supposed to, but I still am not seeing linter errors or popups??" => "Oh, it's cuz you don't have X installed just right..."

EDIT: The above is a user perspective; in no way am I making any claims or recommendations on the logistics or feasibility of supporting this in SublimeLinter if, for instance, some eslint idiosyncrasy precludes it.

@mderazon
Copy link

@malandrew generally I agree with your POV, I also think that it's healthier to bundle your lint dependencies with the project dependencies locally.

But the fact is that sublimelinter ("officially") supports having the dependencies installed globally and when I explicitly don't want a linter to be used locally

I think that if NodeLinter wanna do stuff the "Node way" then it should try to:

  1. First check if the plugin is a dependency in package.json
  2. If it is, try to use it locally (bin folder)
  3. If it isn't, try to use it globally

Steps 2, 3 are practically already implemented, so it's only step 1.

But eventually, Eslint plugins are minor annoyance in this case, I just wasted timed trying to figure out what was going on but form now on I just know to bundle my linters + plugins locally... :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants