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

Update: Personal Config Loads Global-Installed Packages #28

Closed
wants to merge 4 commits into from

Conversation

@mysticatea
Copy link
Member

mysticatea commented Jun 27, 2019

open Rendered RFC

Summary

This RFC allows the personal config to load also global-installed packages (shareable configs, parsers, and plugins).

Related Issues

mysticatea added 3 commits Jun 27, 2019
@mysticatea mysticatea changed the title Update: Personal Config Loads Global-Installed Paclages Update: Personal Config Loads Global-Installed Packages Jun 27, 2019
@ljharb

This comment has been minimized.

Copy link

ljharb commented Jun 27, 2019

I think this is a bad idea - eslint shouldn’t be encouraging anything to be installed globally, and having a personal config imo is also a bad practice. If a project doesn’t have eslint configured, eslint shouldn’t be run on it.

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jun 27, 2019

I agree with @ljharb's concern about encouraging the use of global installation. That being said, we do currently support the use case of having a personal config in the user's home directory, and this does seem like it would help those who use it out.

I know we've hashed this out a lot over the years, but I wonder it it would be worth digging into this discussion again (though it doesn't have to be in this RFC!). I wonder if we could avoid a lot of confusion if we just didn't support running ESLint globally (showing a helpful error message letting the user know they should install locally) and instead offered something like eslint-cli for those who really need a globally accessible eslint in their PATH. That way, eslint would always be running locally per project - it's just being invoked through different means.

@not-an-aardvark

This comment has been minimized.

Copy link
Member

not-an-aardvark commented Jun 27, 2019

I don't think we should do this. The idea of a "global installation" isn't well-defined by Node -- it's something defined by npm that doesn't always exist for other package managers. I think we should avoid any kind of "magic" that tries to make guesses about the user's package management strategy.

I also agree with @ljharb that we shouldn't encourage the use of global installations/home-directory configs.

@platinumazure

This comment has been minimized.

Copy link
Member

platinumazure commented Jun 27, 2019

Yes-- I would prefer we either have personal configs resolve from CWD, leave them as they are, or remove support for personal configs entirely in 7.0.

@mysticatea

This comment has been minimized.

Copy link
Member Author

mysticatea commented Jun 28, 2019

I agree with the personal config is a bad practice. But it's supported now.

The problem that this RFC wants to resolve is that people have to install dependencies into their home directory when they use the existing personal config functionality.

My position is that it's pretty worse than global installation. Because people have often had their project directories in their home directory, so those projects can load the packages in the home directory in mistakes.

This problem was not in ESLint 5 and older.

I strongly don't want to encourage to install dependencies into the home directory instead of global installation.

@aladdin-add

This comment has been minimized.

Copy link
Member

aladdin-add commented Jun 29, 2019

is personal config documented somewhere? if yes, we might have to fix it in eslint v6, and give user a warning so we can remove it in eslint v7/v8.

@not-an-aardvark

This comment has been minimized.

Copy link
Member

not-an-aardvark commented Jun 29, 2019

I agree with the personal config is a bad practice. But it's supported now.

The problem that this RFC wants to resolve is that people have to install dependencies into their home directory when they use the existing personal config functionality.

My position is that it's pretty worse than global installation. Because people have often had their project directories in their home directory, so those projects can load the packages in the home directory in mistakes.

I agree that this isn't ideal. However, I don't think the solution is worthwhile.

  • This same problem exists for any other Node files in the user's home directory -- if I have a file in ~/foo.js that calls require("lodash"), then installing lodash can cause the lodash installation to be reachable for other projects. This is a disadvantage of the home directory in general -- it's difficult to manage dependencies
  • Conceptually, there isn't any link between home directories and global installations. A "global installation" just means that a package is installed in a special folder known to the npm client, which is also added to the user's $PATH. It's not intended for dependencies of files in the filesystem.

It seems like the reasoning for this RFC boils down to saying, "It's inconvenient to manage dependencies for files in a home directory, so we should unilaterally designate another folder to install dependencies of those files instead". But this could end up being confusing, because it would no longer be the case that shareable configs are loaded the same way as calling require from the config that loads them. Since home-directory configs always have dependency management problems, I'm not sure it's necessary for us to solve them here when users have better alternatives anyway.

Personally, I would rather deprecate home-directory configs instead and keep encouraging people to use local installations. Home-directory configs can seem convenient at first, but they cause confusion when working on projects with other people.

is personal config documented somewhere? if yes, we might have to fix it in eslint v6, and give user a warning so we can remove it in eslint v7/v8.

Personal configs are documented. However, this proposal isn't a bugfix -- it's an additional feature to try to improve the ergonomics of personal configs. So I don't think we have to fix it in ESLint v6.

@webOS101

This comment has been minimized.

Copy link

webOS101 commented Jul 12, 2019

It would be a real shame to lose home level (personal?) configs. Having this means that I can lint any JS file on my computer, regardless of whether it's part of an npm-managed repo or not. Not every use of eslint should have to be part of an npm (or yarn, or some other package-manager) project.

@ljharb

This comment has been minimized.

Copy link

ljharb commented Jul 12, 2019

I'd disagree; I think that all code should be in a project, and that if the project doesn't lint the files, they should not be linted at all.

@webOS101

This comment has been minimized.

Copy link

webOS101 commented Jul 12, 2019

Perhaps the tagline on the web site should be updated:

ESLint
The pluggable linting utility for JavaScript and JSX inside of npm projects

Every legacy website without bundling tools and package.json files will be cut-off.

@ljharb

This comment has been minimized.

Copy link

ljharb commented Jul 12, 2019

Sounds good to me, since all they have to do is add a package.json instead of just magically hoping all the projects developers use the same versions of everything :-)

@webOS101

This comment has been minimized.

Copy link

webOS101 commented Jul 12, 2019

Not everyone works on a team of developers.

@ljharb

This comment has been minimized.

Copy link

ljharb commented Jul 12, 2019

Sure they do - a team of 1 is still you, and "you in 6 months".

@mysticatea

This comment has been minimized.

Copy link
Member Author

mysticatea commented Jul 15, 2019

OK. We, the ESLint team, look to have the consensus that we should deprecate the personal config. Therefore, I will make the RFC that deprecates the personal config rather than I continue to champion this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.