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

Suggestion: Move the "ECMAScript 6" rules into the other categories #7991

Closed
lydell opened this issue Jan 27, 2017 · 45 comments
Closed

Suggestion: Move the "ECMAScript 6" rules into the other categories #7991

lydell opened this issue Jan 27, 2017 · 45 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint

Comments

@lydell
Copy link

lydell commented Jan 27, 2017

On the rules page, there are a number of categories:

  • Possible Errors
  • Best Practices
  • Strict Mode
  • Variables
  • Node.js and CommonJS
  • Stylistic Issues
  • ECMAScript 6

While ES6 used to be special, it has now become "the default", I'd say. Node.js and browsers support most of it these days, and people have become familiar with it.

My suggestion is to remove the "ECMAScript 6" category, and move its rules into the other categories.

Why?

For example, if you're only interested in "Possible Errors", you'll miss out on several helpful rules unless you manually filter through the "ECMAScript 6" category.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 27, 2017
@platinumazure
Copy link
Member

@lydell Thanks for the issue! We've seen a few issues like this. Personally, I agree that the rules list needs an overhaul.

What I think would really work well, though, is to drastically reduce the number of categories and support tags. In my ideal model, rules would be in one of three categories: Possible Errors, Best Practices, and Stylistic Issues. Everything else is a tag and rules can have more than one tag. For example, prefer-const would be Best Practices and have tags "es6" and "variables" (and whatever I might be missing). That way, someone could browse rules by impact level, or use tags to look for topically related sets of rules.

What do you think?

@lydell
Copy link
Author

lydell commented Jan 28, 2017

@platinumazure Your idea sounds great! 👍

@ilyavolodin
Copy link
Member

Tags are probably a good idea for rule exploration. We would need a concrete proposal with the last of tags we want to support before we can move forward with this.

@lydell
Copy link
Author

lydell commented Jan 28, 2017

Just brainstorming some tags:

  • ES5
  • ES2015
  • ES2016 (might be no rules needing it yet, though)
  • IE8
  • Node.js
  • CommonJS
  • Variables
  • Whitespace
  • Naming
  • Restricted names

Extra brainstorming idea: Get rid of all categories, and go with tags only, along with UI to filter the rules.

@platinumazure
Copy link
Member

platinumazure commented Jan 29, 2017

I probably would have started with the current list of categories less the three main categories I had called out earlier, but if there is team consensus, I'd also be okay with starting with more tags.

Here's what I see as the steps to implementing my proposal, if we go with that:

  1. Agree initial list of supported tags
  2. Add "tags" array support to meta.docs
  3. Change all core rules to use main category and add tags as needed
  4. (Optional) Create internal ESLint rule to enforce that all core rules are Possible Errors, Best Practices, or Stylistic Issues
  5. Augment eslint.github.io to support filtering by one or more tags

Let me know if I've missed something or if something isn't clear.

@lydell Sorry for hijacking this issue. Just want to confirm that my proposal still meets your original goal; I think it should, but please let me know if that's not the case.

@lydell
Copy link
Author

lydell commented Jan 29, 2017

@platinumazure I like your proposal, and it does meet my original goal. My original idea was to do the minimal thing to make things better (spread out the ES6 rules over the other categories, without changing anything else), while your proposal is much more ambitious.

@platinumazure
Copy link
Member

platinumazure commented Jan 29, 2017 via email

@vitorbal
Copy link
Member

Could we also remove the ES6 category, but add already a meta.tags property to those rules that used to be part of the ES6 category? We can incrementally add more tags to the other rules, and the UI for filtering can be done separately as a PR for eslint.github.io I imagine.

@vitorbal vitorbal added documentation Relates to ESLint's documentation 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 Jan 30, 2017
@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 30, 2017

I would dispute that ES6 is the default. Most companies still support browsers that are pre-ES6. Virtually every npm module I maintain is written in ES3.

It's only same for ES6 to be the default if you're node-only, or if you use Babel. That's not anywhere close to what the majority of developers are doing yet.

