From 3950fb403a1d2489247fb33f013b58d287349b73 Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Fri, 17 Sep 2021 21:55:54 +0000 Subject: [PATCH 1/3] Check nested properties of sx objects --- package-lock.json | 7 +++++- package.json | 5 +++- .../__tests__/no-deprecated-colors.test.js | 9 +++++++ src/rules/no-deprecated-colors.js | 24 +++++++++---------- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 021422f5..c7e5ecd3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-primer-react", - "version": "0.4.0", + "version": "0.4.1", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -2534,6 +2534,11 @@ } } }, + "eslint-traverse": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/eslint-traverse/-/eslint-traverse-1.0.0.tgz", + "integrity": "sha512-bSp37rQs93LF8rZ409EI369DGCI4tELbFVmFNxI6QbuveS7VRxYVyUhwDafKN/enMyUh88HQQ7ZoGUHtPuGdcw==" + }, "eslint-utils": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/eslint-utils/-/eslint-utils-2.1.0.tgz", diff --git a/package.json b/package.json index eaa66492..841e5dee 100644 --- a/package.json +++ b/package.json @@ -32,5 +32,8 @@ "eslint": ">=4.19.0", "@primer/primitives": ">=4.6.2" }, - "prettier": "@github/prettier-config" + "prettier": "@github/prettier-config", + "dependencies": { + "eslint-traverse": "^1.0.0" + } } diff --git a/src/rules/__tests__/no-deprecated-colors.test.js b/src/rules/__tests__/no-deprecated-colors.test.js index d4d46eeb..8f16eb08 100644 --- a/src/rules/__tests__/no-deprecated-colors.test.js +++ b/src/rules/__tests__/no-deprecated-colors.test.js @@ -79,6 +79,15 @@ ruleTester.run('no-deprecated-colors', rule, { } ] }, + { + code: `import {Box} from "@primer/components"; `, + output: `import {Box} from "@primer/components"; `, + errors: [ + { + message: '"bg.primary" is deprecated. Use "canvas.default" instead.' + } + ] + }, { code: `import {Box} from "@primer/components"; `, errors: [ diff --git a/src/rules/no-deprecated-colors.js b/src/rules/no-deprecated-colors.js index d55f40b3..7501e570 100644 --- a/src/rules/no-deprecated-colors.js +++ b/src/rules/no-deprecated-colors.js @@ -1,4 +1,5 @@ const deprecations = require('@primer/primitives/dist/deprecations/colors') +const traverse = require('eslint-traverse') const styledSystemColorProps = ['color', 'bg', 'backgroundColor', 'borderColor', 'textShadow', 'boxShadow'] @@ -25,19 +26,18 @@ module.exports = { // Check for the sx prop if (propName === 'sx' && attribute.value.expression.type === 'ObjectExpression') { - // Ignore non-literal properties - const sxProperties = attribute.value.expression.properties.filter( - property => property.type === 'Property' && property.value.type === 'Literal' - ) - - for (const prop of sxProperties) { - const propName = prop.key.name - const propValue = prop.value.value - - if (styledSystemColorProps.includes(propName) && Object.keys(deprecations).includes(propValue)) { - replaceDeprecatedColor(context, prop.value, propValue) + // Search all properties of the sx object (even nested properties) + traverse(context, attribute.value, path => { + if (path.node.type === 'Property' && path.node.value.type === 'Literal') { + const prop = path.node + const propName = prop.key.name + const propValue = prop.value.value + + if (styledSystemColorProps.includes(propName) && Object.keys(deprecations).includes(propValue)) { + replaceDeprecatedColor(context, prop.value, propValue) + } } - } + }) } // Check if styled-system color prop is using a deprecated color From bdf389c5b3d5e608e87bcfc292c07b233f48e5fc Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Fri, 17 Sep 2021 22:56:59 +0000 Subject: [PATCH 2/3] Handle functions in sx objects --- .../__tests__/no-deprecated-colors.test.js | 27 +++++++++++ src/rules/no-deprecated-colors.js | 48 ++++++++++++++++--- 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/rules/__tests__/no-deprecated-colors.test.js b/src/rules/__tests__/no-deprecated-colors.test.js index 8f16eb08..0cb8b754 100644 --- a/src/rules/__tests__/no-deprecated-colors.test.js +++ b/src/rules/__tests__/no-deprecated-colors.test.js @@ -79,6 +79,33 @@ ruleTester.run('no-deprecated-colors', rule, { } ] }, + { + code: `import {Box} from "@primer/components"; theme.shadows.autocomplete.shadow}} />`, + output: `import {Box} from "@primer/components"; theme.shadows.shadow.medium}} />`, + errors: [ + { + message: '"theme.shadows.autocomplete.shadow" is deprecated. Use "theme.shadows.shadow.medium" instead.' + } + ] + }, + { + code: `import {Box} from "@primer/components"; \`0 1px 2px \${theme.colors.text.primary}\`}} />`, + output: `import {Box} from "@primer/components"; \`0 1px 2px \${theme.colors.fg.default}\`}} />`, + errors: [ + { + message: '"theme.colors.text.primary" is deprecated. Use "theme.colors.fg.default" instead.' + } + ] + }, + { + code: `import {Box} from "@primer/components"; \`0 1px 2px \${t.colors.text.primary}\`}} />`, + output: `import {Box} from "@primer/components"; \`0 1px 2px \${t.colors.fg.default}\`}} />`, + errors: [ + { + message: '"t.colors.text.primary" is deprecated. Use "t.colors.fg.default" instead.' + } + ] + }, { code: `import {Box} from "@primer/components"; `, output: `import {Box} from "@primer/components"; `, diff --git a/src/rules/no-deprecated-colors.js b/src/rules/no-deprecated-colors.js index 7501e570..d2af5de7 100644 --- a/src/rules/no-deprecated-colors.js +++ b/src/rules/no-deprecated-colors.js @@ -37,6 +37,34 @@ module.exports = { replaceDeprecatedColor(context, prop.value, propValue) } } + + // Check functions passed to sx object properties + // (e.g. boxShadow: theme => `0 1px 2px ${theme.colors.text.primary}` ) + if (path.node.type === 'Property' && path.node.value.type === 'ArrowFunctionExpression') { + traverse(context, path.node.value.body, path => { + if (path.node.type === 'MemberExpression') { + // Convert MemberExpression AST to string + const code = context.getSourceCode().getText(path.node) + + const [param, key, ...rest] = code.split('.') + const name = rest.join('.') + + if (['colors', 'shadows'].includes(key) && Object.keys(deprecations).includes(name)) { + replaceDeprecatedColor( + context, + path.node, + name, + str => [param, key, str].join('.'), + str => str + ) + } + + // Don't traverse any nested member expressions. + // The root-level member expression gives us all the data we need. + return traverse.SKIP + } + }) + } }) } @@ -103,14 +131,20 @@ function isGet(identifier, scope) { return isImportedFrom(/^\.\.?\/constants$/, identifier, scope) && identifier.name === 'get' } -function replaceDeprecatedColor(context, node, deprecatedName, getDisplayName = str => str) { +function replaceDeprecatedColor( + context, + node, + deprecatedName, + transformName = str => str, + transformReplacementValue = str => JSON.stringify(str) +) { const replacement = deprecations[deprecatedName] if (replacement === null) { // No replacement context.report({ node, - message: `"${getDisplayName( + message: `"${transformName( deprecatedName )}" is deprecated. Go to https://primer.style/primitives or reach out in the #primer channel on Slack to find a suitable replacement.` }) @@ -118,11 +152,11 @@ function replaceDeprecatedColor(context, node, deprecatedName, getDisplayName = // Multiple possible replacements context.report({ node, - message: `"${getDisplayName(deprecatedName)}" is deprecated.`, + message: `"${transformName(deprecatedName)}" is deprecated.`, suggest: replacement.map(replacementValue => ({ - desc: `Use "${getDisplayName(replacementValue)}" instead.`, + desc: `Use "${transformName(replacementValue)}" instead.`, fix(fixer) { - return fixer.replaceText(node, JSON.stringify(getDisplayName(replacementValue))) + return fixer.replaceText(node, transformReplacementValue(transformName(replacementValue))) } })) }) @@ -130,9 +164,9 @@ function replaceDeprecatedColor(context, node, deprecatedName, getDisplayName = // One replacement context.report({ node, - message: `"${getDisplayName(deprecatedName)}" is deprecated. Use "${getDisplayName(replacement)}" instead.`, + message: `"${transformName(deprecatedName)}" is deprecated. Use "${transformName(replacement)}" instead.`, fix(fixer) { - return fixer.replaceText(node, JSON.stringify(getDisplayName(replacement))) + return fixer.replaceText(node, transformReplacementValue(transformName(replacement))) } }) } From dd14594b05e4d800baa76771f5b911d77352a983 Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Fri, 17 Sep 2021 19:42:03 -0700 Subject: [PATCH 3/3] Create weak-tips-brush.md --- .changeset/weak-tips-brush.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .changeset/weak-tips-brush.md diff --git a/.changeset/weak-tips-brush.md b/.changeset/weak-tips-brush.md new file mode 100644 index 00000000..8f37206b --- /dev/null +++ b/.changeset/weak-tips-brush.md @@ -0,0 +1,17 @@ +--- +"eslint-plugin-primer-react": patch +--- + +The `no-deprecated-colors` rule can now find deprecated colors in the following cases: + +* Nested `sx` properties: + + ```jsx + + ``` + +* Functions in `sx` prop: + + ```jsx + `0 1px 2px ${theme.colors.text.primary}`}}> + ```