Skip to content

Commit

Permalink
[stylelint-polaris] Fix custom property allowed list plugin (#7877)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

The `polaris/custom-property-allowed-list` plugin is currently reporting
a single problem for what should be two different problems that can
potentially exist in the same decl (`--p-test: var(--p-unknown);`):
- one problem for definition of custom property name with disallowed
prefix
  - `--p-test: var(--p-space-1);`
- one problem for definition of custom property value with invalid
Polaris token or private Polaris component token
  - `color: var(--p-unknown);`

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

This PR:
- Removes remnants of internal vs external configs
- Reports problems with specific error messages for each of the two rule
configuration options
- Clarifies the intentions and configuration of the rule
- Fixes the metadata URLs of custom rules

### 🎩 checklist

- [x] Tested in VS Code
(https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
- [x] Tested in terminal
  • Loading branch information
chloerice committed Feb 17, 2023
1 parent 20d17c6 commit 065df23
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 160 deletions.
7 changes: 7 additions & 0 deletions .changeset/bright-snails-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/stylelint-polaris': patch
---

- Updated the `polaris/custom-property-allowed-list` plugin tests for unified config
- Updated `polaris/custom-property-allowed-list` to report problems with tailored messages for each of the two configuration options
- Fixed metadata URLs for `polaris/*` plugins
34 changes: 14 additions & 20 deletions stylelint-polaris/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,28 +370,22 @@ const stylelintPolarisCoverageOptions = {
message: 'Please use a Polaris z-index token',
},
],
conventions: [
{
'polaris/custom-property-allowed-list': {
// Allow any custom property not prefixed with `--p-`, `--pc-`, or `--polaris-version-`
allowedProperties: [/--(?!(p|pc|polaris-version)-).+/],
allowedValues: {
'/.+/': [
// Note: Order is important.
// The first pattern validates `--p-*`
// custom properties are valid Polaris tokens
...getCustomPropertyNames(tokens),
// and the second pattern flags unknown `--p-*` custom properties
// or usages of our "private" `--pc-*` custom properties
/--(?!(p|pc)-).+/,
],
},
conventions: {
'polaris/custom-property-allowed-list': {
// Allows definition of custom properties not prefixed with `--p-`, `--pc-`, or `--polaris-version-`
allowedProperties: [/--(?!(p|pc|polaris-version)-).+/],
// Allows use of custom properties prefixed with `--p-` that are valid Polaris tokens
allowedValues: {
'/.+/': [
// Note: Order is important
// This pattern allows use of `--p-*` custom properties that are valid Polaris tokens
...getCustomPropertyNames(tokens),
// This pattern flags unknown `--p-*` custom properties or usage of deprecated `--pc-*` custom properties private to polaris-react
/--(?!(p|pc)-).+/,
],
},
},
{
message: 'Please use a Polaris token or component',
},
],
},
'media-queries': [
{
'polaris/media-query-allowed-list': {
Expand Down
5 changes: 4 additions & 1 deletion stylelint-polaris/plugins/coverage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ function getMeta(category, stylelintRuleName) {
'https://polaris.shopify.com/tools/stylelint-polaris/rules';

return {
url: `${baseMetaUrl}/${category}-${stylelintRuleName.replace('/', '-')}`,
url: `${baseMetaUrl}/${category}-${stylelintRuleName.replace(
'polaris/',
'',
)}`,
};
}

Expand Down
2 changes: 1 addition & 1 deletion stylelint-polaris/plugins/coverage/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ testRule({
code: '.class {color: var(--p-legacy-var);}',
description: 'Uses disallowed legacy variable (custom rule)',
message:
'Unexpected disallowed value "--p-legacy-var" - https://polaris.shopify.com/tools/stylelint-polaris/rules/legacy-polaris-global-disallowed-list',
'Unexpected disallowed value "--p-legacy-var" - https://polaris.shopify.com/tools/stylelint-polaris/rules/legacy-global-disallowed-list',
},
],
});
102 changes: 62 additions & 40 deletions stylelint-polaris/plugins/custom-property-allowed-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@ const messages = stylelint.utils.ruleMessages(ruleName, {
/**
* @type {stylelint.RuleMessageFunc}
*/
rejected: (prop, value, invalidProperty, invalidValue) => {
const invalidPropertyMessage = invalidProperty
? `Unexpected custom property "${prop}"`
: null;

const invalidValueMessage = invalidValue
? `Unexpected value "var(${invalidValue})" for property "${prop}"`
: null;

return [invalidPropertyMessage, invalidValueMessage]
.filter(Boolean)
.join(' ');
rejected: (prop, value, prefix, isInvalidProp, invalidValues) => {
if (isInvalidProp) {
return `Unexpected prefix "${prefix}" for defined custom property "${prop}" - Properties with prefixes "--p-" or "--pc-" cannot be defined outside of Polaris"`;
}

if (invalidValues) {
const plural = invalidValues.length > 1;
return `Unexpected value "${value}" for property "${prop}" - Token${
plural ? 's' : ''
} ${invalidValues.map((token) => `"${token}"`).join(', ')} ${
plural ? 'are' : 'is'
} either private or ${plural ? 'do' : 'does'} not exist`;
}
},
});

Expand Down Expand Up @@ -60,7 +61,7 @@ const {rule} = stylelint.createPlugin(

if (!validOptions) {
throw new Error(
`Invalid options were provided to the [${ruleName}] stylelint plugin.\n`,
`Invalid options were provided to the [${ruleName}] rule in your Stylelint config.\n`,
);
}

Expand All @@ -70,58 +71,81 @@ const {rule} = stylelint.createPlugin(
const prop = decl.prop;
const value = decl.value;

const invalidProperty = validateCustomProperties(
const isInvalidProperty = isInvalidCustomProperty(
allowedProperties,
prop,
);

const invalidValues = validateCustomPropertyValues(
const invalidValues = getInvalidCustomPropertyValues(
allowedValues,
prop,
value,
);

if (!invalidValues && !invalidProperty) return;

stylelint.utils.report({
message: messages.rejected(
prop,
value,
/** @type {string} */ (invalidProperty),
/** @type {string} */ (invalidValues?.join(', ')),
),
node: decl,
result,
ruleName,
});
if (isInvalidProperty) {
stylelint.utils.report({
message: messages.rejected(
prop,
value,
getCustomPropertyPrefix(prop),
isInvalidProperty,
invalidValues,
),
node: decl,
result,
ruleName,
});
}

if (invalidValues) {
stylelint.utils.report({
message: messages.rejected(
prop,
value,
undefined,
isInvalidProperty,
invalidValues,
),
node: decl,
result,
ruleName,
});
}
});
};
},
);

/**
* Returns the prefix of a custom property.
* @param {string} property
* @returns {string}
*/
function getCustomPropertyPrefix(property) {
return `--${property.split('-')[2]}-`;
}

/**
* @param {NonNullable<PrimaryOptions['allowedProperties']>} allowedProperties
* @param {string} prop
* @param {string} property
*/
function validateCustomProperties(allowedProperties, prop) {
if (!isCustomProperty(prop)) return;
function isInvalidCustomProperty(allowedProperties, property) {
if (!isCustomProperty(property)) return false;

const isValid = allowedProperties.some((allowedProperty) => {
return matchesStringOrRegExp(prop, allowedProperty);
return matchesStringOrRegExp(property, allowedProperty);
});

if (isValid) return;

return prop;
return !isValid;
}

/**
* @param {NonNullable<PrimaryOptions['allowedValues']>} allowedValues
* @param {string} prop
* @param {string} value
* @returns {string[] | undefined}
*/
function validateCustomPropertyValues(allowedValues, prop, value) {
/** @type {string[]} */
function getInvalidCustomPropertyValues(allowedValues, prop, value) {
const invalidValues = [];

const unprefixedProp = vendorUnprefixed(prop);
Expand All @@ -147,9 +171,7 @@ function validateCustomPropertyValues(allowedValues, prop, value) {
}
});

if (!invalidValues.length) return;

return invalidValues;
if (invalidValues.length > 0) return invalidValues;
}

module.exports = {
Expand Down

0 comments on commit 065df23

Please sign in to comment.