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

Move *-no-vendor-prefix rules to a plugin pack #4730

Closed
jeddy3 opened this issue May 1, 2020 · 9 comments
Closed

Move *-no-vendor-prefix rules to a plugin pack #4730

jeddy3 opened this issue May 1, 2020 · 9 comments
Labels
status: needs discussion triage needs further discussion

Comments

@jeddy3
Copy link
Member

jeddy3 commented May 1, 2020

What is the problem you're trying to solve?

The vendor prefix rules, e.g. at-rule-no-vendor-prefix, wrap autoprefixer, which depends on the caniuse-lite database. These accounts for 15% or 1mb of the bundle size. This is this problematic for #3935.

As mentioned in #2454 (comment), we should have removed them in 8.0.0. We can remedy that in 14.0.0.

What solution would you like to see?

We would move them to a plugin pack before releasing 14.0.0 so that users can continue to, if they choose, use these rules by installing the plugin. We should consider releasing them in the@stylelint scope, e.g. @stylelint/plugin-no-vendor-prefixes. The pack will contain all 5 rules, further reducing the obstacles to migrating.

@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

Honestly, removing such rules is not very good, removing good linter rules is slightly contrary to the meaning of linter.

These accounts for 15% or 1mb of the bundle size.

We don’t have a problem with size, because we are dev tool, bundle of prettier is also big

@alexander-akait
Copy link
Member

I don’t think our performance is critically bad and I would rather see more of these rules. Given the existence of prettier and many of our rules for stylistics removing these rules is not good from core.

You can look at popular configurations and almost all contain these rules, which means we will also worsen DX, because developers still continue to install plugin and use it.

@jeddy3
Copy link
Member Author

jeddy3 commented May 1, 2020

bundle of prettier is also big

The Prettier browser bundle size is 1mb. By moving these rules to a plugin pack (and bundling the syntaxes separately) we can achieve something similar, which makes #3935 viable.

removing good linter rules is slightly contrary to the meaning of linter.

They will still be available as a plugin, which are easy to use.

Given the existence of prettier and many of our rules for stylistics removing these rules is not good from core.

There are a lot of non-stylistic rules in stylelint. Those that check for possible errors, like overriding properties or duplicate selectors, and those that limit language features, like whitelisting units or defining patterns for class names. The latter rules are especially useful when writing in a language like CSS. I don't think we need to be concerned about the usefulness of stylelint's built-in rules.

@alexander-akait
Copy link
Member

alexander-akait commented May 1, 2020

I don't think we need to be concerned about the usefulness of stylelint's built-in rules.

I disagree here. Usefulness is very important.

What is problem bundle autoprefixer separately and load it lazy as we want to do with syntaxes?

Also, my comment from here applies to this issue too.

As a developer, I don’t want to get basic rules from plugins that most likely no one will support, let's be honest with each other, we have very few contributors

@jeddy3
Copy link
Member Author

jeddy3 commented May 1, 2020

Usefulness is very important.

I agree with you wholeheartedly. I meant that moving these vendor prefix rules to a plugin won't impact overall stylelint's usefulness because we have so many other great (non-stylistic) rules. And even these vendor prefix rules will still be available as a plugin.

What is problem bundle autoprefixer separately and load it lazy as we want to do with syntaxes?

We will have to bundle it in the browser version (#3935). This is the crux of the problem.

If we bundle autoprefixer separately then have to explain to the user that they need to include autoprefixer for the vendor prefix works to work. This is counter-intuitive as we already have a plugin system:

<script src="https://unpkg.com/stylelint@14.0.0/universal.js"></script>
<script src="https://unpkg.com/stylelint@14.0.0/syntax-scss.js"></script>
<!-- You need to include autoprexifer for some heavy rules to work -->
<script src="https://unpkg.com/stylelint@14.0.0/autoprefixer.js"></script>
<script>
  stylelint.lint("a#{var} { color: red; }", {
    config: { 
      "rules": { 
        "at-rule-no-vendor-prefix: true;"
      },
    },
    syntax: "scss",
  });
</script>

Vs:

<script src="https://unpkg.com/stylelint@14.0.0/universal.js"></script>
<script src="https://unpkg.com/stylelint@14.0.0/syntax-scss.js"></script>
<script src="https://unpkg.com/@stylelint/no-vendor-prefix@1.0.0/index.js"></script>
<script>
  stylelint.lint("a#{var} { color: red; }", {
    config: { 
      "plugins": ["@stylelint/plugin-no-vendor-prefix"],
       "rules": { 
          "plugin/property-no-vendor-prefix": true
       }
    },
    syntax: "scss",
  });
</script>

It's only these 5 vendor prefix rules that are problematic to the bundling for the browser as they are significantly bigger than all the other rules combined.

Is there a way, other than moving these 5 rules to a plugin, that will make the browser bundle small enough for tools like JS Fiddle?

@hudochenkov
Copy link
Member

I agree with @evilebottnawi. Removing rules for thousands of users for the sake of handful of users (JSFiddle, our demo, etc.) makes bad user experience. Having stylelint in a browser is not a very popular use case, we shouldn't sacrifice majority of the users experience. Having to install plugins is always a hustle. More burden on the user to update. More burden on developers (us) to update them an maintain with all infrastructure. Release new stylelint core version? Go to every plugin and bump peerDependencies there.

If really want to make it work in browser, I see at least on possible solution. In addition to decoupling engine from file system part (#3935 (comment)) we could have decoupled list of rule. We'll have two lists: all rules, and all but heavy rules. So for browser bundle in addition to stylelint bundle, consumer should also either include all rules or all but heavy rules.

@vankop
Copy link
Member

vankop commented May 1, 2020

We can start with stubing problematic rules, for example resolving all this rules to stub-rule and throw error in it

@jeddy3
Copy link
Member Author

jeddy3 commented May 1, 2020

Good points everyone.

We'll have two lists: all rules, and all but heavy rules. So for browser bundle in addition to stylelint bundle, consumer should also either include all rules or all but heavy rules.

This feels like a suitable solution. It scales should we want to add more useful, yet heavy, rules in the future.

@jeddy3
Copy link
Member Author

jeddy3 commented May 10, 2020

Closing as the consensus is to not move the rules to a plugin. Instead, we'll deal with the weight as part of the bundling process which is being tracked in #3935.

@jeddy3 jeddy3 closed this as completed May 10, 2020
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

4 participants