-
Notifications
You must be signed in to change notification settings - Fork 9
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #81 from primer/new-css-vars
New rule: `new-color-css-vars`
- Loading branch information
Showing
8 changed files
with
3,067 additions
and
530 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"eslint-plugin-primer-react": patch | ||
--- | ||
|
||
New rule: `new-color-css-vars` to find/replace legacy CSS color vars in sx prop |
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
const rule = require('../new-color-css-vars') | ||
const {RuleTester} = require('eslint') | ||
|
||
const ruleTester = new RuleTester({ | ||
parserOptions: { | ||
ecmaVersion: 'latest', | ||
sourceType: 'module', | ||
ecmaFeatures: { | ||
jsx: true | ||
} | ||
} | ||
}) | ||
|
||
ruleTester.run('no-color-css-vars', rule, { | ||
valid: [ | ||
{ | ||
code: `{color: 'fg.default'}` | ||
}, | ||
{ | ||
code: `<circle stroke="var(--color-border-default)" strokeWidth="2" />` | ||
}, | ||
{ | ||
code: `<circle fill="var(--color-border-default)" strokeWidth="2" />` | ||
}, | ||
{ | ||
code: `<div style={{ color: 'var(--color-border-default)' }}></div>` | ||
}, | ||
{ | ||
code: `<Blankslate border></Blankslate>` | ||
} | ||
], | ||
invalid: [ | ||
{ | ||
code: `<Button sx={{color: 'var(--color-fg-muted)'}}>Test</Button>`, | ||
output: `<Button sx={{color: 'var(--fgColor-muted, var(--color-fg-muted))'}}>Test</Button>`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-fg-muted) with var(--fgColor-muted, var(--color-fg-muted))' | ||
} | ||
] | ||
}, | ||
{ | ||
code: ` | ||
<Box sx={{ | ||
'&:hover [data-component="copy-link"] button, &:focus [data-component="copy-link"] button': { | ||
color: 'var(--color-accent-fg)' | ||
} | ||
}}> | ||
</Box>`, | ||
output: ` | ||
<Box sx={{ | ||
'&:hover [data-component="copy-link"] button, &:focus [data-component="copy-link"] button': { | ||
color: 'var(--fgColor-accent, var(--color-accent-fg))' | ||
} | ||
}}> | ||
</Box>`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-accent-fg) with var(--fgColor-accent, var(--color-accent-fg))' | ||
} | ||
] | ||
}, | ||
{ | ||
code: `<Box sx={{boxShadow: '0 0 0 2px var(--color-canvas-subtle)'}} />`, | ||
output: `<Box sx={{boxShadow: '0 0 0 2px var(--bgColor-muted, var(--color-canvas-subtle))'}} />`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-canvas-subtle) with var(--bgColor-muted, var(--color-canvas-subtle))' | ||
} | ||
] | ||
}, | ||
{ | ||
code: `<Box sx={{border: 'solid 2px var(--color-border-default)'}} />`, | ||
output: `<Box sx={{border: 'solid 2px var(--borderColor-default, var(--color-border-default))'}} />`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-border-default) with var(--borderColor-default, var(--color-border-default))' | ||
} | ||
] | ||
}, | ||
{ | ||
code: `<Box sx={{backgroundColor: 'var(--color-canvas-default)'}} />`, | ||
output: `<Box sx={{backgroundColor: 'var(--bgColor-default, var(--color-canvas-default))'}} />`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-canvas-default) with var(--bgColor-default, var(--color-canvas-default))' | ||
} | ||
] | ||
}, | ||
{ | ||
name: 'variable in scope', | ||
code: ` | ||
const baseStyles = { color: 'var(--color-fg-muted)' } | ||
export const Fixture = <Button sx={baseStyles}>Test</Button> | ||
`, | ||
output: ` | ||
const baseStyles = { color: 'var(--fgColor-muted, var(--color-fg-muted))' } | ||
export const Fixture = <Button sx={baseStyles}>Test</Button> | ||
`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-fg-muted) with var(--fgColor-muted, var(--color-fg-muted))' | ||
} | ||
] | ||
}, | ||
{ | ||
name: 'merge in sx', | ||
code: ` | ||
import {merge} from '@primer/react' | ||
export const Fixture = props => <Button sx={merge({color: 'var(--color-fg-muted)'}, props.sx)}>Test</Button> | ||
`, | ||
output: ` | ||
import {merge} from '@primer/react' | ||
export const Fixture = props => <Button sx={merge({color: 'var(--fgColor-muted, var(--color-fg-muted))'}, props.sx)}>Test</Button> | ||
`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-fg-muted) with var(--fgColor-muted, var(--color-fg-muted))' | ||
} | ||
] | ||
}, | ||
{ | ||
code: `<Box sx={{borderColor: 'var(--color-border-default)'}} />`, | ||
output: `<Box sx={{borderColor: 'var(--borderColor-default, var(--color-border-default))'}} />`, | ||
errors: [ | ||
{ | ||
message: 'Replace var(--color-border-default) with var(--borderColor-default, var(--color-border-default))' | ||
} | ||
] | ||
} | ||
] | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
const cssVars = require('../utils/css-variable-map.json') | ||
|
||
module.exports = { | ||
meta: { | ||
type: 'suggestion', | ||
hasSuggestions: true, | ||
fixable: 'code', | ||
docs: { | ||
description: 'Upgrade legacy CSS variables to Primitives v8 in sx prop' | ||
}, | ||
schema: [ | ||
{ | ||
type: 'object', | ||
properties: { | ||
skipImportCheck: { | ||
type: 'boolean' | ||
}, | ||
checkAllStrings: { | ||
type: 'boolean' | ||
} | ||
}, | ||
additionalProperties: false | ||
} | ||
] | ||
}, | ||
/** @param {import('eslint').Rule.RuleContext} context */ | ||
create(context) { | ||
const styledSystemProps = [ | ||
'bg', | ||
'backgroundColor', | ||
'color', | ||
'borderColor', | ||
'borderTopColor', | ||
'borderRightColor', | ||
'borderBottomColor', | ||
'borderLeftColor', | ||
'border', | ||
'boxShadow', | ||
'caretColor' | ||
] | ||
|
||
return { | ||
/** @param {import('eslint').Rule.Node} node */ | ||
JSXAttribute(node) { | ||
if (node.name.name === 'sx') { | ||
if (node.value.expression.type === 'ObjectExpression') { | ||
// example: sx={{ color: 'fg.default' }} or sx={{ ':hover': {color: 'fg.default'} }} | ||
const rawText = context.sourceCode.getText(node.value) | ||
checkForVariables(node.value, rawText) | ||
} else if (node.value.expression.type === 'Identifier') { | ||
// example: sx={baseStyles} | ||
const variableScope = context.sourceCode.getScope(node.value.expression) | ||
const variable = variableScope.set.get(node.value.expression.name) | ||
|
||
// if variable is not defined in scope, give up (could be imported from different file) | ||
if (!variable) return | ||
|
||
const variableDeclarator = variable.identifiers[0].parent | ||
const rawText = context.sourceCode.getText(variableDeclarator) | ||
checkForVariables(variableDeclarator, rawText) | ||
} else { | ||
// worth a try! | ||
const rawText = context.sourceCode.getText(node.value) | ||
checkForVariables(node.value, rawText) | ||
} | ||
} else if ( | ||
styledSystemProps.includes(node.name.name) && | ||
node.value && | ||
node.value.type === 'Literal' && | ||
typeof node.value.value === 'string' | ||
) { | ||
checkForVariables(node.value, node.value.value) | ||
} | ||
} | ||
} | ||
|
||
function checkForVariables(node, rawText) { | ||
// performance optimisation: exit early | ||
if (!rawText.includes('var')) return | ||
|
||
Object.keys(cssVars).forEach(cssVar => { | ||
if (Array.isArray(cssVars[cssVar])) { | ||
cssVars[cssVar].forEach(cssVarObject => { | ||
const regex = new RegExp(`var\\(${cssVar}\\)`, 'g') | ||
if ( | ||
cssVarObject.props.some(prop => rawText.includes(prop)) && | ||
regex.test(rawText) && | ||
!rawText.includes(cssVarObject.replacement) | ||
) { | ||
const fixedString = rawText.replace(regex, `var(${cssVarObject.replacement}, var(${cssVar}))`) | ||
if (!rawText.includes(fixedString)) { | ||
context.report({ | ||
node, | ||
message: `Replace var(${cssVar}) with var(${cssVarObject.replacement}, var(${cssVar}))`, | ||
fix: function(fixer) { | ||
return fixer.replaceText(node, node.type === 'Literal' ? `"${fixedString}"` : fixedString) | ||
} | ||
}) | ||
} | ||
} | ||
}) | ||
} | ||
}) | ||
} | ||
} | ||
} |
Oops, something went wrong.