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

Remove syntax option #5289

Closed
Tracked by #5205
jeddy3 opened this issue May 12, 2021 · 6 comments · Fixed by #5297
Closed
Tracked by #5205

Remove syntax option #5289

jeddy3 opened this issue May 12, 2021 · 6 comments · Fixed by #5297
Labels
status: ready to implement is ready to be worked on by someone
Milestone

Comments

@jeddy3
Copy link
Member

jeddy3 commented May 12, 2021

Ref: #4942 (comment)

We'll need to:

  1. Replace syntax with customSyntax in the Jest preset (and release a new version)
  2. Move PostCSS 8 compatible syntaxes to dev dependencies and remove the non-compatible ones
  3. Update rule tests to require the needed syntaxes, and update syntax to customSyntax
  4. Remove the syntax and postcss-syntax logic from the engine
@jeddy3 jeddy3 added this to the future-major milestone May 12, 2021
@jeddy3 jeddy3 mentioned this issue May 12, 2021
30 tasks
@jeddy3 jeddy3 added the status: ready to implement is ready to be worked on by someone label May 12, 2021
This was referenced May 12, 2021
@hudochenkov
Copy link
Member

Did we really think it through?

I think we're solving our inconvenience by creating inconvenience to the users.

stylelint is downloaded 2,669,762 times a week.
stylelint-scss is downloaded 1,064,045 times a week.

At least 40% of stylelint usage is with SCSS! I'm saying “at least”, because not every Sass usage with stylelint is with stylelint-scss. The number probably even higher.

I think we should remove only syntaxes, which lack maintainance — html, markdown, css-in-js.

@ybiquitous
Copy link
Member

I also have a concern that a significant number of users will be inconvenienced... 🤔

@jeddy3
Copy link
Member Author

jeddy3 commented May 13, 2021

I'd like to explain this change in a broader and longer-term context, particularly around two problems we face:

  • confused users
  • unmaintained dependencies

Confused users

Up until a couple of years ago, we had a simple premise:

Stylelint is a CSS linter that can be extended to support other styling languages.

It was an early design decision to gear the built-in parser, built-in rules and official configs towards CSS while providing extension points. It was a good decision as it was easier to:

  • communicate to users how the Stylelint pieces fitted together
  • sustain the project by limiting scope and complexity

We've muddied the waters by bundling syntaxes into Stylelint. We can now parse various styling languages out-of-the-box, but the built-in rules and the official configs are for CSS.

The most evident symptom of this problem are issues like #4933, where users are left confused. It's worse for users of whitespace syntaxes like SugarSS and SASS, as many of the rules turned on in the official configs are incompatible with them.

I also think some users are missing out on comprehensive linting. We produce gaps when we use our isStandardSyntax* utils and ignore* secondary options to make the built-in rules somewhat compatible with other styling languages. Plugins like postcss-scss fill these gaps. I suspect some users aren't aware these gaps exist and don't reach for postcss-scss because they assume stylelint, as it can parse their styling language, is capable of everything out-of-the-box.

Unmaintained dependencies

Styling languages and libraries come and go. Particularly within the CSS-in-JS community, where the churn is eye-watering.

The most evident symptom of this problem is the situation we're in right now, where unmaintained dependencies have led to vulnerabilities. How long until postcss-sass (or the underlying parser) is unmaintained, and we have to remove it as a breaking change?

Why removing postcss-syntax and the syntax option solves both these problems and more

Although the two issues seem unrelated, we can solve both by removing syntax in favour of customSyntax.

Confused users

Removing syntax will make it easier for us to communicate to users:

  • what stylelint can do out-of-the-box
  • when it needs to be extended

i.e. it can be used to lint CSS files out-of-the-box and must be extended otherwise.

New users will benefit from more straightforward instructions. For example:

"Linting CSS? Grab an official config, and away you go. Linting something else? You'll need to extend stylelint with a community config to do that."

