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

Linting problems with javascript layer #10131

Closed
heyarne opened this issue Jan 11, 2018 · 15 comments
Closed

Linting problems with javascript layer #10131

heyarne opened this issue Jan 11, 2018 · 15 comments

Comments

@heyarne
Copy link

heyarne commented Jan 11, 2018

Description :octocat:

I'm having problems with the javascript layer not properly finding my local eslint config. I've discussed the issue in greater detail here, but what it boils down to is this:

  • If SPC e v shows me that javascript-eslint is enabled for flycheck, the js2-mode checker is used still. Linter output of eslint . in my shell is different from the one I see in emacs.
  • In subfolders of a project a local version of eslint installed in $PROJECT_ROOT/node_modules/.bin/eslint is not found and javascript-eslint is shown as automatically disabled
$ which eslint
node_modules/.bin/eslint
$ eslint .

/Users/heyarne/test/controllers/games.js
  11:1  error  More than 1 blank line not allowed  no-multiple-empty-lines

✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

$ eslint --no-eslintrc .

/Users/heyarne/test/controllers/games.js
  1:1  error  Parsing error: The keyword 'const' is reserved

/Users/heyarne/test/index.js
  1:1  error  Parsing error: The keyword 'const' is reserved

/Users/heyarne/test/migrations/20180110170854-init.js
  3:15  error  Parsing error: Assigning to rvalue

/Users/heyarne/test/migrations/20180110184039-create-player-table.js
  3:15  error  Parsing error: Assigning to rvalue

/Users/heyarne/test/utils/database.js
  1:1  error  Parsing error: The keyword 'const' is reserved

/Users/heyarne/test/utils/generate-name.js
  7:3  error  Parsing error: The keyword 'const' is reserved

✖ 6 problems (6 errors, 0 warnings)

The warnings I see in emacs:

screen shot 2018-01-11 at 13 25 57

Reproduction guide 🪲

  • Start Emacs
  • Enable the syntax-checking and javascript layers
  • Add an .eslintrc to your project root with content { "extends": "standard" }
  • npm i -D eslint eslint-config-standard eslint-plugin-import eslint-plugin-node eslint-plugin-promise eslint-plugin-standard
  • Open any javascript file and check the linter output

Observed behaviour: 👀 💔
See above.

Expected behaviour: ❤️ 😄
I'd expect the output to be identical to what my local eslint install provied.

System Info 💻

  • OS: darwin
  • Emacs: 25.3.1
  • Spacemacs: 0.200.10
  • Spacemacs branch: master (rev. 4bb4cb4)
  • Graphic display: t
  • Distribution: spacemacs
  • Editing style: vim
  • Completion: helm
  • Layers:
(javascript helm
            (auto-completion :variables auto-completion-enable-help-tooltip t auto-completion-enable-sort-by-usage t auto-completion-tab-key-behavior 'complete auto-completion-return-key-behavior 'complete)
            syntax-checking emacs-lisp python clojure html php yaml git
            (version-control :variables version-control-diff-tool 'git-gutter version-control-global-margin t)
            markdown
            (org :variables org-projectile-file "docs/PROJECT.org")
            osx themes-megapack)
  • System configuration features: JPEG RSVG IMAGEMAGICK NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES
@syl20bnr
Copy link
Owner

Is there a way to overwrite the executable of a checker in Flycheck ?

@heyarne
Copy link
Author

heyarne commented Jan 11, 2018

flycheck/flycheck#272 seems to suggest that this is possible.

@zer09
Copy link
Contributor

zer09 commented Jan 12, 2018

@heyarne I don't know if it is related but have you tried setting this in your .S>/user-init

  (setq js2-mode-show-parse-errors nil)
  (setq js2-mode-show-strict-warnings nil)

@heyarne
Copy link
Author

heyarne commented Jan 12, 2018

@zer09 Nice! That solves a part of the problem. It now uses the local eslint when I'm in the same location and output is expected. It doesn't underline the whole lines but just shows a mark underneath the affected cols, which is okay I gues.. When I'm in a subfolder the output of SPC e v however is still as shown below and linting is disabled:

Syntax checkers for buffer 20180110170854-init.js in js2-mode:

  javascript-eslint (disabled)
    - may enable:  Automatically disabled!
    - executable:  Not found
    - config file: missing or incorrect

  javascript-jshint (disabled)
    - may enable:         Automatically disabled!
    - executable:         Not found
    - configuration file: Not found

  javascript-jscs (disabled)
    - may enable:         Automatically disabled!
    - executable:         Not found
    - configuration file: Not found

  javascript-standard (disabled)
    - may enable: Automatically disabled!
    - executable: Not found

  drupal-phpcs (disabled)
    - may enable: Automatically disabled!
    - predicate:  nil
    - executable: Not found

Flycheck Mode is enabled. Use SPC u C-c ! x to enable disabled checkers.

--------------------

Flycheck version: 32snapshot (package: 20171109.216)
Emacs version:    25.3.1
System:           x86_64-apple-darwin16.7.0
Window system:    ns

@zer09
Copy link
Contributor

zer09 commented Jan 13, 2018

I can't remember if I experience this before, but when I fully move from vscode to spacemacs for my js dev, I end up installing eslint globally.

I will try tomorrow if I can reproduced this by installing eslint locally.

@heyarne
Copy link
Author

heyarne commented Jan 13, 2018

The problem with installing it globally is that all modules referenced in the config need to be installed globally as well. This can quickly become a problem when working with different teams and running into dependency-conflicts.

@zer09
Copy link
Contributor

zer09 commented Jan 14, 2018

Ok I am able to reproduced your issue, if eslint installed locally.

@heyarne
Copy link
Author

heyarne commented Jan 14, 2018

There is this suggested solution that's using projectile to find the local .eslintrc and use that instead but I couldn't get it to work? Any thoughts?

@zer09
Copy link
Contributor

zer09 commented Jan 14, 2018

Maybe this issue can help flycheck/flycheck#1087 specially the 2nd to the last comment.

@heyarne
Copy link
Author

heyarne commented Jan 14, 2018

Nice, that solved it! Thanks a lot.

@heyarne heyarne closed this as completed Jan 14, 2018
@jameslaneconkling
Copy link

@heyarne can you post your solution? I tried the solutions suggested in flycheck/flycheck#1087, but couldn't get it to work, perhaps b/c I'm not configuring it correctly for spacemacs. Thanks!

@heyarne
Copy link
Author

heyarne commented Jan 18, 2018

Sure: In my spacemacs config I added javascript to dotspacemacs-configuration-layers and add-node-modules-path to dotspacemacs-additional-packages. In dotspacemacs/user-config I added (setq js2-mode-show-parse-errors nil js2-mode-show-strict-warnings nil) to disable the standard linter that comes with js2-mode. Finally I made sure that all the packages are installed locally in your project (eslint + x).

If you're still having trouble, you can check your flycheck config via SPC e v.

@peterspicer
Copy link

peterspicer commented Feb 25, 2018

In addition to all of the above (credit goes to those folks).... for Spacemacs users ...

  1. in dotspacemacs-configuration-layers
     ;;==================== +checkers
     ;; https://github.com/syl20bnr/spacemacs/tree/master/layers/%2Bcheckers/syntax-checking
     syntax-checking
     ;;
     ;;==================== +completion
     ;; http://spacemacs.org/layers/+completion/auto-completion/README.html
     ;; https://github.com/syl20bnr/spacemacs/tree/master/layers/%2Bcompletion/auto-completion
     (auto-completion :variables
                      auto-completion-enable-snippets-in-popup t
                      ;; tab key to complete as much of common completion as possible
                      auto-completion-tab-key-behavior 'cycle
                      ;; automatic docstring display
                      auto-completion-enable-help-tooltip t
                      ;; enable the most frequent matches to show first
                      auto-completion-enable-sort-by-usage t
                      )
     ;;
     ;; https://github.com/syl20bnr/spacemacs/tree/master/layers/%2Blang/javascript
     ;;  Project local
     ;; $ sudo npm install tern
     ;; $ sudo npm install js-beautify
     ;; $ sudo npm install eslint
     javascript

1-a. then

;; https://melpa.org/#/add-node-modules-path
dotspacemacs-additional-packages '(add-node-modules-path)
  1. in dotspacemacs/user-config ()
  ;;===== Node setup:
  ;;  1. Careful of global ($npm -g install <module>) vs. local ./node-modules
  ;;  2. Best to use local as conflicts between projects might occur
  ;;  3. See inclusion of "add-node-modules-path" in dotspacemacs-additional-packages
  ;;  4. https://melpa.org/#/add-node-modules-path
  ;;
  ;; stop default linter - use ESLint linter as part of FlyCheck
  (setq js2-mode-show-parse-errors nil js2-mode-show-strict-warnings nil)
  (eval-after-load 'js2-mode
    '(add-hook 'js2-mode-hook #'add-node-modules-path))

Then, while in the "Javascript IDE" <your-filename>.js Spc + e + v (flycheck-verify-setup)

image

@samuy
Copy link

samuy commented Jul 28, 2020

Is this still a viable solution? I'm reading that node-add-modules-path introduces a security risk. Sorry if I'm not understanding the solution correctly -- still very new to spacemacs

@MarkVasile
Copy link

@samuy The security risk you linked to was referring the adding a relative path the the shell $PATH variable, "especially at the beginning". Looking at the source code of node-add-modules-path you can see that it's adding the project's ./node_modules/.bin to the executable path list. I think there's still a risk of having some payload installed in .bin (other than common commands like ls, which are still safe), but then again, we devs are responsible for checking that we don't import just any package we find on stack overflow :).

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

7 participants