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

ROADMAP #2259

Closed
davidtheclark opened this issue Jan 14, 2017 · 28 comments
Closed

ROADMAP #2259

davidtheclark opened this issue Jan 14, 2017 · 28 comments
Labels
status: needs discussion triage needs further discussion

Comments

@davidtheclark
Copy link
Contributor

davidtheclark commented Jan 14, 2017

I'd like to chat about the next steps for stylelint. Our 8.0 release will be mostly focused on standardizing option names; and we already had a major release that was mostly about standardizing rule names; so I expect that more standardization like this will not be necessary for quite a while, if ever. We've create a very fine configuration system. So let's think about what might be our next major goals, if any.

First off, I'll say that I personally have lost all sympathy for and interest in non-standard syntax. I am now a hard-line anti-Sass militant: as far as I'm concerned it should be banished from the realm. I am not planning to spend any of my time fixing issues related to non-standard syntax. I've been toying with the idea of forking stylelint to create a much simplified CSS-only version — that's extreme, but the next paragraph shows why.

There are two directions that interest me the most right now:

  • Automatic fixes
  • Spec validation

I agree with the growing opinion in the frontend world (evidence by the immediate popularity of prettier for JS) that style format rules are best enforced by an automatic fixer rather than a linter. I might say that I'm interested, then, in contributing to stylefmt — but I'm discouraged by the idea of getting involved in another project that is going to be poisoned by non-standard syntaxes. Just as I have an inclination to fork stylelint to create a CSS-only linter, I have an inclination to fork stylefmt to create a CSS-only formatter.

Spec validation would involve contributing to css-values or csstree and their stylelint plugins. These important tools are another reason to hate Sass its kin: these projects will be hindered, stalled, even derailed if they try to accommodate non-standard syntax completely.

