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

ESLint should respect XDG basedir spec when finding user-level .eslintrc #6008

Closed
jasonkarns opened this issue Apr 29, 2016 · 7 comments
Closed
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible cli Relates to ESLint's command-line interface enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@jasonkarns
Copy link

  1. The version of ESLint you are using.
    eslint v2.8.0
  2. The problem you want to solve.
    eslint does not respect the XDG basedir specification which defines where user-specific configuration files should be located.
  3. Your take on the correct solution to problem.
    Since eslint already expects $HOME/.eslintrc, it needs to continue supporting that. (At least until a major version bump.) I propose:

XDG_CONFIG_DIRS is a colon (:) separated list of paths under which config files can be found. They are listed by preference (with XDG_CONFIG_HOME taking precedence over them all). As it seems that eslint currently does not merge local .eslintrc file with ~/.eslintrc, I propose those semantics remain and carry over to the config dir lookup. So eslint would search $XDG_CONFIG_HOME/eslint/config then each of the paths from XDG_CONFIG_DIRS (suffixed with /eslint/config) taking the first file it finds (not merging if multiple files are found). If no file is found, fall back to ~/.eslintrc as before.

Also note, both XDG_CONFIG_HOME and XDG_CONFIG_DIRS have default values specified (~/.config and /etc/xdg, respectively), so it is not sufficient to check for the env vars being set.

Luckily, @sindresorhus has published xdg-basedir so actually following the logic above is trivial.

require('xdg-basedir').configDirs returns a preference-sorted array, with XDG_CONFIG_HOME already as first element; and with default values for the env vars already applied. We merely need to iterate over the array, searching for eslint/config and take the first one we find (defaulting again to ~/.eslintrc)

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Apr 29, 2016
@nzakas
Copy link
Member

nzakas commented Apr 29, 2016

ESLint doesn't expect $HOME/.eslintrc to be present - that's just one option, and is only used if there are no other config files at the project level.

I'm not familiar with XDG, so let me try to understand what you're saying. Are you saying that when we search for $HOME/.eslintrc, we should instead search through XDG directories plus $HOME to find a config file rather than just looking at $HOME?

@nzakas nzakas added enhancement This change enhances an existing feature of ESLint cli Relates to ESLint's command-line interface 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 Apr 29, 2016
@qfox
Copy link

qfox commented Apr 29, 2016

There is also rc module that respect XDG: https://github.com/dominictarr/rc/blob/master/index.js#L37-L47

I guess we can use any good module to reach this logic.

@nzakas
Copy link
Member

nzakas commented Apr 29, 2016

Just realized that this would be a breaking change, since putting $HOME/.eslintrc at the bottom of a priority list with other directories could affect people who had a .eslintrc file in one of those other directories (currently this would be ignored and go directly to $HOME/.eslintrc.

@nzakas nzakas added the breaking This change is backwards-incompatible label Apr 29, 2016
@nzakas nzakas mentioned this issue Jun 9, 2016
7 tasks
@nzakas
Copy link
Member

nzakas commented Jun 10, 2016

@jasonkarns there are a few open questions I asked on this. Can you take a look?

@jasonkarns
Copy link
Author

jasonkarns commented Jun 14, 2016

Sorry for the inactivity...

Are you saying that when we search for $HOME/.eslintrc, we should instead search through XDG directories plus $HOME to find a config file rather than just looking at $HOME?

The goal of the XDG basedir spec is to replace the role of $HOME as a place for configuration files. So the ideal scenario is that the $XDG_CONFIG_HOME and $XDG_CONFIG_DIRS are searched instead of $HOME. However, as a means of transitioning from current state to this future state, many projects still check in $HOME as a fallback.

and is only used if there are no other config files at the project level.

Does this mean that eslint does not merge project-level and user-level ($HOME) config files, but rather user-level config files are completely ignored in favor of project-level config files (if they exist)? I ask to see if there is prior art within eslint for merging if multiple config files are found during lookup.

Just realized that this would be a breaking change

I suppose it could be looked at this way, though I feel that's a bit of a stretch. (There's no change to the API, for one.) Personally, I would consider this a new feature. Further, the likelihood that anyone has existing eslint directories under any of the XDG-defined paths is astronomically low; let alone having an existing eslint directory in these paths that also contains a config.{js,json,yaml} file.

could affect people who had a .eslintrc file in one of those other directories

The common practice when following the XDG basedir spec is to expect a tool-specific folder (eslint/) under the XDG paths which contains a config.{js,json,yaml} file.

So, assuming defaults, one would use ~/.config/eslint/config.{js,json,yaml} ($XDG_CONFIG_HOME/eslint/config) instead of ~/.eslintrc. This illustrates the near-0 likelihood that anyone has an existing file that would take precedence over ~/.eslintrc.

@platinumazure
Copy link
Member

platinumazure commented Jun 14, 2016

Does this mean that eslint does not merge project-level and user-level ($HOME) config files, but rather user-level config files are completely ignored in favor of project-level config files (if they exist)? I ask to see if there is prior art within eslint for merging if multiple config files are found during lookup.

To answer your first question: User-level config files are ignored completely in favor of project config files if project config files are found.

Regarding configuration merge: When linting a file, we first look for an .eslintrc.{json,yaml,js} (or .eslintrc with no extension for backward compatibility) in the same directory as the file being linted. Then we look in the parent directory, then its parent, etc., until

  1. We reach the root directory
  2. We find a configuration file which has "root": true specified inside

And with each config found in ancestor directories, we merge those configurations, with child-directory configurations having higher precedence.

Only if we find no configuration files, either in the project or in ancestor directories, we will look in $HOME and we will not do any merging.

@alberto
Copy link
Member

alberto commented Aug 1, 2016

Thanks for your interest in improving eslint. Unfortunately, it looks like consensus couldn't be reached on this issue and so I'm closing it. While we wish we'd be able to accommodate everyone's requests, we do need to prioritize. We've found that issues failing to reach consensus after 21 days tend never to reach consensus, and as such, we close those issues. This doesn't mean the idea isn't interesting, just that it's not something the team can commit to.

@alberto alberto closed this as completed Aug 1, 2016
@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
archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible cli Relates to ESLint's command-line interface enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

6 participants