Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

CLI and Sublime Text integration #49

Closed
mquandalle opened this issue Oct 29, 2015 · 11 comments
Closed

CLI and Sublime Text integration #49

mquandalle opened this issue Oct 29, 2015 · 11 comments

Comments

@mquandalle
Copy link

I recently merged your PR wekan/wekan#370. Even if I like Atom, my editor of choice is still Sublime Text and I was unable to get the new linting plugins work in there. It might be related to the fact that eslint ./ doesn’t work either on the CLI:

➜  wekan git:(devel) npm install        
[...]
➜  wekan git:(devel) eslint ./          
module.js:338
    throw err;
    ^

Error: Cannot find module 'babel-runtime/core-js/object/keys'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:286:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/usr/lib/node_modules/eslint-plugin-meteor/dist/index.js:3:20)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)

Maybe it’s a problem with Eslint or with one of the plugin used, but I wanted to ask you @dferber90 first before reporting an issue elsewhere. I’m on Ubuntu 15.04, I have installed the devDependencies locally and I have also the following versions installed globally:

$ node --version
v4.0.0
$ npm --version
2.14.2
$ eslint --version
v1.7.3
@dferber90
Copy link
Contributor

Thanks for the report!

It might be related to the fact that eslint ./ doesn’t work either on the CLI:

This is intended behavior. As eslint and eslint-plugin-meteor are no longer installed globally, you can't use them directly. Instead use $ npm run lint, or $ npm test. Let me know if it works for you with this command.

The reason it throws for you is because running eslint ./ will run the globally installed version instead of the local one. And this global version had a bug where I added babel-runtime as a devDependency instead of a normal one. If you update the globally installed version it would work again.
But using the local version is preferred, so you can remove the global versions to be sure to always use the local ones.

Even if I like Atom, my editor of choice is still Sublime Text

It should work just fine in ST. That was my previous editor. Make sure to configure it to use the local versions of ESLint if it finds them. Here's some details.

Let me know if something is unclear still 😊

@mquandalle
Copy link
Author

The actual Sublime Text linter error is:

Error: ESLint crashed: Error while loading rule 'meteor.globals': Invariant Violation: Linted file is not in parent.

@dferber90
Copy link
Contributor

Oh, looks like SublimeLinter does not pass the filename to ESLint (of the file being linted). ESLint-plugin-Meteor relies on it to determine where a file is going to run. When they add that, then it should work.

I'll file an issue there.

@dferber90
Copy link
Contributor

Actually SublimeLinter passes the relative filename instead of the absolute one (which is expected by ESLint-plugin-Meteor).
This seems to be the fix https://github.com/roadhump/SublimeLinter-eslint#contextgetfilename-in-rule-returns-relative-path. Does that work for you?

Maybe ESLint-plugin-Meteor can be extended to accept relative paths as well.

@mquandalle
Copy link
Author

Yes that fixed the issue :)

@mquandalle
Copy link
Author

I guess that would be good to document — both here and on Wekan contributor guide.

@dferber90
Copy link
Contributor

As I understand it, setting this option breaks support of .eslintignore in Sublime Text. I have added support for relative file paths instead. After updating to the newest release, you'll be able to use Sublime Text without any issues (hopefully). You can remove that fix with the extra settings again then :)

This way it just works out-of-the-box for everybody hopefully.


By the way, you should enable greenkeeper for wekan. It's an amazing tool that sends you a PR every time a new version of one of the dependencies in package.json is updated. So you'd get a PR with the new release of ESLint-plugin-Meteor now.

dferber90 added a commit that referenced this issue Oct 29, 2015
SublimeLinter passes relative paths through context.getFilename(). Support that as well.

closes #49
mquandalle added a commit to wekan/wekan that referenced this issue Oct 29, 2015
@mquandalle
Copy link
Author

Yep, it works! I’m far more happy with this solution :-)

@mquandalle
Copy link
Author

About Greenkeeper, I didn’t now it until I saw it in your project. I guess the first step to make it valuable would be to add some tests to wekan :-) I also like to read the changelog (and not only the commit history before merging). And anyway almost all Wekan dependencies are actually meteor packages so Greenkeeper wouldn’t help — but a Greenkeeper for Meteor packages would be great!

@dferber90
Copy link
Contributor

I also like to read the changelog

There's nothing stopping you from doing it. It just sends a PR, so you know there's been a new version. It doesn't auto-merge or anything :)

@mquandalle
Copy link
Author

Yes of course :-) (but sending the changelog directly in the PR would be a great improvement!)

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

No branches or pull requests

2 participants