@lydell
Copy link
Author

lydell commented Jan 30, 2017

@ljharb I know that I used "ES6 is now default" as an argument for removing the category, but thinking about it again, is that even a relevant argument? How does your view on what is "default" impact the actual proposals in this issue? I take it as another vote for the "ambitious tags proposal."

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 30, 2017

Your claim is that ES6 used to be special, and is now the default. My claim is that ES6 will be special for many many years to come, and is still special now.

@not-an-aardvark
Copy link
Member

My opinion is that regardless of whether ES6 is a "default", it's still not very useful to have all ES6 rules in their own category. The other categories describe things that the rule does (e.g. enforcing style, enforcing best practices, etc.). The ES6 category indicates that a rule only applies to code that uses certain syntax, which is a different concern and generally overlaps with the other categories.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 30, 2017

If you're suggesting a general change to having rules be able to have N categories/tags, I think that's an improvement, but I'd still want to be able to list them all by "ES6" :-)

@lydell
Copy link
Author

lydell commented Jan 30, 2017

Sounds like there's only one option then: Tags + filter. Then everyone can find the set of rules they're looking for. Is this a good plan, perhaps?

  1. Don't change any categories. Too much bikeshedding.
  2. Add tags to core rules.
  3. Make use of the tags in the eslint.github.io

@platinumazure
Copy link
Member

platinumazure commented Jan 30, 2017

@ljharb @lydell Any particular objection to the proposal I outlined here?

Edit: I'm certainly okay with rearranging the steps so that not everything has to be done at once, or whatnot.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 30, 2017

LGTM!

@vitorbal
Copy link
Member

I'm 👍 for this improvement and I'm willing to champion too. @lydell are you interested in submitting a PR for these changes?

@vitorbal vitorbal self-assigned this Jan 30, 2017
@not-an-aardvark
Copy link
Member

Just to clarify, is the proposal to remove the ES6 category and replace it with an ES6 tag, or are we planning to keep the ES6 category?

@platinumazure
Copy link
Member

platinumazure commented Jan 30, 2017

@not-an-aardvark My proposal is to (eventually) remove all categories except Possible Errors, Best Practices, Stylistic Issues and replace the removed categories with tags. We don't have to do all at once, so for ES6, my proposal is to remove that category and replace with tag.

We can certainly add tag support first and only remove the category later, if we want to wait on eslint.github.io changes to support filtering on tags before removing the category.

@lydell
Copy link
Author

lydell commented Jan 30, 2017

@platinumazure

  1. (Optional) Create internal ESLint rule to enforce that all core rules are Possible Errors, Best Practices, or Stylistic Issues

This seems to be the controversial one. Perhaps we should do tags first, and revisit categorization in the future, as you say.

  1. Change all core rules to use main category and add tags as needed

I'm not really sure what this means?

@vitorbal
Copy link
Member

vitorbal commented Feb 1, 2017

@lydell I guess by 4. he just meant update the categories of all rules to be one of the remaining 3 categories (Possible Errors, Best Practices or Stylistic Issues).

I agree that adding the tags would be a good first step. We could even start by simply adding the "ES6" tag to the relevant rules as the first PR.

However, adding a new property to the rules meta field might require TSC-approval, but I'm not sure. @eslint/eslint-tsc thoughts?

@kaicataldo
Copy link
Member

kaicataldo commented Feb 2, 2017

I personally don't think adding a tags field to meta is a big deal since it's not a breaking change. I'm 👍 for simplifying the categories and adding tag support.

@platinumazure I can't seem to find it now, but wasn't there another issue where we discussed the idea of tags?

@platinumazure
Copy link
Member

@kaicataldo I mentioned the concept here: eslint/archive-website#310

And I think I've mentioned it once or twice on chat, but I can't remember.

@platinumazure
Copy link
Member

platinumazure commented Feb 2, 2017

TSC Summary

