css-code-quality icon indicating copy to clipboard operation
css-code-quality copied to clipboard

Make an exception for Custom Properties

Open jfbrennan opened this issue 1 year ago • 4 comments

Excellent tool btw. Thanks for creating this!

The only red I got was one "Avoid many Declarations in single RuleSet". It's because of a large block of Custom Properties set on :root:

:root {
  --l-color-gray-0: #253843;
  --l-color-gray-1: #55646d;
  --l-color-gray-2: #959fa4;
  --l-color-gray-3: #999fa3;
  --l-color-gray-4: #c4c5c6;
  --l-color-gray-5: #d8dee1;
  --l-color-gray-6: #f2f2f2;
  ...
}

This common and correct, especially in global stylesheets like design systems. Naming conventions and comments in the source code make it more maintainable.

Can the tool be made to understand large blocks of Custom Properties set on root? Can it then make an exception for this case and not flag it under "Avoid many Declarations in single RuleSet"?

jfbrennan avatar Aug 21 '24 15:08 jfbrennan

That feels like a very legit use case that I've run into myself as well.

Based on the current implementation we'd need some work to make it happen but it's definitely possible.

bartveneman avatar Sep 02 '24 12:09 bartveneman

@bartveneman if you can send me to a good starting point I'm willing to help

jfbrennan avatar Sep 05 '24 20:09 jfbrennan

First we need changes in https://github.com/projectwallace/css-analyzer, because currently is calculates the size of rules by looking how many children it has. I haven't decided yet how I'd want to reflect counting custom properties yet, either via an extra metric or maybe something else. After that we need to update this library with the latest analyzer version, update the corresponding rule(s).

bartveneman avatar Sep 06 '24 14:09 bartveneman

Another option is to remove this rule entirely: we have another rule to check for a low enough average, so maybe we should leave it at that?

All these rules are highly arbitrary anyway and this one is always biting me too, so why not just drop it?

bartveneman avatar Sep 06 '24 14:09 bartveneman

I read https://css-tricks.com/organizing-design-system-component-patterns-with-css-cascade-layers/#aa-defining-button-state-styles last week and I'm now leaning towards keeping this rule as-is. You can break up large rules with dozens of custom properties like this:

:root {
  @layer colors {
    --brand-primary: red;
    --brand-secondary: green;
    /* etc. */
  }

  @layer sizing {
    --space-1: 0.25rem;
    /* etc. */
  }

  @layer fonts {
    /* etc. */
  }

  @layer shadows {
    /* etc. */
  }

bartveneman avatar Feb 16 '25 10:02 bartveneman