My interests in the above come from belief that there are 3 things people typically want from a code quality tool:

  1. Clean-up & standardize formatting: stylelint does this, but this is better done with an auto-formatter (using stylelint's config).
  2. Enforce best practices: stylelint does this as well as anything.
  3. Validate that the code is not broken: stylelint does a little of this, but this is better done via major projects like css-values and csstree.

These priorities make me very, very inclined to steer away from non-standard syntaxes. I could just step back, as mentioned above, and hope that others who are not so hostile will take charge of handling anything non-standard-syntax-related issues. But actually I'm revisiting @jeddy3's idea in #1749 , that we offload all handling of non-standard syntax, push it out of stylelint. This could be v9.

I'm not thinking we should try to create another AST format that is stylelint-compatible — that seems like more than necessary — but that we should extract our accumulated code and knowledge into tools that prepare code before it enters stylelint, and offer those up to the non-standard syntax communities to maintain. Maybe stylelint's formatters would work for this purpose: the scss-formatter takes SCSS code and transforms it into code that stylelint can read, just as the markdown-formatter takes markdown code and transforms it into code that stylelint can read. The more I think about this, the more I like the idea.

I'm not sure what to do about stylefmt. @morishitter what are your thoughts about non-standard syntax and the future of stylefmt? Are you worried that supporting non-standard syntax will hurt the project, and do you have any interest in dropping support? Or if I want to work on a CSS-only formatter, should I fork stylefmt?

Thoughts @stylelint/core?

@davidtheclark
Copy link
Contributor Author

@jeddy3 I realize that most of ^^ means that I'm coming around to your views that I've argued against in the past! 😜

@ntwb
Copy link
Member

ntwb commented Jan 15, 2017

Fantastic write up, well thought out and put forward arguments, no objections being CSS only 👍

@jeddy3
Copy link
Member

jeddy3 commented Jan 15, 2017

@jeddy3 I realize that most of ^^ means that I'm coming around to your views that I've argued against in the past! 😜

😆

You know I'm going to be in agreement with this proposal :)

You articulate the arguments for it very well here and within your blog post.

Our 8.0 release will be mostly focused on standardizing option names;

Yes, and deprecating rules based on this audit.

so I expect that more standardization like this will not be necessary for quite a while, if ever.

Agreed.

My interests in the above [Automatic fixes & Spec validation] come from belief that there are 3 things people typically want from a code quality tool:

I agree. Additionally, I think people typically want to specify a subset of CSS features that can be used by their team. The CSS spec is vast, with numerous ways of solving any given problem (e.g. font sizing). Restricting what features are available to a team can greatly improve the consistency of the code base.

For clarity's sake, I'll briefly expand on how, and to what extent, we currently fulfil each of these wants...

  1. Clean-up & standardize formatting: stylelint does this, but this is better done with an auto-formatter (using stylelint's config).

There are, as outlined in #2079 (comment), 86 formatting rules in stylelint. These laser-focused rules work together to enforce a wide variety of formatting styles. I agree that these formatting styles should be auto-fixed.

  1. Enforce best practices: stylelint does this as well as anything.

We achieve this in stylelint by not enforcing any particular best practices per se, but rather by providing a series of 53 rules to disallow (no-*, *-whitelist, *-blacklist), limit (max-*) or enforce a custom pattern (*-pattern) on nearly all of the CSS language constructs and features. This allows stylelint to move with the shifting landscape of best practices. These rules also allow the fulfilment of the fourth thing I believe people typically want in a code quality tool; subsetting what features and constructs are available to a team.

  1. Validate that the code is not broken: stylelint does a little of this, but this is better done via major projects like css-values and csstree.

We split this into two camps:

  1. Code that is invalid (our spec validation rules).
  2. Code that is valid but likely has unintended consequences (no-duplicates, no-overrides etc)

I agree that the former is better done via major projects like css-values and csstree.

These priorities make me very, very inclined to steer away from non-standard syntaxes.

Agreed.

but that we should extract our accumulated code and knowledge into tools that prepare code before it enters stylelint, and offer those up to the non-standard syntax communities to maintain.

SGTM. A lot of this is captured within the isStandardSyntax* utils (with just two bits to do with parser limitations of standard syntax).

Maybe stylelint's formatters would work for this purpose: the scss-formatter takes SCSS code and transforms it into code that stylelint can read, just as the markdown-formatter takes markdown code and transforms it into code that stylelint can read.

I assume you mean processors here? But yes, this avenue is definitely worth exploring. An alternative avenue could be the --custom-syntax option.

I'm revisiting @jeddy3's idea in #1749 , that we offload all handling of non-standard syntax, push it out of stylelint. This could be v9.

I agree. We get 8.0.0 out with its options standardisations and rule deprecations. We then deprecate support for non-standard syntaxes soon after and offload all handling of non-standard syntax to outside the org for 9.0.0. I think this could be done with --custom-syntax. I was secretly hoping that adding support for it in #2017 would be the backdoor to getting #1749 approved :)

For 9.0.0 stylelint core rules and stylelint-config-standard should only support standard CSS syntax and language features. All the systems of extensibility (plugins, formatters, processors/custom-syntaxes etc) would still be in place for non-standard syntaxes though, should users of those syntaxes want to make use of stylelint's engine.

We can then focus on exploring the very interesting possibilities of automatic fixes and spec validation for 9.0.0!

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Jan 15, 2017
@hudochenkov
Copy link
Member

What about isStandardSyntaxDeclaration and such helpers in CSS-only stylelint?

For example there is a stylesheet which is processed by postcss-simple-vars:

a {
    $width: 10px;
    width: $width;
}

This stylesheet processed by default PostCSS parser. $width is declaration, but not a standard one. What declaration-empty-line-before will do with these declaration? Will it apply? Right now it won't apply because isStandardSyntaxDeclaration returns false for this declaration.

@jeddy3
Copy link
Member

jeddy3 commented Jan 16, 2017

I think working out the particulars still needs to be done.

What declaration-empty-line-before will do with these declaration? Will it apply?

It'll probably treat them as declarations and be applied.

a {
    $width: 10px; /* parsed as delc, treated as declaration */
    width: $width; /* parsed as delc, treated as declaration */
    --custom-prop: value; /* parsed as decl, treated as custom property */
}

Unless, as @davidtheclark suggests, something is used to prepare the code before it enters stylelint and these constructs are removed.

What are your thoughts?

@hudochenkov
Copy link
Member

If it will be applied, there should be new ways to achieve the same as now behaviour.

There should be plugin with identical logic to new declaration-empty-line-before, but which will check if it's a CSS property or not. And this plugin should be always in sync with core declaration-empty-line-before to have same options and to avoid problems, which can be found in core rule and needed to be copied to plugin rule. It'll be a big headache for plugin maintainer and for users, because it's not clear what to use: core declaration-empty-line-before or stylelint-scss/declaration-empty-line-before (hypothetical name :)).

And it's only one rule in my mind. I think there will be more rules, which can potentially hurt users, who use default PostCSS parser and some PostCSS plugins with non-standard syntax.

@hudochenkov
Copy link
Member

Autoformatting tool is great! There few “rules” what will make an autoformatting tool an excellent companion to stylelint:

  1. It should use stylelint config.
  2. It should support all possible rules. It's very hard to support all rules from the start. But eventually, it should support all of them.
  3. If rule claimed as supported, it should have all options, from stylelint. Always.
  4. Rule's logic should be the same. With all options, workarounds and edge cases.
  5. A formatting tool should be in sync with current stylelint. Otherwise, it may hurt.

It would be hard to have formatting tool as a separate project, because of “rules” above. I remember one of the main reasons for keeping stylefmt and stylelint as separate projects were stylelint's core team disinterest in supporting and developing such tool. Now we have at least one core team member with interest in this subject :)

I have an example regarding “in sync” rules logic and options. I've released postcss-sorting 2.0 recently and have to add support for *-empty-line-before rules. I did it because I need them, but I can't rely on stylefmt yet :( To avoid changes in a few weeks I've implemented options that will be in the next stylelint release. For example, { "at-rule-empty-line-before": [ "always", { except: ["after-same-name"] } ] }. It's not released yet in stylelint but will be soon.

Now I have a linter — stylelint without option above, and formatter — postcss-sorting with the option above. I can't lint "at-rule-empty-line-before", but I can already format with this option.

It's reversed situation (formatter ahead of linter), but it makes a point. A logic for a linter and formatter should be the same. Otherwise there will be problems: linter has a new option, formatter didn't get this option yet, formatter make changes to style sheet without this option and linter fails.

@hudochenkov
Copy link
Member

One more thing for autoformatter: it should support stylelint plugins somehow. For example, there should be a way to fix stylelint-order warnings. postcss-sorting can be used in formatter to fix them.

@davidtheclark
Copy link
Contributor Author

subsetting what features and constructs are available to a team

@jeddy3 Exactly — I was thinking of that as part of "best practices" but I do agree that it's more clear to name those goals separately, since people often think of "best practices" as global rather than particular.


After @hudochenkov's comments and some thinking this morning, I have some new ideas.

Considering what it would actually take to extract SCSS/Less/SSS exceptions into external tools, and to do that in a way that preserves current functionality — if it's possible at all — I'm worried it would be much, much more work to conduct such an experiment than to stick to the path we're on. The more practical approach might be this:

  • Work to bring on more non-standard syntax devotees interested in fixing bugs and refining the tool for their pipeline. (I sent out a plea on Twitter and got several responses.)
  • Those of us who are uninterested in non-standard syntax can just steer clear of the issues entirely, spending our time on other things.

(Spec validation with css-values or csstree was always going to be a plugin, so it's no problem if that particular plugin just doesn't work for non-standard syntax users.)

I worry that @jeddy3's ultra-systematic mind might not be satisfied by such an approach. But don't you agree that the alternative would be a ridiculous amount of work, whereas what we have going now is sustainable, especially if you personally ignore non-standard syntax issues? Anybody who wants to spend time working on css-values can do so; and anybody who wants to spend time working on SCSS bugs can do so.

I think that the next focus should be on automatic fixes. All @hudochenkov's goals make sense to me.


Another goal to consider is improving performance. There's one big change we could make that I think would make a big difference: Change the rule API so that rules provide callbacks for particular nodes instead of looping through the nodes themselves. Then the function that invokes rules would walk through each type of nodes only once. This would dramatically decreases the number of times we walk through all the nodes of a given type, so ought to improve performance dramatically.

Essentially, we'd be doing what PostCSS folks are calling an "event-based API". PostCSS is going to have this at some point, but it seems like it's been in the works for a very long time and might not be delivered for another very long time. We could make our own, and hook it into PostCSS's whenever it's available.

This would be quite a bit of work. But there is some consolation:

  • It could be done piecemeal — we don't have to convert all the rules at once.
  • The tests should not change at all.

@alexander-akait
Copy link
Member

@davidtheclark if I understand correctly, there is a proposal to use selectors (as do eslint)
example:

module.exports = {
  return {
    Decls: someValidateFunction()
  }
}

Am i right?

@davidtheclark
Copy link
Contributor Author

@evilebottnawi Yes, same idea.

@alexander-akait
Copy link
Member

@davidtheclark I'm voiting for it, also it will greatly reduce the code and make it more readable

@kristerkari
Copy link
Contributor

kristerkari commented Jan 17, 2017

I agree with the growing opinion in the frontend world (evidence by the immediate popularity of prettier for JS) that style format rules are best enforced by an automatic fixer rather than a linter.

For me this is a feature I'd really like to see stylelint support properly. I use ESLint's autofixer very often. It is very powerful when you can take any code outside your project and just automatically format it to what ever you have defined in your project.

I'm discouraged by the idea of getting involved in another project that is going to be poisoned by non-standard syntaxes. Just as I have an inclination to fork stylelint to create a CSS-only linter, I have an inclination to fork stylefmt to create a CSS-only formatter.

I really like this idea, because trying to support preprocessors or other syntaxes when autofixing is just a big mess (I've been contributing to stylefmt, and just look at the bugs it has currently).

Considering what it would actually take to extract SCSS/Less/SSS exceptions into external tools, and to do that in a way that preserves current functionality — if it's possible at all — I'm worried it would be much, much more work to conduct such an experiment than to stick to the path we're on.

My current project has almost 20 000 lines of SCSS. Using stylelint in the project is super useful, but switching to use something else than Sass would be a huge task, and something that probably won't happen. Even if Sass has its shortcomings, good parts and bad parts, it is still a good tool when you need to do something like generating CSS using math and loops etc.

I don't see extracting SCSS/Less/SSS exceptions into external tools as a bad idea. Surely it requires a lot of work, but it would also mean that stylelint/stylelint repo can stay "clean", and does not have to worry about fixing bugs for non-standard syntaxes.

For example, if my project would be using using Sass, I could do npm install stylelint stylelint-compatibility-scss. That compatibility plugin would try to offer all the same rules as stylelint, but extend/modify them to work with Sass. When using SCSS compatibility, you would then likely have less rules that work with --fix option. You could start by offering, let's say 50-75% of the core rules with SCSS compatibility (with or without autofixing), and then add more as time goes on.

I'm not sure how this would work if your project is using multiple syntaxes (what would stylelint to if you have both stylelint-compatibility-scss and stylelint-compatibility-less in use?).

@alexander-akait
Copy link
Member

@kristerkari I can not imagine how many things we use should moved to the separate package as in many rules we use different checking based on non-standard syntax. The only solution in my eyes is a postprocessor on postcss (call it stylelint-compatibility) and each stylelint-compatibility-scss (and etc) should produce from a non-standard syntax to abstracted ast tree (stylelint compatibility ast). But it's just a lot of work, can we find time for this? 🤔

@nirazul
Copy link
Contributor

nirazul commented Jan 17, 2017

  • Work to bring on more non-standard syntax devotees interested in fixing bugs and refining the tool for their pipeline. (I sent out a plea on Twitter and got several responses.)
  • Those of us who are uninterested in non-standard syntax can just steer clear of the issues entirely, spending our time on other things.

I'd be really happy with such a solution, as well as ready to help if something pops up. Even though we are using stylelint exclusively with scss in about 20 - 30 projects, I've never had the urge to add plugins like stylelint-scss to improve linting. Stylelint already does a great job at linting literally everything that is thrown at it.

Just out of curiosity, how do you guys work? Do you use exclusively CSS with PostCSS?
I'm asking because preprocessor adoption is still high and a high percentage of deveopers are using custom syntaxes (not judging here if that is good or bad).
What I'm most interested in is how your vision looks like, how a scss (or less or stylus.. etc) developer uses stylelint beyond v8.

@alexander-akait
Copy link
Member

BTW, i have about 100+ projects and in all i use stylelint and sass, maybe we should postpone the idea of dropping non standard syntax support

@kristerkari
Copy link
Contributor

kristerkari commented Jan 17, 2017

But it's just a lot of work, can we find time for this?

I know that it is a huge effort, but I also doubt that the autofix feature would work that well if stylelint supports multiple, non-standard syntaxes. :/

@sergesemashko
Copy link
Contributor

sergesemashko commented Jan 17, 2017

Another thing that may give significant performance gain is caching the results (like in eslint). So, only changed files are passed to linter on subsequent runs.
On my current setup with >100 .scss files linting time is about ~5-7 seconds, which is not good enough for development mode.
I'd be happy to take a look at this, if such feature is welcomed.

@davidtheclark, what do you think?

@alexander-akait
Copy link
Member

alexander-akait commented Jan 17, 2017

@sergesemashko eslint have caching (but it is not related to stylelint webpack plugin), best way use preloader for this, in webpack all loaders have cache

@davidtheclark
Copy link
Contributor Author

@sergesemashko We'd love any issue/PR suggesting performance improvements! For your idea, how about opening an issue explaining the idea a little more thoroughly, and then we'll discuss and move on to a PR?


I also doubt that the autofix feature would work that well if stylelint supports multiple, non-standard syntaxes

@kristerkari I think that this might be one of the main advantages of moving the --fix code to within stylelint, instead of out in stylefmt. Because stylelint already has the logic to ignore non-standard syntax, it should be easier for --fix to understand the same exceptions and edge-cases that stylelint does.


i have about 100+ projects and in all i use stylelint and sass, maybe we should postpone the idea of dropping non standard syntax support

@evilebottnawi 😆 It's true: almost everybody uses SCSS. If stylelint is to remain relevant to the typical frontend dev/designer, we can't abandon SCSS.

Just out of curiosity, how do you guys work

@nirazul I've been using PostCSS custom properties and custom media queries. I sometimes write Node modules to generate CSS that others might be tempted to implement with Sass loops and mixins and other nonsense. Maybe at some point I'll write a blog post ...

@hudochenkov
Copy link
Member

Because stylelint already has the logic to ignore non-standard syntax, it should be easier for --fix to understand the same exceptions and edge-cases that stylelint does.

True. I've borrowed *-empty-line-before logic for postcss-sorting 2.0 and it works great. Maybe with some rules there will be exceptions and fix logic will require some tweaks, but I believe most of the rules won't require any fixes.

@jeddy3
Copy link
Member

jeddy3 commented Feb 11, 2017

I worry that @jeddy3's ultra-systematic mind might not be satisfied by such an approach.

I try to be pragmatic sometimes too :)

I agree that your approach is the more practical one.

I think we're also in agreement that the complexity cost of non-standard constructs/syntax is too high, and that only using standard constructs is where the future of CSS authoring and tooling lies.

I think there's an opportunity to clarify stylelint's position on non-standard syntax/constructs. I think this clarification would be especially useful when we come to communicate that new feature (e.g. autofixing) will only be for standard syntax/constructs. In 8.0.0, I suggest we deprecate the syntax option (and it being automatically inferred by the file extensions), in favour of promoting the customSyntax option for all non-standard syntaxes. In 9.0.0 we'd remove the syntax option and move postcss-less, postcss-scss and sugarss to devDependencies, where they'd only be used for testing.

I think this simple change would communicate our vision for adding new features that only support standard constructs/syntax. I think it'll also become easier to communicate that stylelint's core rules are only applicable to standard constructs/syntax by (and this is the important bit) ignoring, rather than tripping-up on non-standard ones. People can continue to contribute fixes (and tests) to stylelint so that non-standard are correctly ignored. I agree that pushing this outside of stylelint's core isn't practical at the moment.

From an end user's perspective, the only thing that will change is that they will need to explicitly state that they are using a non-standard syntax (by installing another parser and using the customSyntax option).

As an aside, I think our decision last year to use whitelisting for standard constructs, rather blacklisting non-standard constructs, for the utils has proven to be a sound choice. And I agree with your earlier assessment that such an approach has proven sustainable.

What do you think?

I believe this is just a small twist on, what I feel, is the consensus reached in this discussion. As such, I'm going to write-up a VISION.md (as suggested in this blog post), which will hopefully capture all the good ideas e.g. autofixing being a key goal (but only for standard syntax) etc.

Do you use exclusively CSS with PostCSS?

Yes, I'm much like @davidtheclark.

Node modules to generate CSS that others might be tempted to implement with Sass loops and mixins and other nonsense. Maybe at some point I'll write a blog post ...

That would be awesome.

@davidtheclark
Copy link
Contributor Author

@jeddy3: On changing the syntax option to a customSyntax option.

Upside:

  • Users can update their custom syntax on their own, without stylelint doing that. (However, we have had absolutely no trouble with this, as the custom syntaxes have not been updating often at all, while we update frequently.)

(I don't really think the psychological effect you suggest, that this will "clarify stylelint's position" or "communicate our vision", is worth any kind of change. Documentation will be much more effective in conveying such intentions than implicit communication via subtle option changes.)

Downsides:

  • It is an insignificant but breaking change, and once again users will be forced to make minor alterations to their configurations when they upgrade. More almost every user, I think this will be more of an annoyance than a meaningful communication.
  • It puts our codebase in a weird (bad) situation: we'll accept issues and write tests to cover the usage of modules that are not actually dependencies. This becomes more of a problem as people's custom syntax modules, which they are individually updating, get out of sync.

I just don't see it as a change worth making.

@jeddy3
Copy link
Member

jeddy3 commented Feb 12, 2017

Downsides:

Good points.

Documentation will be much more effective in conveying such intentions than implicit communication via subtle option changes.)

Ok, agreed.

@davidtheclark
Copy link
Contributor Author

I think we're all on the same page, and sounds like @jeddy3 is interested in working out a VISION.md document to articulate our aims (great idea!). So I'll close this discussion and if we want to elaborate on certain details we can open new ones.

@matype
Copy link

matype commented Mar 2, 2017

@stylelint/core @davidtheclark I'm sorry to be late. I think, it's hard for stylefmt team to follow stylelint development speed. So, I have a proposal: May I transfer stylefmt repository to stylelint organization? It seems happy for users and our community. Thanks.

@alexander-akait
Copy link
Member

alexander-akait commented Mar 2, 2017

@morishitter #2096 i think we should to reach a compromise in the future to join forces to build stylelint better. Now move stylefmt to stylelint org create many new issue and it is a lot of work for us, we don't have time to contribute separate package. I think we should asap resolve #2096 and merge our team.

@matype
Copy link

matype commented Mar 3, 2017

@evilebottnawi I see, Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

9 participants