From 7a0f948d6c783d87b76dbb27c25e8e654800a6e6 Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Thu, 25 Nov 2021 13:44:17 +0100 Subject: [PATCH 1/9] Add skip import check --- docs/rules/no-system-props.md | 6 +++++- src/rules/no-system-props.js | 39 +++++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/docs/rules/no-system-props.md b/docs/rules/no-system-props.md index 0bc2f706..ecb908ba 100644 --- a/docs/rules/no-system-props.md +++ b/docs/rules/no-system-props.md @@ -1,4 +1,4 @@ -# Disallow use of styled-system props (no-system-colors) +# Disallow use of styled-system props (no-system-props) 🔧 The `--fix` option on the [ESLint CLI](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. @@ -41,6 +41,10 @@ import {Avatar} from 'some-other-library' ## Options +- `skipImportCheck` (default: `false`) + + By default, the `no-system-props` rule will only check for styled system props used in functions and components that are imported from `@primer/components`. You can disable this behavior by setting `skipImportCheck` to `true`. This is useful for linting custom components that pass styled system props down to Primer React components. + - `includeUtilityComponents` (default: `false`) By default, `Box` and `Text` are excluded because styled system props are not deprecated in our utility components. If you prefer to avoid styled system props there as well for consistency, you can set `includeUtilityComponents` to `true`. diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index a76a6b11..608693ea 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -1,6 +1,6 @@ -const {isPrimerComponent} = require('../utils/is-primer-component') -const {pick} = require('@styled-system/props') -const {some, last} = require('lodash') +const { isPrimerComponent } = require('../utils/is-primer-component') +const { pick } = require('@styled-system/props') +const { some, last } = require('lodash') // Components for which we allow all styled system props const alwaysExcludedComponents = new Set([ @@ -29,6 +29,9 @@ module.exports = { schema: [ { properties: { + skipImportCheck: { + type: 'boolean' + }, includeUtilityComponents: { type: 'boolean' } @@ -40,6 +43,10 @@ module.exports = { } }, create(context) { + // If `skipImportCheck` is true, this rule will check for deprecated colors + // used in any components (not just ones that are imported from `@primer/components`). + const skipImportCheck = context.options[0] ? context.options[0].skipImportCheck : false + const includeUtilityComponents = context.options[0] ? context.options[0].includeUtilityComponents : false const excludedComponents = new Set([ @@ -49,7 +56,7 @@ module.exports = { return { JSXOpeningElement(jsxNode) { - if (!isPrimerComponent(jsxNode.name, context.getScope(jsxNode))) return + if (!skipImportCheck && !isPrimerComponent(jsxNode.name, context.getScope(jsxNode))) return if (excludedComponents.has(jsxNode.name.name)) return // Create an object mapping from prop name to the AST node for that attribute @@ -65,7 +72,7 @@ module.exports = { // Create an array of system prop attribute nodes let systemProps = Object.values(pick(propsByNameObject)) - let excludedProps = excludedComponentProps.has(jsxNode.name.name) + const excludedProps = excludedComponentProps.has(jsxNode.name.name) ? new Set([...alwaysExcludedProps, ...excludedComponentProps.get(jsxNode.name.name)]) : alwaysExcludedProps @@ -100,15 +107,15 @@ module.exports = { ...systemProps.map(node => fixer.remove(node)), ...(stylesToAdd.size > 0 ? [ - existingSxProp - ? // Update an existing sx prop - fixer.insertTextAfter( - last(existingSxProp.value.expression.properties), - `, ${objectEntriesStringFromStylesMap(stylesToAdd)}` - ) - : // Insert new sx prop - fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(systemPropstylesMap)) - ] + existingSxProp + ? // Update an existing sx prop + fixer.insertTextAfter( + last(existingSxProp.value.expression.properties), + `, ${objectEntriesStringFromStylesMap(stylesToAdd)}` + ) + : // Insert new sx prop + fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(systemPropstylesMap)) + ] : []) ] } @@ -143,12 +150,12 @@ const excludeSxEntriesFromStyleMap = (stylesMap, sxProp) => { if ( !sxProp.value || sxProp.value.type !== 'JSXExpressionContainer' || - sxProp.value.expression.type != 'ObjectExpression' + sxProp.value.expression.type !== 'ObjectExpression' ) { return stylesMap } return new Map( - [...stylesMap].filter(([key, _value]) => { + [...stylesMap].filter(([key]) => { return !some(sxProp.value.expression.properties, p => p.type === 'Property' && p.key.name === key) }) ) From f5bf869d5d4c099bab0a52dffdc3160681bc8157 Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Thu, 25 Nov 2021 13:50:04 +0100 Subject: [PATCH 2/9] Prettify code Signed-off-by: Alexis Rico --- src/rules/no-system-props.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index 608693ea..aaf47877 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -1,6 +1,6 @@ -const { isPrimerComponent } = require('../utils/is-primer-component') -const { pick } = require('@styled-system/props') -const { some, last } = require('lodash') +const {isPrimerComponent} = require('../utils/is-primer-component') +const {pick} = require('@styled-system/props') +const {some, last} = require('lodash') // Components for which we allow all styled system props const alwaysExcludedComponents = new Set([ @@ -107,15 +107,15 @@ module.exports = { ...systemProps.map(node => fixer.remove(node)), ...(stylesToAdd.size > 0 ? [ - existingSxProp - ? // Update an existing sx prop - fixer.insertTextAfter( - last(existingSxProp.value.expression.properties), - `, ${objectEntriesStringFromStylesMap(stylesToAdd)}` - ) - : // Insert new sx prop - fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(systemPropstylesMap)) - ] + existingSxProp + ? // Update an existing sx prop + fixer.insertTextAfter( + last(existingSxProp.value.expression.properties), + `, ${objectEntriesStringFromStylesMap(stylesToAdd)}` + ) + : // Insert new sx prop + fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(systemPropstylesMap)) + ] : []) ] } From 4dc79f0ec6fe7e415b4c3a861eb0955a90bd7848 Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Thu, 25 Nov 2021 13:59:47 +0100 Subject: [PATCH 3/9] Add circle octicon to excluded components --- src/rules/no-system-props.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index aaf47877..84a247f4 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -14,6 +14,7 @@ const utilityComponents = new Set(['Box', 'Text']) const excludedComponentProps = new Map([ ['AnchoredOverlay', new Set(['width', 'height'])], ['Avatar', new Set(['size'])], + ['CircleOcticon', new Set(['size'])] ['Dialog', new Set(['width', 'height'])], ['ProgressBar', new Set(['bg'])], ['Spinner', new Set(['size'])], From 2bd941940c361a0b678ce262a2b8474a9e2c5f32 Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Thu, 25 Nov 2021 21:52:34 +0100 Subject: [PATCH 4/9] Update comment --- src/rules/no-system-props.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index 84a247f4..bcd33981 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -44,7 +44,7 @@ module.exports = { } }, create(context) { - // If `skipImportCheck` is true, this rule will check for deprecated colors + // If `skipImportCheck` is true, this rule will check for deprecated styled system props // used in any components (not just ones that are imported from `@primer/components`). const skipImportCheck = context.options[0] ? context.options[0].skipImportCheck : false From 11cfeb92f1f335f19117234d3c72ee8f813e1eed Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Wed, 8 Dec 2021 13:17:38 -0800 Subject: [PATCH 5/9] @primer/components -> @primer/react --- docs/rules/no-system-props.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/no-system-props.md b/docs/rules/no-system-props.md index ecb908ba..5f3eda73 100644 --- a/docs/rules/no-system-props.md +++ b/docs/rules/no-system-props.md @@ -43,7 +43,7 @@ import {Avatar} from 'some-other-library' - `skipImportCheck` (default: `false`) - By default, the `no-system-props` rule will only check for styled system props used in functions and components that are imported from `@primer/components`. You can disable this behavior by setting `skipImportCheck` to `true`. This is useful for linting custom components that pass styled system props down to Primer React components. + By default, the `no-system-props` rule will only check for styled system props used in functions and components that are imported from `@primer/react`. You can disable this behavior by setting `skipImportCheck` to `true`. This is useful for linting custom components that pass styled system props down to Primer React components. - `includeUtilityComponents` (default: `false`) From b6aabf55da99ff416c0462c2cbb71d5e2aeb3e9c Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Wed, 8 Dec 2021 22:30:25 +0100 Subject: [PATCH 6/9] Fix tests --- src/rules/no-system-props.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index bcd33981..9137dcc3 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -14,7 +14,7 @@ const utilityComponents = new Set(['Box', 'Text']) const excludedComponentProps = new Map([ ['AnchoredOverlay', new Set(['width', 'height'])], ['Avatar', new Set(['size'])], - ['CircleOcticon', new Set(['size'])] + ['CircleOcticon', new Set(['size'])], ['Dialog', new Set(['width', 'height'])], ['ProgressBar', new Set(['bg'])], ['Spinner', new Set(['size'])], From c94f28ade611a69719c2bca8d999ce7f6223d77c Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Wed, 12 Jan 2022 08:54:53 +0100 Subject: [PATCH 7/9] Exclude ``token`` size prop --- src/rules/no-system-props.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index 9137dcc3..138d5dd8 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -18,7 +18,8 @@ const excludedComponentProps = new Map([ ['Dialog', new Set(['width', 'height'])], ['ProgressBar', new Set(['bg'])], ['Spinner', new Set(['size'])], - ['StyledOcticon', new Set(['size'])] + ['StyledOcticon', new Set(['size'])], + ['Token', new Set(['size'])] ]) const alwaysExcludedProps = new Set(['variant']) From 4f3270a5737fd69e506ec61476ce3f1011777ba4 Mon Sep 17 00:00:00 2001 From: Alexis Rico Date: Wed, 12 Jan 2022 09:22:27 +0100 Subject: [PATCH 8/9] Exclude other ``token`` components --- src/rules/no-system-props.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rules/no-system-props.js b/src/rules/no-system-props.js index 138d5dd8..5552c0fe 100644 --- a/src/rules/no-system-props.js +++ b/src/rules/no-system-props.js @@ -14,8 +14,10 @@ const utilityComponents = new Set(['Box', 'Text']) const excludedComponentProps = new Map([ ['AnchoredOverlay', new Set(['width', 'height'])], ['Avatar', new Set(['size'])], + ['AvatarToken', new Set(['size'])], ['CircleOcticon', new Set(['size'])], ['Dialog', new Set(['width', 'height'])], + ['IssueLabelToken', new Set(['size'])], ['ProgressBar', new Set(['bg'])], ['Spinner', new Set(['size'])], ['StyledOcticon', new Set(['size'])], From 5f68eb7e222e2a41c527b5036b1308ff293e7a0d Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Wed, 22 Mar 2023 10:44:04 -0600 Subject: [PATCH 9/9] Create .changeset/gorgeous-dots-promise.md --- .changeset/gorgeous-dots-promise.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gorgeous-dots-promise.md diff --git a/.changeset/gorgeous-dots-promise.md b/.changeset/gorgeous-dots-promise.md new file mode 100644 index 00000000..100f79c2 --- /dev/null +++ b/.changeset/gorgeous-dots-promise.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-primer-react": minor +--- + +`no-system-props`: Add `skipImportCheck` option