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

New: Simplify the resolution of third-party plugins/configs/parsers #7

Merged
merged 3 commits into from
Feb 5, 2019

Conversation

not-an-aardvark
Copy link
Member

Summary

This change would update ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This would simplify ESLint's package-loading, resulting in fewer confusing errors about missing packages. It would also fix an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Related Issues

Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far, subject to the resolution of the open questions. Thanks!

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good. I like that it eliminates a lot of the confusion around global vs. local installation of ESLint. In my mind, the breaking changes are worthwhile to eliminate that confusion. I just left a couple of questions.

designs/2018-simplified-package-loading/README.md Outdated Show resolved Hide resolved
designs/2018-simplified-package-loading/README.md Outdated Show resolved Hide resolved
@not-an-aardvark
Copy link
Member Author

Updated to make the requested changes. I decided to keep the term "project root" at least for now, since the choice to make it equivalent to the CWD is an explicit design choice worth reviewing as part of the RFC. If the RFC gets approved, we can just refer to it as the CWD in documentation without introducing the term "project root".

@not-an-aardvark
Copy link
Member Author

@eslint/eslint-tsc I'm moving this PR into the final commenting period, as described in the lifecycle documentation here.

@not-an-aardvark not-an-aardvark added Final Commenting This RFC is in the final week of commenting and removed Initial Commenting This RFC is in the initial feedback stage labels Jan 29, 2019
@nzakas
Copy link
Member

nzakas commented Jan 29, 2019

I think this is a good near-term solution to some of the confusion we have with configs right now. 👍

Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited for this! I think this will make all this behavior much more intuitive.

## Alternatives

* Along with configs/parsers, ESLint could load plugins relative to the configs that depend on them. That would introduce the possibility of having two plugins with the same name, requiring a disambiguation mechanism for configuring that plugin's resources; see [RFC #5](https://github.com/eslint/rfcs/pull/5) for an example. This RFC is intended to be a simpler step to solve some problems, while allowing additional work to proceed on that problem if desirable.
* To mitigate some compatibility impact, ESLint could fall back to the existing behavior when it fails to load a package using the new behavior, instead of throwing a fatal error. However, this would likely cause more confusion because users wouldn't understand why ESLint was finding their packages in certain cases, which would make it more difficult to debug problems. This fallback is only provided for the `espree` package (which falls back to the default parser if no user-installed version of the pacakge is found).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with having less magic and making this easier to debug 👍

@sheerun
Copy link

sheerun commented Feb 1, 2019

Finally something eslint agrees on :)

@not-an-aardvark
Copy link
Member Author

The final comment period has been going on for over 7 days and there are 6 TSC members in favor with none opposed, so this RFC is accepted.

@not-an-aardvark not-an-aardvark merged commit 3a2cac4 into master Feb 5, 2019
@not-an-aardvark not-an-aardvark deleted the 2018-simplified-package-loading branch February 5, 2019 18:14
not-an-aardvark added a commit to eslint/eslint that referenced this pull request Feb 13, 2019
This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7.
not-an-aardvark added a commit to eslint/eslint that referenced this pull request Feb 13, 2019
This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7.
not-an-aardvark added a commit to eslint/eslint that referenced this pull request Feb 19, 2019
This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7.
not-an-aardvark added a commit to eslint/eslint that referenced this pull request Feb 22, 2019
This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7.
not-an-aardvark added a commit to eslint/eslint that referenced this pull request Feb 22, 2019
This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7.
not-an-aardvark added a commit to eslint/eslint that referenced this pull request Apr 4, 2019
…1388)

This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7.
@mysticatea mysticatea added the implemented This RFC has been implemented label Oct 14, 2019
maxwell-k added a commit to maxwell-k/applications that referenced this pull request Mar 10, 2020
Prefer to install inside node_modules of each project.

A [RFC] for eslint proposed simplifying the process of package loading.
This change was implemented in eslint 6.0.0 and [reduced the level of
support] for
installing plugins globally

A ["highly dangerous and ugly package"] was published as a workaround.

At the same time eslint addressed [another issue] about loading
packages.

After [this change] packages are loaded relative to where they are specified in
a configuration file.

One simple way for the configuration and the location of the
configuration files to match is to install from the same `package.json`.

[another issue]: eslint/eslint#10125
["highly dangerous and ugly package"]:
  https://github.com/wintercounter/eslint-global-patch
[rfc]: eslint/rfcs#7
[reduced the level of support]:
  https://eslint.org/docs/user-guide/migrating-to-6.0.0#plugins-and-shareable-configs-are-no-longer-affected-by-eslints
[this change]:
  eslint/eslint@25cc63d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Final Commenting This RFC is in the final week of commenting implemented This RFC has been implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants