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

Do not require modules with shareable config to be named eslint-config-* #9188

Closed
dotchev opened this issue Aug 30, 2017 · 10 comments
Closed
Labels
core Relates to ESLint's core APIs and features 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

@dotchev
Copy link

dotchev commented Aug 30, 2017

In our team we use eslint with a bunch of projects. Naturally we want to use the same eslint configuration in our projects. To do that we have created a separate package with the shared .eslintrc. We include it as dev dependency in our projects and pass its .eslintrc on the command line via -c option.
This works but then we found that we cannot use local .eslintrc files to extend/override the shared configuration.
Now I have found Shareable Configs which is a great idea. Unfortunately it requires that "the module name begins with eslint-config-". This seems rather restrictive. To use it, we would have to rename our package with shared config, which would be a breaking change.

Why not make this an optional prefix?
For example if we have

extends: xyz

Then search for package:

  1. eslint-config-xyz
  2. xyz
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Aug 30, 2017
@not-an-aardvark not-an-aardvark added core Relates to ESLint's core APIs and features 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 and removed triage An ESLint team member will look at this issue soon labels Sep 2, 2017
@not-an-aardvark
Copy link
Member

Thanks for the proposal. I'm pretty sure there was a reason for requiring configs to be prefixed with eslint-config-, but I'm not sure what that reason was, so I'll let someone who has been around longer address whether this would be a good idea.

Keep in mind that you can also specify a relative path in the extends clause. So as a workaround, I think you could do something like this:

extends: './node_modules/my-config'

@klaasman
Copy link

klaasman commented Sep 22, 2017

It looks like this issue is related: #2415

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 10, 2018

@not-an-aardvark it'd be nice if when a path to a specific file was provided, the prefix wasn't added. Specifically, package/path/to/file.js.

@not-an-aardvark
Copy link
Member

That makes sense, although I think some people use files in shareable configs which are intended to be resolved in that manner. For example, if I put browser.js in the root of eslint-config-foo, then I could do something like extends: foo/browser to easily reference a browser-specific set of rules.

As a workaround, you could use extends: ./node_modules/package/path/to/file.js.

@platinumazure
Copy link
Member

@eslint/eslint-team Could someone comment on why we required sharable configs to start with eslint-config- in all cases, even when using path resolution?

I'm pretty sure there was a reason for requiring configs to be prefixed with eslint-config-, but I'm not sure what that reason was, so I'll let someone who has been around longer address whether this would be a good idea.

@ilyavolodin
Copy link
Member

I don't remember the exact reasoning behind having a prefix, but when shareable configs were created, one of the reasons was discoverability, as well as a way to differentiate between configs and plugins. Also I think we used prefixes to avoid naming collisions.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 10, 2018

All of that makes perfect sense, but doesn't necessitate the prefix when including a relative file path :-)

@yhaskell
Copy link

yhaskell commented May 8, 2018

Also it breaks everyone who use one repo for all linters (not only for eslint) in the same repo. Plus it actually breaks the nodejs resolution which is a bad practice in the first place.

@nzakas
Copy link
Member

nzakas commented Oct 19, 2018

Unfortunately, it looks like there wasn't enough interest from the team or community to implement this change. While we wish we'd be able to accommodate everyone's requests, we do need to prioritize. We've found that issues failing to be implemented after 90 days tend to never be implemented, and as such, we close those issues. This doesn't mean the idea isn't interesting or useful, just that it's not something the team can commit to.

@nzakas nzakas closed this as completed Oct 19, 2018
@tunnckoCore
Copy link

tunnckoCore commented Nov 22, 2018

Absolutely agree with @yhaskell. It seems that I'm not the only one, which is cool. Recently i created one scoped package to host all the configs for all the linters and tools.

For example, I have @tunnckocore/config and there are all the configs on the root (so extension can be omitted)

eslint

extends: @tunnckocore/config/eslint

babel

extends: @tunnckocore/config/babel

typescript

extends: @tunnckocore/config/typescript

nyc

extends: @tunnckocore/config/nyc

Before few months I dreamed about:

Current behavior is wrong, for sure. In reality it should be easy:

  1. Check if it starts with the prefix eslint-config- -> try to load
  2. Check if it is @your-scope/eslint-config -> try to load
  3. If it is correct relative path, e.g. starting with . -> if so, depending on if there is extension:
  • if has extension -> try to load
  • if not have extension -> try to load all regular variants of configs
  1. otherwise try to load as usually with the original require mechanism, so all the following should work
  • foo-bar-baz & @scope/pkg - load package's main field (why not try the module too?)
  • foo-bar-baz/some/path/to/config.js - load the file
  • foo-bar/eslint load the eslint.js file from the root of foo-bar
  • @scope/pkg/path/to/config - load

Meeh, isn't it pretty intuitive and making sense? Everything which was working until now will work or I'm wrong? Yes, it is breaking change, because someone may depend on extends: src/foo.js which tries to load eslint-config-src/foo.js.. but.. yea. In my proposal, it will try to load it too, but if eslint-config-src is not found, it will try to fallback to the foo.js file in src package.

For last few months I'm thinking to make a wrapper package that create a CLI wrapper around some tool. And so, it willl search/resolve the configs that way and pass the the full filepath as a --config flag.
For example: @tunnckocore/eslint which will wrap the ESLint's CLI and pass to it a --config flag with the found config path.

@eslint eslint locked as resolved and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Relates to ESLint's core APIs and features 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

10 participants