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

commented Dec 27, 2018

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

@platinumazure
Copy link
Member

left a comment

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

@nzakas
Copy link
Member

left a comment

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.

not-an-aardvark added some commits Jan 15, 2019

@not-an-aardvark

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2019

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

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

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

@nzakas

nzakas approved these changes Jan 29, 2019

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 29, 2019

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

@kaicataldo
Copy link
Member

left a comment

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).

This comment has been minimized.

Copy link
@kaicataldo

kaicataldo Jan 29, 2019

Member

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

@sheerun

This comment has been minimized.

Copy link

commented Feb 1, 2019

Finally something eslint agrees on :)

@not-an-aardvark

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2019

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

3 checks passed

commit-message PR title follows commit message guidelines
Details
licence/cla Contributor License Agreement is signed.
Details
release-monitor No patch release is pending
Details

@not-an-aardvark not-an-aardvark deleted the 2018-simplified-package-loading branch Feb 5, 2019

not-an-aardvark added a commit to eslint/eslint that referenced this pull request Feb 13, 2019

Breaking: simplify config/plugin/parser resolution (fixes #10125)
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

Breaking: simplify config/plugin/parser resolution (fixes #10125)
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

Breaking: simplify config/plugin/parser resolution (fixes #10125)
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

Breaking: simplify config/plugin/parser resolution (fixes #10125)
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

Breaking: simplify config/plugin/parser resolution (fixes #10125)
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.

@nzakas nzakas referenced this pull request Mar 11, 2019

Closed

TSC meeting 14-March-2019 #122

not-an-aardvark added a commit to eslint/eslint that referenced this pull request Apr 4, 2019

Breaking: simplify config/plugin/parser resolution (fixes #10125) (#1…
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.