Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Fallback to globally installed eslint if not installed in project #845

Open
gnestor opened this issue Mar 9, 2017 · 19 comments
Open

Fallback to globally installed eslint if not installed in project #845

gnestor opened this issue Mar 9, 2017 · 19 comments

Comments

@gnestor
Copy link

gnestor commented Mar 9, 2017

Issue Type

Feature Request

Issue Description

I have a global eslint setup with ~/.eslintrc and globally installed eslint, babel-eslint, etc. This is great because I can now lint all JS projects, not just those that have eslint installed and/or configured. However, if I am working on a project that does have its own eslint setup, then I start running into errors like "eslint-plugin-flowtype not found" because it's required by the project's eslint config but not installed globally.

I propose that this package offer a global eslint installation fallback option so that project installations take precedence.

Thoughts?

@Arcanemagus
Copy link
Member

An option like the Use Global ESLint option that already exists? 😛

As a note, from what you are describing the project in question does have an ESLint configuration, so it should be installed in the project.

@IanVS
Copy link
Member

IanVS commented Mar 9, 2017

@Arcanemagus, actually we don't 'fall-back'. The Use Global ESLint forces the global version to always be used, regardless if the project has its own locally installed ESLint.

We have talked about making this change in the past, and agreed it would be worthwhile. But so far, nobody has done the work and submitted a PR.

@IanVS
Copy link
Member

IanVS commented Mar 9, 2017

@gnestor I assume you have Use global checked, right? Can you share the output of the Linter Eslint: Debug command?

@gnestor
Copy link
Author

gnestor commented Mar 9, 2017

@IanVS Currently, when I am working in a project that DOES NOT have eslint set up, I enable Use global. When I switch to a project that DOES have a eslint set up, I need to disable Use global. I am proposing that Use global only uses the global eslint installation if there is no local installation.

My debug:

Atom version: 1.14.4
linter-eslint version: 8.1.3
ESLint version: 3.17.1
Hours since last Atom restart: 14.3
Platform: darwin
Using global ESLint from: /usr/local/lib/node_modules/eslint
Current file's scopes: [
  "source.js.jsx",
  "meta.class.body.js",
  "meta.function.method.js",
  "meta.tag.jsx",
  "JSXAttrs",
  "JSXNested",
  "meta.embedded.expression.js",
  "source.js.jsx",
  "meta.tag.jsx",
  "JSXAttrs",
  "comment.line.double-slash.js"
]
linter-eslint configuration: {
  "disableWhenNoEslintConfig": false,
  "disableWhenNoEslintrcFileInPath": true,
  "lintHtmlFiles": true,
  "useGlobalEslint": true,
  "showRuleIdInMessage": true,
  "eslintrcPath": "",
  "globalNodePath": "",
  "advancedLocalNodeModules": "",
  "eslintRulesDir": "",
  "disableEslintIgnore": false,
  "disableFSCache": false,
  "fixOnSave": false,
  "scopes": [
    "source.js",
    "source.jsx",
    "source.js.jsx",
    "source.babel",
    "source.js-semantic"
  ],
  "rulesToSilenceWhileTyping": [],
  "rulesToDisableWhileFixing": []
}

@IanVS
Copy link
Member

IanVS commented Mar 9, 2017

Yep, that's what I figured. I totally agree. Are you willing to take a crack at a PR?

@gnestor
Copy link
Author

gnestor commented Mar 9, 2017

Ya!

@tillydray
Copy link

Not sure if I should comment here or start another issue, let me know if I should start another issue. I spent hours yesterday trying to get .eslintrc to be recognized globally. My primary question is, if the .eslintrc Path in Atom doesn't mean that my .eslintrc will be used globally, what does it mean? And is there anything I can do to help with this PR?

What Happened?