Users have experienced confusion in the past looking for useful rules for their ES6 codebase. Users may start in "Possible Errors" looking to implement all of our recommended rules, or they might want to go through the style rules and make choices about those. In either case, a user might miss that we have an entire ES6 section full of extra rules. Furthermore, those rules go across the whole spectrum with regard to how critical the rule might be (possible error vs best practice vs stylistic issue).

I believe our users would be better served by having fewer categories, and those categories should focus on the impact of enabling/disabling the rule (i.e., we should only have Possible Errors, Best Practices, and Stylistic Issues). We can then use "tags" to label rules by topic, and then the website could be augmented with tag filters so that a user could focus on a topic (such as ES6, Variables, Strict Mode) and see all the Possible Error, Best Practice, and Stylistic Issue rules matching that topic.

Other issues where the concept was at least mentioned:

TSC Question

Should we pursue this recategorization in core?

If so:

  • Should we support "tags" in rule metadata, which would be an array of strings?
  • Should we augment the website to support filtering by tags?
  • Should we recategorize rules that are currently in a category like "ES6"? If so, is it preferred that we add tag support and add the ES6 (or whatever) tag, and then we can change the actual category later on?

An early version of this proposal can be found here. I encourage the TSC to decide what steps we actually need.

@platinumazure platinumazure added feature This change adds a new feature to ESLint and removed documentation Relates to ESLint's documentation labels Feb 2, 2017
@platinumazure
Copy link
Member

Changing tag from "documentation" to "core"/"feature" since the proposal included a new rule metadata field.

Next step is to agree on a tags list.

I like this proposed tag list, and we can certainly choose not to use tags if they don't strongly apply to rules. Can we work with this to start? (Although personally, I don't think we need to restrict the tags in the core implementation since plugin authors might want to create their own tags-- we just want to be careful about new tags being added for core rules without proper vetting.)

@kaicataldo
Copy link
Member

Regarding JS version tags (ES6, ES2016, ES2017, etc.), on what basis do we tag rules as such? Example: quotes supports backticks, but is it an ES6 rule?

@ljharb
Copy link
Sponsor Contributor

ljharb commented Feb 2, 2017

I'd say for JS versions you'd want to tag a feature in either every version it works in, or, the version in which it was introduced.

@platinumazure
Copy link
Member

@kaicataldo I'd say that ES2015 tag should only be applied if the primary use of the rule is in ES2015+ codebases (bonus points if it uses ES2015-only syntax). My preference would be to tag only the earliest version a rule is compatible with (i.e., ES2015 rules don't also get ES2016 tags). The idea being a user could choose to apply multiple tag filters on the website, if they wanted to look at both ES6 and ES2016 rules.

@not-an-aardvark
Copy link
Member

Maybe we should say a rule is ES6 if the main purpose of the rule relates to an ES6 feature. For example, template-tag-spacing and prefer-const are ES6 rules, because they are directly related to ES6 features. quotes is not an ES6 rule because it's applicable to ES5 too.

@ilyavolodin
Copy link
Member

