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

Make --reset the default behavior, remove flag #2100

Closed
mickaeltr opened this issue Mar 18, 2015 · 58 comments

Comments

Projects
None yet
@mickaeltr
Copy link
Contributor

commented Mar 18, 2015

Hello,

I have the following use case:

  • prevent the use of console.* calls in the whole codebase
  • in a specific folder, check all default eslint rules + some specific rules

So what I first did is call eslint --reset with:

  • a simple .eslintrc at the root, with {"rules": {"no-console": 2}} only
  • a .eslintrc in the specific folder, with some specific rules

The problem is that the --reset flag prevents from applying the default eslint rules on the specific folders.

I see 3 options:

  1. Call eslint twice: one time with --reset, another time on the specific folder with --reset
  2. Disable all rules manually in the root .eslintrc
  3. Enable all rules manually in the specific folder .eslintrc

Those options are not really desirable, so I am thinking that it would be nice to be able to reset all rules in the .eslintrc file, so it could be overriden by more specific configuration files below.

What do you think?

@nzakas nzakas added the triage label Mar 18, 2015

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 18, 2015

I'm not sure how feasible it is to introduce reset into the middle of a configuration hierarchy. Can you explain more about why you need this? Why would you want reset to apply to only some files?

@mickaeltr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2015

My use case is the following:

  • / => apply no-console rule (only) in the root and all children folders
  • /specific/ => apply default eslint rules + some specific rules

If I disable the default rules at the root with --reset, these default rules won't apply to my specific folder.

So my best solution so far is to run twice eslint:

  • / => eslint --reset …
  • /specific => eslint …

Thus, the idea is to be able to reset the rules from the configuration file, not only from the command line. I would then end up with something like:

  • /.eslintrc => {"reset": true, "rules": {"no-console": 2}}
  • /specific/.eslintrc => {"reset": false, "rules": {…}}
@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 18, 2015

You just repeated your original description. I'm asking why you want to do this? I'd like to understand what situation you're dealing with that led you to want to do this because this is the first time sometime has presented such a case and I need to better understand the rationale to see if it's worth introducing a change or if there's another option.

@mickaeltr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2015

I work on a legacy project and we want to gradually improve it.
We want to apply a minimal set of very important rules to the whole project (at the root).
We want to apply stricter rules in newly created modules (in specific folders).

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 18, 2015

Okay, so it's not that you want to turn off all rules in a particular directory, it's that you want to enable all rules in a particular directory. I think this might be better served by config extension (as described in #1637).

@puzrin

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2015

Supporting --reset from config is useful to simplify eslint calls from different programs. For example, many editors have plugins to run linters. They catch configs automatically, but need explicit directive for --reset.

NB. it's enougth to have such --reset support only for top level config.

@mickaeltr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2015

My first use case was to apply a single rule only (no-console). I first wrote a big .eslintrc file disabling all other default rules. Then I figured out that the combination of .estlintrc {"rules": {"no-console": 2}} + --reset option would be simpler.

Although, I would strongly prefer a self sufficient configuration file.

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 18, 2015

@mickaeltr you can already create a self sufficient config file, just use --reset and then put in exactly what you want in your config file.

@mickaeltr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2015

Oh yes, I figured it out. :)

By self sufficient, I meant a configuration file that does not need any specific option (such as --reset) to work as required. As @puzrin mentionned, it would facilitation the use of eslint from different programs (command line, code editors).

@pesla

This comment has been minimized.

Copy link

commented Apr 9, 2015

I would like to be able to reset all rules from .eslintrc as well. I'm using JSCS to run code style checks and want ESLint to perform some tasks that JSCS can't. It's annoying to have ESLint perform the same tasks as JSCS just because these rules are turned on by default. Running ESLint with the --reset flag isn't an option, as these processes are run from within my IDE without support for setting flags or whatsoever.

Some out there are actually creating ESLint Resets to have some control, but this looks like maintenance hell.

Anyway +1 for the suggestion of @mickaeltr.

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 9, 2015

