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 function-calc-no-invalid #4731

Closed
Tracked by #5205
jeddy3 opened this issue May 1, 2020 · 4 comments
Closed
Tracked by #5205

Remove function-calc-no-invalid #4731

jeddy3 opened this issue May 1, 2020 · 4 comments
Labels
status: wip is being worked on by someone
Milestone

Comments

@jeddy3
Copy link
Member

jeddy3 commented May 1, 2020

What is the problem you're trying to solve?

The generated parser that sits behind the function-calc-no-invalid rule weighs 130k.
This is smaller than the very heavy *-no-vendor-prefix rules (see (#4730), but 130k is still a significant weight to a browser bundle and is problematic for #3935.

Additionally, @evilebottnawi mentioned in #2454 (comment) that the rule is problematic for other reasons.

What solution would you like to see?

Move the rule into a plugin.

We should consider using this opportunity to move the other validator rules into a plugin, e.g. at-rule-no-unknown. Including the validator rules in core is:

Users can choose whether and how they want to validate their CSS:

  • a dedicated and comprehensive validator like csstree if they're writing vanilla CSS
  • the @stylelint/plugin-validate plugin if they want to use stylelint to validate (as best it can)

Shared configs like stylelint-scss could include the plugin and configure it appropriately.

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label May 1, 2020
@alexander-akait
Copy link
Member

alexander-akait commented May 1, 2020

We should consider using this opportunity to move the other validator rules into a plugin, e.g. at-rule-no-unknown.

We will become not a linter, but the second prettier with options and the ability to install plugins

#1612 (comment)

Other our rules also can be broken, invalid AST === broken rule

@alexander-akait
Copy link
Member

alexander-akait commented May 1, 2020

Thus we lose our main goal. For me personally it is very sad that we sacrifice the main goal because of performance, what for us at the moment is not critical at all.

We constantly recommend using csstree for validation. Does anyone really use this? It contains many bugs and is still under development. If necessary, I can provide examples.

I agree function-calc-no-invalid has bad implementation and probably we should not have put it in the core in the past. But the rest of the rule is quite simple, logical, comfortable and work fine, excluding problems with parsers.

After we transfer them, who will maintenance these repositories, I think no one.

Let's look on real world:

  • Stylelint is linter, more linter rules is good, removing such rules is the loss of our main goal
  • Stylistics rules can be simplified because we have prettier and many developers have been using it for a long time
  • We have good performance right now, improving it is not critical
  • Bundle size is not critical, we can bundle some packages separately and load them lazy
  • Almost all developers will install this plugin
  • CSStree is not good right now (is still under development) and we should not recommended it as stable solution
  • We don't have enough contributors to support plugin packs, and these rules is very popular, look on real configurations on github.

These are my thoughts

@jeddy3
Copy link
Member Author

jeddy3 commented May 1, 2020

All good points.

I agree function-calc-no-invalid has bad implementation and probably we should not have put it in the core in the past. But the rest of the rule is quite simple, logical, comfortable and work fine, excluding problems with parsers

We could move only function-calc-no-invalid to a plugin and keep the other rules in core. That will help us support running stylelint in the browser (#3935).

@alexander-akait
Copy link
Member

Agree, let's move function-calc-no-invalid out of core, just note - latest CSS have min/max/clamp functions like calc, so name is wrong too 👍

@jeddy3 jeddy3 mentioned this issue Apr 12, 2021
30 tasks
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone and removed status: needs discussion triage needs further discussion labels May 12, 2021
@jeddy3 jeddy3 added this to the future-major milestone May 12, 2021
@jeddy3 jeddy3 changed the title Move function-calc-no-invalid (and perhaps other validate rules) to a plugin pack Remove function-calc-no-invalid May 12, 2021
@jeddy3 jeddy3 added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels May 12, 2021
@jeddy3 jeddy3 closed this as completed May 14, 2021
@jeddy3 jeddy3 mentioned this issue May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone
Development

No branches or pull requests

2 participants