I've installed linter-eslint globally using npm. I've also globally installed eslint-config-google using npm. When I try to do Linter eslint: fix file in Atom, I get Notification must be created with string message: as a warning in the top right of Atom. (It doesn't show the rest of the message, if there is any further message.) And it wasn't linting on file change or file save.

What did I try?

There's an option in Atom for .eslintrc Path that says It will only be used when there's no config file in project. That leads me to believe that a global .eslintrc is possible. I set the .eslintrc Path to a variety of directories:

  • /usr/local (which is in my PATH)
  • ~/.atom/packages/linter because someone suggested it, and
  • ~/Desktop

using both objective and relative paths for the last two. And of course .eslintrc was inside each of those when I changed the .eslintrc Path. Changing .eslintrc Path to these 3 different locations still resulted in the above warning and no linting.

Once I put my .eslintrc in the project directory, it linted on file change.

@Arcanemagus
Copy link
Member

@JasonMFry Yours is a separate issue 😉.

To answer your question though, that setting requires the full path to your "global" .eslintrc. For example /home/JasonMFry/lintConfigs/.eslintrc.

Note that ESLint itself already supports a fallback config. If you place a .eslintrc file in your home directory then it will use that if it is unable to find any other configuration for a file. The setting you are talking about only takes effect when both of the following conditions are met:

  • There is no configuration in the project, or any parent directory of the project.
  • There is no configuration in the user's home directory.

@IanVS
Copy link
Member

IanVS commented Aug 7, 2017

@gnestor, just checking in, do you still think you'll be able to attempt a PR for this? No pressure, just asking.

@skylize
Copy link
Contributor

skylize commented Sep 11, 2017

I've been thinking of opening this exact issue, except with one extra feature. I would like to see a fallback that includes a "Using global eslint settings" warning listed as a linter message (line number could be last line of the file). This would encourage using local installations, while still making linter-eslint always available.

I'm willing to stab at a fix also. @gnestor if you've made any progress on this, please link to your working branch on github. Thx

@gnestor
Copy link
Author

gnestor commented Sep 12, 2017

@skylize Thank you for stepping up! I've been so busy with work so I haven't had a chance to start this. Let me know I can assist 👍

@bcnichols3
Copy link

Has there been any movement on implementing this feature? Its been several months since the last comment — now that I'm using CRA it's becoming a big annoyance for me.

@IanVS
Copy link
Member

IanVS commented Jun 20, 2018

This is still open to anyone who has the time and willingness to implement. Would you like to take a crack at it, @bcnichols3?

@bcnichols3
Copy link

Hold my beer.

@bcnichols3
Copy link

bcnichols3 commented Jun 20, 2018

quick question: in findESLintDirectory what is the else statement for? If I'm moving global to the base case, as opposed to the first case (which is where it is now) it'll need an actual statement.

... } else { locationType = 'advanced specified'; eslintDir = Path.join(projectPath || '', cleanPath(config.advanced.localNodeModules), 'eslint') }

@IanVS
Copy link
Member

IanVS commented Jun 20, 2018

The last two blocks (with locationType = 'advanced specified') both have to do with config.advanced.localNodeModules, and whether an absolute or relative path has been specified in the config.

Since I'm not a huge fan of negative if checks, I'd recommend inverting that a bit, and checking for the presence of config.advanced.localNodeModules, then inside that block checking whether it is absolute or not.

So, I envision what you'll have to do is:

  1. first check config.advanced.localNodeModules and whether it is absolute or relative

else:

  1. build a local path to test and call isDirectory on it, and if that succeeds then return that path

but if it fails:

  1. check for config.global.useGlobalEslint and either return that or throw an error if it isn't found.

Does that make any sense?

@bcnichols3
Copy link

bcnichols3 commented Jun 20, 2018

Definitely. My original thinking is to make it so useGlobalEslint remains as an option to supersede local eslint? Though that sounds like a terrible idea. I wonder if that check should be removed entirely, so as to not make it easy to jump over a local eslint — which is definitely a bad idea.

Currently I have:

  1. if (localNodeModules && !useGlobalEslint) then locationType = 'advanced specified'; and eslintDir uses a ternary to determine relative or absolute path.
  2. else if (!useGlobalEslint) then locationType = 'local project' and eslintDir is pulled from the modules.
  3. if (!isDirectory(eslintDir)) then locationType = 'global' and eslintDir is resolved to global directory, with respect to OS.
  4. if (isDirectory(eslintDir)) then return the object
  5. else then `throw new Error('ESLint not found, please add a local eslint to this project or ensure the global Node path is set correctly.')

But I can change that to reflect the thinking above. I also altered worker helper tests accordingly, but two eslint provider tests are failing:

  • when a file is specified in an eslintIgnore key in package.json it will not give warnings when linting the file is throwing the wrong error
  • it errors when no config file is found is also throwing the wrong error

@IanVS
Copy link
Member

IanVS commented Jun 20, 2018

Do you mind opening a PR with what you have so far? It's easier to talk alongside the code I think. Thanks!

@bcnichols3
Copy link

any thoughts @IanVS ?

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

No branches or pull requests

7 participants