The problem is that configs cascade, so resetting in one config could affect others. We apply the reset before the first config is read, and then use each subsequent config to override those settings. If we allow reset in a config, then at any point all previous configs will be blown away. This is why it must be an option on the command line - it would completely break config cascade.

@puzrin

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2015

reset does not need cascading. It's needed in root config only.

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 10, 2015

Of course, that's why it's a command line option. :) Saying that just one option only applies when the file is in a specific place is confusing. We don't even really have the concept of a root, since we keep searching up for .eslintrc files.

@puzrin

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2015

Saying that just one option only applies when the file is in a specific place is confusing.

Yes, that's not good, but better than nothing. I think, root location is not diificult to formalize. That's topmost file from cwd or explicitly defined in command line.

@pesla

This comment has been minimized.

Copy link

commented Apr 10, 2015

In the meantime I have "fixed" our issue by using ESLint as our primary linter using a config file with all rules explicitly set to 0, 1, or 2. JSCS comes in to check some additional stuff (and doesn't use defaults that cause side effects).

I see the complexity of the issue, BTW. At least there is some demand for the ability to reset defaults to off. I'm pretty sure that there are enough users that would benefit to justify a (new) feature to do so.

Anyway, thanks for your hard work in ESLint, awesome tool!

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 10, 2015

Yup, I hear the request, I'm just not sure there's a rational way to implement.

@rlidwka

This comment has been minimized.

Copy link

commented Apr 10, 2015

In the meantime I have "fixed" our issue by using ESLint as our primary linter using a config file with all rules explicitly set to 0, 1, or 2.

I tried the same approach for a while. But it turned out to be too painful to change config after each eslint update (it usually has new rules to turn off).

Frankly, I think --reset should be the default. Maybe add presets on top of it for whoever wants them.

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 10, 2015

That's an interesting idea. My concern is that there's a belief that something should work "out of the box" in some meaningful way. If we turn off all rules from the start, we lose that completely and people are left needing to manually configure every single rule they want. While that would be fine for power users, it's ominous for everyone else.

So, I'm not opposed to having --reset be the default at some future point, but we'd definitely need some way to get people bootstrapped before we could make such a drastic change.

@puzrin

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2015

but we'd definitely need some way to get people bootstrapped before we could make such a drastic change.

  • built-in option to create config or use some defaults.
  • warning when no rules, with recommendation what to do.

@nzakas nzakas changed the title Reset all rules from .eslintrc Make --reset the default behavior, remove flag May 1, 2015

@nzakas nzakas added this to the v1.0.0 milestone May 1, 2015

@nzakas

This comment has been minimized.

Copy link
Member

commented May 1, 2015

Okay, now that we are working on --init, I think we can go ahead and make --reset the default behavior. What this means:

  • Make the default behavior be not to use eslint.json
  • Remove --reset flag
  • Remove reset option from CLIEngine
  • Remove reset from Config
  • Add a question to --init "Do you want to use the rules ESLint recommends?" Of yes, then use eslint.json as the base for the created config
  • Change docs so the rules that are on by default have "(recommended)" next to them; remove "(off by default)" from all rules
  • Update build script to verify "(recommended)" is present on appropriate rules
@ilyavolodin

This comment has been minimized.

Copy link
Member

commented May 1, 2015

I think I somehow missed this discussion completely. But I'm not sure I buy the idea of reset by default. There are probably over 200-300 different projects that rely on eslint already. With this change, each and every one of them will have to update their .eslintrc file in order to enable just a few people who are having a problem currently.
I would much rather see a reset option inside .eslintrc file that will reset any upstream configs all together.

@pascalduez

This comment has been minimized.

Copy link

commented May 1, 2015

Indeed, this sounds a bit concerning. Given the high number of projects we are using ESLint on, what would that imply for existing implementations ?

One of the base feature I got to love about ESLint is this ability to have it working out of the box, and inherit all default rules, just overriding the needed ones (which turns out to be few).
That way we keep our .eslintrc small.

If it's just about adding an "extends": "default" in existing config files then that's completely fine :-)

