[stylelint-polaris] Validate rule configs

[chloerice]

tl;dr:

The rule settings configured have a few problems that need to be addressed before we ship @shopify/stylelint-polaris to Shopify/web.

---

Why

The rule configurations in each category need to be checked for validity to address known (and uncover unknown) issues before we ship to Shopify/web. Look for and flag or fix:

• Unwarranted or duplicate errors or warnings
  • e.g., property present in property-disallowed-list and declaration-property-value-disallowed-list
• Unreported errors or warnings
  • e.g., colors => declaration-property-value-disallowed-list => opacity: [/(?!0|1)\d$|^\d{2,}|^[1-9]+\.|^\d+\.\d+\.|^0\.\d{3,}/] doesn't report problems at all as configured
• Properties in the property-disallowed-list that map to specific components and should move to declaration-property-value-disallowed-list and have detailed error messages
  • e.g., should position be wholly disallowed, or should position sticky; be specifically disallowed since folks should use the Sticky component
• Regexs that don't do what they're intended to
• Other unknown discrepancies you may find

How

To uncover issues with the rule configs, prioritize validating the disabled errors and warnings for each category using the stylesheets of some of the more complex/configurable @shopify/polaris components, such as:

• Button.scss
• TextField.scss
• Frame.scss
• RangeSlider.scss
• IndexTable.scss

For example, run yarn stylelint polaris-react/src/components/Button/Button.scss --ignore-disables and check that errors and warnings are valid.

---

Let's split up the work by category so we can move quickly without duplicating effort (just add your name to the category when you start to tackle it).

• Colors (Chloe)
• Motion (Sophie: no failures in polaris-react but I also searched to make sure this was true)
• Typography (Sophie)
• Layout (Chloe)
• Spacing (Sophie)
• Shape (Sophie)
• Depth (Sophie)
• Z-index (Sophie)
• Conventions (Sophie)
• Media queries (Sophie)
• Legacy (Sophie)

[sophschneider]

created this google sheet with current stylelint errors to track validation progress on the files Chloe listed above
https://docs.google.com/spreadsheets/d/1JbtYE56DoUR9K3P4StjPmOJFYBbvFlJ3QiI_T7YQl-A/edit#gid=0

[sophschneider]

Bugs

• [stylelint-polaris][bug hunt] Z-index correct token use is being falsely reported #7865
• [stylelint-polaris][bug hunt] p-border-radius-base is reported even though it is a token #7869
• [stylelint-polaris][bug hunt] Modify at-rule-disallowed-list regexes to match whole word before parameter #7892

Questions

• Should we be reporting z-index: auto, 1, 0, -1 ? (we are currently)
• Should we be reporting top: 0, bottom: 0, etc ? (we are currently)
• For not allowed @includes, as a consumer I would be confused what my other options would be
• Where would we document that its ok to ignore things with context?
• focus ring mix in is used A LOT... should we have a token or component to deal with this?
• can we use spacing tokens for box shadows?

Replacement components

• Could padding and margin failures be replaced by Stack?
• Do we suggest to use Text component on typography failures?
• Suggest Bleed component for negative margins?
• Suggest Box component for opacity, position, padding, border, box-shadow, etc?
• Suggest Divider for border-bottom?
• Lots of the legacy at-rule-disallowed-list can be replaced with components
  • i.e. @include unstyled-button can be replaced with <Button plain />
• Suggest Page or Layout components for media query failures

Note

• There are no motion failures in polaris-react, should we test in shopify/web?

[chloerice]

Bugs

• The custom-property-allowed-list plugin is currently reporting a single problem for what should be two different problems (fix WIP [stylelint-polaris] Fix custom property allowed list plugin #7877)
  • one problem for definition of custom property name with disallowed prefix
  • one problem for definition of custom property value with invalid or disallowed token
• Disable comments for custom plugins need to be de-namespaced

  - // stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list
  + // stylelint-disable-next-line polaris/conventions/custom-property-allowed-list

Questions

• Should custom-properties-allowed-list or at-rule-allowed-list be refactored to account for unified config, or are we intentionally disallowing --pc- and non-legacy @mixins in Polaris React?

  • For example in polaris-react/src/components/Button/Button.scss

   459:3  ✖  Unexpected value "var(--pc-button-large-min-height)"  polaris/conventions/custom-property-allowed-list
             for property "min-width"   
             
   335:5  ✖  Unexpected @include rule "recolor-icon(var(--p-interactive-critical))" -  polaris/colors/at-rule-disallowed-list                                                                              
             Please use a Polaris color token