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

--config flag doesn't prevent searching for ~/.eslintrc #4881

Closed
kentcdodds opened this issue Jan 8, 2016 · 12 comments
Closed

--config flag doesn't prevent searching for ~/.eslintrc #4881

kentcdodds opened this issue Jan 8, 2016 · 12 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible bug ESLint is working incorrectly core Relates to ESLint's core APIs and features

Comments

@kentcdodds
Copy link
Contributor

See twitter for context:

screen shot 2016-01-07 at 8 08 20 pm

This issue is just meant to discuss the scenario of what happens when you run eslint --config someESLintConfig.js foo.js. Currently, ESLint will merge someESLintConfig.js with a configuration found in the user's home directory (for example ~/.eslintrc). This can lead to some difficult to track issues.

An important note to all of this is if there IS an .eslintrc in the directory where you're running eslint, then ESLint will not merge the user's home directory.

Specifically for me my root .eslintrc was created pre-1.0.0 days so I kept getting a bunch of removed errors and I couldn't identify why that was happening (no amount of grep in that directory would save me). I finally discovered the problem by putting in a bunch of console.logs and throwing errors throughout ESLint in my node_modules until I discovered what was happening.

I'm personally leaning toward ESLint treating an explicit --config flag the same it does when there's an .eslintrc in the directory you're running ESLint. I think the main use case for the --config flag is to specify a specific configuration you want to use and merging the root config under the hood can lead (read: has lead) to confusion for some (read: me) :-)

If this were to happen, it would be a breaking change and would require a major version bump.


As an aside, I think something that would have helped me a great deal is some way to tell ESLint where the rules it's using came from. I realize this could be quite complicated due to the way ESLint merges rules, but I wonder if it may be as simple as monkey-patching the filename to each rule when it's merged. And then providing some kind of flag to have ESLint print out the source of each configuration option. This would be very handy in many more scenarios. If you like this idea, let me know and I'll file another issue for discussion on it.

@eslintbot
Copy link

@kentcdodds Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 8, 2016
@ilyavolodin ilyavolodin added enhancement This change enhances an existing feature of ESLint core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jan 8, 2016
@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible bug ESLint is working incorrectly and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint labels Jan 8, 2016
@nzakas
Copy link
Member

nzakas commented Jan 8, 2016

I think this is a bug. The home directory config is only supposed to be used when there's no other config. I think we get confused because the command line config isn't accounted for in our current check, but I think it would make more sense that the home directory config is only used when there is no other config at all.

FWIW, you can use the --debug flag rather than hacking in console.log.

@kentcdodds
Copy link
Contributor Author

Ah, should have investigating existing debugging options before hacking away 👍

@gyandeeps
Copy link
Member

I think it would make more sense that the home directory config is only used when there is no other config at all

@nzakas Based on a lot of previous discussions specially root: true discussion, we said we will always keep looking for .eslintrc up the chain all the way till home directory and we will stop only when we have a config file with root set to true.

I am fine if we want to change this behavior but just to dig deep into what this change means. Are you saying we will keep looking for files up the chain (as we already do) but will stop right before the home directory? (Let me know if I didnt explain myself well here).

I just want to think this all the way through keeping in mind what we do vs what we are trying to do. I hope I understood the proposed solution correctly 😄

@btmills
Copy link
Member

btmills commented Jan 8, 2016

@gyandeeps that's a good point for clarification. Right now, there are two ways ~/.eslintrc could be included:

  1. Your project is in a directory underneath your home directory, e.g. ~/code/project/, and its config files do not specify root: true. ~/.eslintrc is included as part of a traversal up to /.
  2. Your project is outside of your home directory and has no config files at all. ESLint jumps over and grabs ~/.eslintrc.

If I understand this correctly, this is dealing with case (2), where the project was not under the home directory, and there were no .eslintrc files, but a config file was specified using the CLI -c option. Per @nzakas' comment, ESLint incorrectly jumped over to grab ~/.eslintrc because it didn't find one on the way up to /, but it should have considered the -c config as sufficient without needing to grab ~/.eslintrc.

Did I get that right?

@kentcdodds
Copy link
Contributor Author

Perfect summary of what I experienced

@gyandeeps
Copy link
Member

Thanks a lot @btmills for a very good explanation.
Based on case 2, it makes sense to make the proposed change.

@nzakas
Copy link
Member

nzakas commented Jan 8, 2016

👍

@alberto
Copy link
Member

alberto commented Jan 8, 2016

Why does is make any difference whether the project is inside $HOME or not?

If I understand correctly the purpose of ~/.eslintrc is to have a global config you want always applied, unless you set root: true in another config file up in the file tree. If that was not desired in this case, that config file should have that flag set, shouldn't it?

@nzakas
Copy link
Member

nzakas commented Jan 8, 2016

Because if you're in $HOME, then ~/.eslintrc is a project file, not a personal file (this happens on build systems).

The purpose of ~/.eslintrc is to have a config applied to any files that don't already have a config (no project-level configuration files).

@alberto
Copy link
Member

alberto commented Jan 9, 2016

I did some tests and couldn't reproduce case 1 mentioned by @btmills . ~/.eslintrc is used when no other config is found in the current dir or their parents, regardless of whether the project folder is inside or outside $HOME. Which coincides with your last comment @nzakas

@nzakas
Copy link
Member

nzakas commented Jan 9, 2016

Yeah, root:true is not a part of the equation here. That only applies below the home directory.

ilyavolodin added a commit that referenced this issue Jan 10, 2016
Breaking: don't load ~/.eslintrc when using --config flag (fixes #4881)
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects
None yet
Development

No branches or pull requests

7 participants