Skip to content

Commit

Permalink
Update stylelint-polaris to allow disabling specific coverage rules (
Browse files Browse the repository at this point in the history
…#7866)

### WHAT is this pull request doing?

This PR updates `stylelint-polaris` to allow disabling specific coverage
rules instead of entire categories:
- Updated coverage plugin to append rule names to the created category
rule name
- Removed `stylelint-` prefix from all custom plugin rule names. This is
now extraneous and helps reduce clutter in CI and the VS Code error
display
- Updated `stylelint-polaris` config to match new rule names
- Updated existing stylelint disable comments in `polaris-react`

Example output:

CLI:

![image](https://user-images.githubusercontent.com/32409546/206580343-8113fb68-5fa4-4860-a26d-d57e32a3e13d.png)

VS Code:

![image](https://user-images.githubusercontent.com/32409546/206580372-938ba800-0b9f-4150-9b04-3510656e92f7.png)

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
  • Loading branch information
aaronccasanova committed Dec 9, 2022
1 parent 65131df commit 4b96147
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 62 deletions.
5 changes: 5 additions & 0 deletions .changeset/lovely-taxis-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/stylelint-polaris': patch
---

Updated `stylelint-polaris` to allow disabling specific coverage rules instead of entire categories
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module.exports = {
rules: {
'unit-disallowed-list': ['rem'],
'selector-pseudo-class-no-unknown': true,
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [/([\w-]+\.)?safe-area-for($|\()/],
},
},
Expand Down
7 changes: 3 additions & 4 deletions polaris-react/src/components/Button/Button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,9 @@
.plain {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include recolor-icon(var(--p-interactive));
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable-next-line polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
margin: calc(-1 * var(--pc-button-vertical-padding))
calc(-1 * var(--p-space-2));
// stylelint-enable stylelint-polaris/coverage/conventions
padding-left: var(--p-space-2);
padding-right: var(--p-space-2);
background: transparent;
Expand Down Expand Up @@ -358,10 +357,10 @@
}

&.sizeLarge {
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
margin: calc(-1 * var(--pc-button-large-vertical-padding))
calc(-1 * var(--p-space-5));
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list
}

&.iconOnly {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
// stylelint-disable selector-max-compound-selectors -- generated by polaris-migrator DO NOT COPY
// stylelint-disable selector-max-type -- generated by polaris-migrator DO NOT COPY
.ConnectedFilterControl {
// stylelint-disable stylelint-polaris/coverage/conventions -- Polaris component custom properties
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
--pc-connceted-filter-control-item: 10;
--pc-connceted-filter-control-focused: 20;
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
display: flex;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@ $range-output-size: 32px;
transform: translateY(calc(-1 * var(--pc-range-slider-output-spacing)));

@media #{$p-breakpoints-md-up} {
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
transform: translateY(
calc(-1 * (var(--pc-range-slider-output-spacing) * 0.5))
);
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list
}
}
// stylelint-enable selector-max-specificity, selector-max-combinators, selector-max-class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@
width: 100%;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
height: var(--pc-range-slider-track-height);
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- Polaris component custom properties
background-image: linear-gradient(
to right,
var(--pc-single-thumb-gradient-colors)
);
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list
border: none;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
border-radius: var(--pc-range-slider-track-height);
Expand Down Expand Up @@ -141,19 +141,19 @@

&::-ms-thumb {
margin-top: 0;
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
transform: translateY(calc(var(--pc-range-slider-thumb-size) * 0.2))
scale(0.4);
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list
}

&::-webkit-slider-thumb {
// stylelint-disable stylelint-polaris/coverage/conventions, function-calc-no-unspaced-operator -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list, function-calc-no-unspaced-operator -- generated by polaris-migrator DO NOT COPY
margin-top: calc(
(var(--pc-range-slider-thumb-size) - var(--pc-range-slider-track-height)) *
-0.5
);
// stylelint-enable stylelint-polaris/coverage/conventions, function-calc-no-unspaced-operator
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list, function-calc-no-unspaced-operator
}

&:active {
Expand Down Expand Up @@ -202,11 +202,11 @@

/// Output value indicator
$range-output-size: 32px;
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
$range-output-translate-x: calc(
-50% + var(--pc-range-slider-output-factor) * var(--pc-range-slider-thumb-size)
);
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list

.Output {
// stylelint-disable-next-line -- Polaris component custom properties
Expand All @@ -228,17 +228,17 @@ $range-output-translate-x: calc(
transition-timing-function: var(--p-ease);

.Input:active + & {
// stylelint-disable stylelint-polaris/coverage/layout -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/layout/polaris/global-disallowed-list -- generated by polaris-migrator DO NOT COPY
$range-thumb-size-difference: var(--p-range-slider-thumb-size-active) -
var(--p-range-slider-thumb-size-base);
// stylelint-enable stylelint-polaris/coverage/layout
// stylelint-enable polaris/layout/polaris/global-disallowed-list
opacity: 1;
visibility: visible;
// stylelint-disable stylelint-polaris/coverage/conventions, stylelint-polaris/coverage/layout -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/layout/declaration-property-value-disallowed-list, polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
bottom: calc(
var(--pc-range-slider-thumb-size) + #{$range-thumb-size-difference}
);
// stylelint-enable stylelint-polaris/coverage/conventions, stylelint-polaris/coverage/layout
// stylelint-enable polaris/layout/declaration-property-value-disallowed-list, polaris/conventions/polaris/custom-properties-allowed-list
}
}

Expand Down Expand Up @@ -268,11 +268,11 @@ $range-output-translate-x: calc(
transform: translateY(calc(-1 * var(--pc-range-slider-output-spacing)));

@media #{$p-breakpoints-md-up} {
// stylelint-disable stylelint-polaris/coverage/conventions -- generated by polaris-migrator DO NOT COPY
// stylelint-disable polaris/conventions/polaris/custom-properties-allowed-list -- generated by polaris-migrator DO NOT COPY
transform: translateY(
calc(-1 * (var(--pc-range-slider-output-spacing) * 0.4))
);
// stylelint-enable stylelint-polaris/coverage/conventions
// stylelint-enable polaris/conventions/polaris/custom-properties-allowed-list
}
}
// stylelint-enable selector-max-specificity, selector-max-combinators, selector-max-class
Expand Down
56 changes: 28 additions & 28 deletions stylelint-polaris/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?color/,
/([\w-]+\.)?filter/,
],
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [
// Legacy mixins
/([\w-]+\.)?color-icon($|\()/,
Expand All @@ -38,7 +38,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?ms-high-contrast-color/,
],
},
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$polaris-colors/,
/\$color-filter-palette-data/,
Expand All @@ -61,10 +61,10 @@ const stylelintPolarisCoverageOptions = {
'/^transition/': ['ms', 's'],
},
],
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [/([\w-]+\.)?skeleton-shimmer($|\()/],
},
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$duration-data/,
/\$polaris-duration-map/,
Expand All @@ -91,7 +91,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?font-size/,
/([\w-]+\.)?line-height/,
],
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [
/([\w-]+\.)?truncate($|\()/,
/([\w-]+\.)?text-breakword($|\()/,
Expand All @@ -111,7 +111,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?text-style-subheading($|\()/,
],
},
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$typography-condensed/,
/\$typography-condensed/,
Expand Down Expand Up @@ -172,7 +172,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?top-bar-height/,
/([\w-]+\.)?z-index/,
],
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [
/([\w-]+\.)?hidden-when-printing($|\()/,
/([\w-]+\.)?print-hidden($|\()/,
Expand All @@ -182,7 +182,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?skeleton-page-secondary-actions-layout($|\()/,
],
},
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$layout-width-data/,
/\$navigation-width/,
Expand Down Expand Up @@ -214,7 +214,7 @@ const stylelintPolarisCoverageOptions = {
'/^gap/': ['px', 'rem', 'em'],
},
],
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$polaris-spacing/,
/\$spacing-data/,
Expand All @@ -235,7 +235,7 @@ const stylelintPolarisCoverageOptions = {
outline: ['px', 'rem', 'em'],
},
],
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [
/([\w-]+\.)?border-radius/,
/([\w-]+\.)?border-width/,
Expand All @@ -247,7 +247,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?no-focus-ring($|\()/,
],
},
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$border-radius-data/,
/\$border-width-data/,
Expand Down Expand Up @@ -275,7 +275,7 @@ const stylelintPolarisCoverageOptions = {
},
],
'property-disallowed-list': ['text-shadow'],
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$shadows-data/,
/\$fixed-element-stacking-order/,
Expand All @@ -297,7 +297,7 @@ const stylelintPolarisCoverageOptions = {
},
],
'function-disallowed-list': [/([\w-]+\.)?z-index/],
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy mixin map-get data
/\$fixed-element-stacking-order/,
/\$global-elements/,
Expand All @@ -306,7 +306,7 @@ const stylelintPolarisCoverageOptions = {
],
},
conventions: {
'stylelint-polaris/custom-properties-allowed-list': {
'polaris/custom-properties-allowed-list': {
// Allow any custom property not prefixed with `--p-`, `--pc-`, or `--polaris-version-`
allowedProperties: [/--(?!(p|pc|polaris-version)-).+/],
allowedValues: {
Expand All @@ -323,7 +323,7 @@ const stylelintPolarisCoverageOptions = {
},
},
'media-queries': {
'stylelint-polaris/media-queries-allowed-list': {
'polaris/media-queries-allowed-list': {
// Allowed media types and media conditions
// https://www.w3.org/TR/mediaqueries-5/#media
allowedMediaTypes: ['print', 'screen'],
Expand All @@ -339,7 +339,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?layout-width/,
],
// Legacy mixins
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [
/([\w-]+\.)?after-topbar-sheet($|\()/,
/([\w-]+\.)?breakpoint-after($|\()/,
Expand Down Expand Up @@ -375,7 +375,7 @@ const stylelintPolarisCoverageOptions = {
},
legacy: {
// Legacy mixins
'stylelint-polaris/at-rule-disallowed-list': {
'polaris/at-rule-disallowed-list': {
include: [
/([\w-]+\.)?base-button-disabled($|\()/,
/([\w-]+\.)?button-base($|\()/,
Expand All @@ -402,7 +402,7 @@ const stylelintPolarisCoverageOptions = {
/([\w-]+\.)?available-names/,
/([\w-]+\.)?map-extend/,
],
'stylelint-polaris/global-disallowed-list': [
'polaris/global-disallowed-list': [
// Legacy variables
/ \* \$/,
// Legacy custom properties
Expand All @@ -414,22 +414,22 @@ const stylelintPolarisCoverageOptions = {
},
};

const stylelintPolarisCoverageCategories = Object.keys(
stylelintPolarisCoverageOptions,
).join('|');

/** @type {import('stylelint').Config} */
module.exports = {
customSyntax: 'postcss-scss',
reportDescriptionlessDisables: true,
reportInvalidScopeDisables: [
true,
{
except: [
new RegExp(
String.raw`^stylelint-polaris\/coverage\/(?:${stylelintPolarisCoverageCategories})$`,
),
],
// Report invalid scope disables for all rules except coverage rules
// Note: This doesn't affect the default Stylelint behavior/reporting
// and is only need because we dynamically create these rule names
except: Object.entries(stylelintPolarisCoverageOptions).flatMap(
([categoryName, categoryConfigRules]) =>
Object.keys(categoryConfigRules).map(
(categoryRuleName) => `polaris/${categoryName}/${categoryRuleName}`,
),
),
},
],
plugins: [
Expand All @@ -441,6 +441,6 @@ module.exports = {
'./plugins/media-queries-allowed-list',
],
rules: {
'stylelint-polaris/coverage': stylelintPolarisCoverageOptions,
'polaris/coverage': stylelintPolarisCoverageOptions,
},
};
2 changes: 1 addition & 1 deletion stylelint-polaris/plugins/at-rule-disallowed-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const stylelint = require('stylelint');

const {matchesStringOrRegExp, isRegExp, isString} = require('../../utils');

const ruleName = 'stylelint-polaris/at-rule-disallowed-list';
const ruleName = 'polaris/at-rule-disallowed-list';

const messages = stylelint.utils.ruleMessages(ruleName, {
/**
Expand Down
6 changes: 3 additions & 3 deletions stylelint-polaris/plugins/coverage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const stylelint = require('stylelint');

const {isPlainObject} = require('../../utils');

const ruleName = 'stylelint-polaris/coverage';
const ruleName = 'polaris/coverage';

/**
* @typedef {{
Expand Down Expand Up @@ -30,7 +30,7 @@ module.exports = stylelint.createPlugin(
categoryConfigRules,
)) {
rules.push({
coverageRuleName: `${ruleName}/${categoryName}`,
coverageRuleName: `polaris/${categoryName}/${categoryRuleName}`,
categoryRuleName,
categoryRuleSettings,
categoryRuleSeverity: categoryRuleSettings?.[1]?.severity,
Expand Down Expand Up @@ -68,7 +68,7 @@ module.exports = stylelint.createPlugin(
stylelint.utils.report({
result,
ruleName: coverageRuleName,
message: warning.text,
message: warning.text.replace(` (${categoryRuleName})`, ''),
severity:
categoryRuleSeverity ??
result.stylelint.config?.defaultSeverity ??
Expand Down

0 comments on commit 4b96147

Please sign in to comment.