Healthy communities like the SCSS one can maintain their own standard and recommended configs, e.g. stylelint-config-recommended-scss. We can minimise the inconvenience of updating to 14.0.0 by adding customSyntax to the configuration object (#4843). These configs can then include the relevant syntax, like how eslint-plugin-vue includes vue-eslint-parser.

With their bundled syntax, plugins and configuration, these configs will get users of other styling languages up and running quickly with comprehensive linting that doesn't leave gaps.

Unmaintained dependencies

Removing syntax will liberate us from unmaintained dependencies. There will be no impact on Stylelint itself as styling languages and libraries inevitably fall out of favour and are no longer maintained.

Other benefits

As well as addressing the two issues above, there are other benefits:

  • a single option for setting syntax
  • paves the way for leaner custom syntaxes
  • disconnects Stylelint from frameworks
  • leaner package

A single option for setting syntax

We will simplify things for both users and ourselves by having a single option for setting the syntax. It's a bonus that the name custom-syntax aligns with our custom-formatter option, as both signal points of extension.

Paves the way for leaner custom syntaxes

Once someone adds the Document node to PostCSS, people can build lean and simpler custom syntaxes on top of it that only cater to their specific needs. For example, the maintainers of LitElement could build a syntax for their specific use of template literals within classes, and nothing more.

I believe this would be better than a sizeable CSS-in-JS syntax that tries to support every library, especially as CSS-in-JS libraries come and go so often.

Disconnects Stylelint from frameworks

By removing our reliance on postcss-syntax, we move to a sustainable model where users explicitly define the relationship between file extensions and syntaxes. Users can use .vue, .svelte etc. without us needing to maintain a mapping in Stylelint of the different libraries and languages. It will work particularly well when we add an overrides configuration property (#3128).

Leaner package

Users, like Google (#4942 (comment)), will benefit from a leaner package where heavy syntax dependencies are opted-into.

I believe the situation we're in is an opportunity to correct our trajectory for the benefit of our users. And these benefits, especially the longer-term ones, outweigh any inconvenience during an update to 14.0.0.

@hudochenkov
Copy link
Member

hudochenkov commented May 13, 2021

Thank you for an extensive explanation, Richard!

This makes a lot of sense for me.

We would need to make additional work, besided code changes. Due to huge impact on our users we need to have a great migration guide for every syntax we're dropping. Also I would include in our documentation how to make stylelint work with SCSS, and maybe for other syntaxes.

I'm a fan of how Andrey Sitnik is working towards developer experience. E. g. his PostCSS 8.0: Plugin migration guide with step by step instructions. And helpful messages for not used syntax in PostCSS.

We could keep in the code --syntax, but only show instructions what to do. Or if someone is trying to lint *.scss files without customSyntax specified, we also show a message. It doesn't increase maintanence, but makes life of our users easier.

@ybiquitous
Copy link
Member

@jeddy3 Thank you so much for the clarification. I understand the background more and agree with the idea altogether.
Indeed, we need to consider our long-term sustainability more.

Also, I agree with @hudochenkov's idea about a helpful migration guide and kind error messages. 👍🏼

@jeddy3
Copy link
Member Author

jeddy3 commented May 14, 2021

Yes, we'll need a migration guide. I'll create a follow-up issue for that.

We could keep in the code --syntax, but only show instructions what to do. Or if someone is trying to lint *.scss files without customSyntax specified, we also show a message.

Sounds good.

Let's do this as a follow-up pull request so that work can start on #5291 (Move to ESM) and #4942 (Migrate to PostCSS 8), as they can be unblocked by moving to customSyntax. I'll update #5297 to show a message when the --syntax option is used, and then create a new issue to discuss how we can add more messages to improve the DX.

@hudochenkov hudochenkov linked a pull request May 14, 2021 that will close this issue
@jeddy3 jeddy3 closed this as completed May 15, 2021
jeffbyrnes added a commit to abundanthomesma/abundanthousingma.org that referenced this issue Jun 3, 2021
Allows us to properly accommodate SCSS.

Details:
github.com/stylelint/stylelint/issues/5289#issuecomment-840772900
@jeddy3 jeddy3 mentioned this issue Sep 7, 2021
23 tasks
This was referenced Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone
Development

Successfully merging a pull request may close this issue.

3 participants