@MoOx MoOx referenced this issue May 8, 2015

Merged

Add naming rules doc #13

@xjamundx

This comment has been minimized.

Copy link
Contributor

commented May 8, 2015

How about we add a small block of code, that would compare current config object against list of built-in defaults and output a warning if the rule was on by default before and is currently off without explicit setting from the user

Or maybe a helpful command-line switch would be eslint --missing-rules which would report any possible rules that have been not configured locally in an .eslintrc file. I know with the large number of new rules I often miss them. Most of the new rules generally are off by default, so that might be nice.

Either of these could be done as 3rd party tools though if we felt they really were that important.

As far as the topic at hand:

I think people will definitely be put off by the change, but in the long term I think we can say with confidence that we're a non-opinionated linting tool and actually mean it. Combined with providing an easy way to extend existing style guides I think this will actually turn out to be a killer feature ;-)

Let me know how I can help....

@nzakas

This comment has been minimized.

Copy link
Member

commented May 8, 2015

I'd like to wait and see what sort of issues we run into before spending time on solutions. We can imagine 100 ways to get people through the transition and be wing because we are theorizing about it. Then we are stuck with those things for a long time. As I mentioned, we will do release candidates to gather feedback, just like with es6jsx, and use that to figure out what we need to do to help people.

We will leave this change for last, so the best help would be to finish up the 1.0.0 milestone issues (rules first, then the other stuff).

@glenjamin

This comment has been minimized.

Copy link
Contributor

commented May 9, 2015

I have a proposal which maintains backwards compatibility a bit more, but still achieves the same goals.

  1. Implement extends:eslint and extends:none
  2. default to extends:eslint if .eslintrc is being parsed
  3. Begin warning if extends is not specified
  4. have --init insert extends:none into the generated config (or a choice, as suggested above)
  5. wait
  6. default to extends:none

I think this might be a smoother path?

I'd also suggest that "recommended" sounds somewhat opinionated, but as we all know: bikeshedding over naming can go on forever :)

@nzakas

This comment has been minimized.

Copy link
Member

commented May 9, 2015

@glenjamin extends:none has the same problem as putting reset directly in the config (see earlier comments), and defaulting to extends:eslint is also problematic from a cascade point of view. I appreciate the feedback, but this path is pretty much set at this point.

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented May 20, 2015

One more thing that I just though of. Would --reset by default disable plugin defaults as well? I'm pretty sure right now it doesn't (I don't remember writing any code for that, when I implemented plugin defaults).

@xjamundx

This comment has been minimized.

Copy link
Contributor

commented May 20, 2015

I assume they shouldn't apply to plugins, because those are already opt-in.

@nzakas

This comment has been minimized.

Copy link
Member

commented May 20, 2015

Hmm, I'd say it shouldn't apply plugin rules, either.

@fakewaffle

This comment has been minimized.

Copy link
Contributor

commented May 28, 2015

I agree with @nzakas. Having default rules set has been very annoying. We have 70+ repos/projects that rely on ESLint

@btmills

This comment has been minimized.

Copy link
Member

commented Jul 1, 2015

Working on this.

btmills added a commit that referenced this issue Jul 3, 2015

btmills added a commit that referenced this issue Jul 4, 2015

btmills added a commit that referenced this issue Jul 10, 2015

btmills added a commit that referenced this issue Jul 10, 2015

btmills added a commit that referenced this issue Jul 10, 2015

btmills added a commit that referenced this issue Jul 10, 2015

btmills added a commit that referenced this issue Jul 10, 2015

btmills added a commit that referenced this issue Jul 10, 2015

@nzakas nzakas closed this in #2915 Jul 10, 2015

nzakas added a commit that referenced this issue Jul 10, 2015

Merge pull request #2915 from eslint/issue2100
Breaking: Default to --reset behavior (fixes #2100)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.