csstree / stylelint-validator

Stylelint plugin to validate CSS syntax
MIT License
93 stars 10 forks source link

Validating selectors and a plugin pack #13

Open jeddy3 opened 6 years ago

jeddy3 commented 6 years ago

Firstly, thanks again for this plugin. It's great!

Now, how stylelint validates CSS is currently influx:

Invalid code is best handled by emerging dedicated tools e.g. csstree - a language parser with syntax validation. As a stop-gap, while these tools mature, provide invalid code rules for the simplest of cases. (In the future, these tools can be wrapped as plugins to make use of features such as /* stylelint-* */ command comments, severity levels, and options validator.)

At stylelint, we currently direct users to this plugin when they request a rule to validate their declarations e.g. https://github.com/stylelint/stylelint/issues/2816 & https://github.com/stylelint/stylelint/issues/2777.

This plugin validates the property and value pairs of declarations, but I believe csstree can also validate selector lists (and at-rule validation is in the works). What do you think about creating a plugin pack, like stylelint-order and stylelint-scss, to house plugins for declaration and selector list (and at-rule in the future) validation?

In stylelint rules target things (rules, at-rules, selector lists, value lists etc...). We did this to help users find and locate only the rules they need. I think having a validation plugin pack that follows this convention would be beneficial to users. What do you think?

The plugin pack could keep the same name as this plugin (stylelint-csstree-validator) and it could, to be begin with, contain two plugins:

(As an aside, there was a selector-no-empty rule in stylelint but it was removed in 8.0.0. as it was acting as a proxy for validation rather than preference).

I'd be eager to hear your thoughts on whether you think this is a good idea and whether it's aligned how you originally thought csstree and stylelint could work together.

lahmatiy commented 6 years ago

First of all, thank you for support. Feedback is always helpful and welcome.

This plugin validates the property and value pairs of declarations, but I believe csstree can also validate selector lists (and at-rule validation is in the works). What do you think about creating a plugin pack, like stylelint-order and stylelint-scss, to house plugins for declaration and selector list (and at-rule in the future) validation?

I started experiment with validation of things other than declarations. Don't worry, selectors validation is on my way ;) (draft of selector validator)

Few words about the work on that feature. When I started I found that there is no information about various things like valid pseudo classes/elements, at-rules, media features, functions etc. Yes, we have a specs but there are many verdor specific and non-standart things that are supporting by browsers and using on sites. mdn/data is far from to be complete. So I walk through specs, MDN, MSDN, many other sources and finally I analyzed the TOP 250 Alexa sites CSS. As a result we have some dictionaries now. I'm not sure about the correctness and completeness of the dictionaries but looks good for start.

Now I need to integrate those dictionaries into CSSTree and add custom validation to some node types (AtRule, Selector, MediaFeature etc). There are some unsolved problems, e.g. for a validation in some cases we need some kind of context or scopes and so on. So some internal things needed to be implemented into lexer to make feature possible. Unfortunately, I'm lack of time currently and can't say for sure when it will be finished.

In stylelint rules target things (rules, at-rules, selector lists, value lists etc...). We did this to help users find and locate only the rules they need. I think having a validation plugin pack that follows this convention would be beneficial to users. What do you think?

It sounds reasonable, to customize things you want to validate. I don't think making several rules (that we create via createRule) is good solution, because of code duplication and extra tree traversals/parsing/etc. I prefer single rule with options. May be some rule settings from a config may to be merged into a single rule under the hood somehow. If so I would be ok about that.

Anyway, we can discuss the ways how to setup a validation. As a first step there will be a rule that can validate "everything" and have an options.

jeddy3 commented 6 years ago

Thanks for the detailed response!

As a result we have some dictionaries now. I'm not sure about the correctness and completeness of the dictionaries but looks good for start.

That's awesome. The dictionaries are looking good.

Unfortunately, I'm lack of time currently and can't say for sure when it will be finished.

No worries. I'm in the same boat with the amount of time I can give to stylelint these days.

It sounds reasonable, to customize things you want to validate. I don't think making several rules (that we create via createRule) is good solution, because of code duplication and extra tree traversals/parsing/etc. I prefer single rule with options.

Ok, I agree that a single rule would make sense if it avoids code duplication and extra tree traversals.

As a first step there will be a rule that can validate "everything" and have an options.

In stylelint the most requested options are those to increase the leniency of rules by introducing options to ignore something. I anticipate it'll like be the same for this validate plugin.

FYI, across all the rules in stylelint there are two types of ignore options:

Perhaps following a similar convention, at least in the stylelint plugin if not the underlying validator, would be consistent for users?

I'm excited to see csstree progress. I feel stylelint does a good job of helping users to:

  1. Detect code that might, despite being valid, have unintended consequences e.g. declaration-block-no-shorthand-property-overrides, no-descending-specificity, no-duplicate-selectors etc
  2. Limit what css features can and can't be used within a code baes by a team e.g. unit-blacklist, selector-max-id, selector-class-pattern etc.

It feels like csstree will address the missing piece of the puzzle: solid spec validation!

ntwb commented 6 years ago

Yes, we have a specs but there are many verdor specific and non-standart things that are supporting by browsers and using on sites. mdn/data is far from to be complete.

Just as an FYI I just noticed that MDN now also have another repo mdn-browser-compat-data, I'm not sure how relevant it is to this issue but thought it was worth sharing all the same :)