Here's my suggestion for steps on how to implement this:

  • Create a list of tags we want to use (doesn't need to be locked down, just something to start with)
  • Go through the rules and add tags
  • Implement filtering on the rules page of the website
  • Go through the rules and remove unnecessary categories.

@alberto
Copy link
Member

alberto commented Feb 2, 2017

We should also try to limit the number of tags per rule to avoid negatively impacting the usability of the page. I am worried a lot of tags on 300 rules can make that page a mess.

Also, should filtering refine or expand results?

@ilyavolodin
Copy link
Member

@alberto Good question. It can do either. But I think refine is probably more natural behavior.

@vitorbal
Copy link
Member

vitorbal commented Feb 3, 2017

I also think it should refine. @ilyavolodin maybe there's also a way for algolia to index the rules by tag, if people try to search for the tagname through the search bar?

@ilyavolodin
Copy link
Member

@vitorbal Hmm... Not sure. I don't know much about Algolia, but I would imagine that if it can do that, we would need to add tags somewhere on the rules pages.

@btmills
Copy link
Member

btmills commented Feb 3, 2017

We could search by tags as text, or treat them as facets. Check out the gifs on that page for more examples.

@nzakas
Copy link
Member

nzakas commented Feb 7, 2017

Can we scale back this conversation a bit? The request was just to distribute rules currently in the ES6 category into other categories as appropriate. Can we get a consensus on that proposal?

I'm 👎 on this tags proposal, as I don't see a ton of value vs. the complexity it introduces. To me, the only significant question we have to answer is if we want rules indicated by minimum language version or not. I'd much rather see something like meta.docs.minVersion that is output on the individual rule page than go through the complexity of adding generic tags everywhere.

Overall, I think we can probably distribute ES6 rules into other categories without making any other changes, as I feel like the language version is a lot less important going forward than it was when the project started.

@platinumazure
Copy link
Member

👍 to moving ES6 rules to other categories.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Feb 8, 2017

@nzakas what about my concerns here? I think that "ES6" still matters a lot, since most people still support browsers that don't natively run ES6.

@lydell lydell closed this as completed Feb 8, 2017
@nzakas nzakas reopened this Feb 9, 2017
@nzakas
Copy link
Member

nzakas commented Feb 9, 2017

Reopening because the discussion is not complete.

@ljharb I don't see "ES6" as important to split out as a rule category. It is special, but only insofar as people set the parser to parse it. Individual rule pages are still filled with references to ES6, and links for more information about language features. So while I agree ES6 is still a bit special, I disagree that being special necessitates a separate rule category.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Feb 9, 2017

I suspect many devs are unaware of what language edition a given feature is in; for pre-ES6, the distinction has mostly faded as ES3 browsers drop off supported browser lists. For ES6+, people tend to use babel, so the distinction doesn't matter. The dividing line between "you probably need babel" and "you don't need babel" is thus "ES6".

I do think it'd be great if all the rules in the ES6 category were also in other categories - but I think having them notated as being "ES6" is important (which is part of why this drifted into the "tags" discussion; because having things be in only one category is like emails being in "folders" - not an accurate reflection of reality)

@ilyavolodin
Copy link
Member

@nzakas While I understand what you are saying about tagging, this issue has been discussed on the last TSC meeting and included discussion about tags. The decision to add tags was supported by all present TSC members (6 out of 7). So I don't think we can just close the discussion on this with a single vote down comment.

@nzakas
Copy link
Member

nzakas commented Feb 14, 2017

I'm not trying to shut things down, just stating my opinion. The tags proposal just seems like a huge amount of work (both to get started and ongoing -- I can foresee long discussions arguing which tags are appropriate for each rule) for very little gain and a massive scope increase from the request on the issue, and that opinion hadn't been expressed by anyone else, so I felt it needed to be said.

I would have preferred to see a separate issue about tags so that the original request could have been addressed without tying that concern to what is a tangential feature request.

Again, not trying to stop anything, but I don't see that there was any other critique of the tags proposal. It seemed like we could solve the original request without a lot of work if we wanted to, and someone needed to point that out.

@platinumazure
Copy link
Member

So as I noted, I'm happy to just go with moving ES6 rules to better categories if we can get consensus in that direction. However, as @ljharb and others have noted, this can sometimes mean confusion if people try to figure out an ES6 rule when their code isn't ES6, or it can make it harder to find ES6 rules when moving a codebase from ES5 to ES6. So my tags proposal was designed to address the basic idea that sometimes, a rule can be categorized multiple different ways, and was trying to think to the future. But if the consensus is that this is not necessary at this time, I will abide by that.

@nzakas Do you have a concrete proposal in mind for how we can handle users who want to see (for example, without loss of generality) the ES6-specific Possible Errors rules, who might be searching with either ES6 or Possible Errors in mind first? I'm afraid to just move the ES6 rules without having something in mind for people who want to search specifically for ES6.

@nzakas
Copy link
Member

nzakas commented Oct 4, 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 4, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Apr 3, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint
Projects
None yet
Development

No branches or pull requests