Skip to content

Commit f35e97a

Browse files
jfuchsjfuchs
committed
Fixes and comments
1 parent 557e6f2 commit f35e97a

File tree

4 files changed

+81
-32
lines changed

4 files changed

+81
-32
lines changed

‎package-lock.json

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
"dependencies": {
3737
"@styled-system/props": "^5.1.5",
3838
"eslint-traverse": "^1.0.0",
39+
"lodash": "^4.17.21",
3940
"styled-system": "^5.1.5"
4041
}
4142
}

‎src/rules/__tests__/no-system-props.test.js

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ ruleTester.run('no-system-props', rule, {
1717
`import {Button} from 'coles-cool-design-system'; <Button width={200} />`,
1818
`import {Button} from '@primer/components'; <Button someOtherProp="foo" />`,
1919
`import {Box} from '@primer/components'; <Box width={200} />`,
20-
`import {ProgressBar} from '@primer/components'; <ProgressBar bg="howdy" />`
20+
`import {ProgressBar} from '@primer/components'; <ProgressBar bg="howdy" />`,
21+
`import {Button} from '@primer/components'; <Button {...someExpression()} />`
2122
],
2223
invalid: [
2324
{
@@ -26,7 +27,7 @@ ruleTester.run('no-system-props', rule, {
2627
errors: [
2728
{
2829
messageId: 'noSystemProps',
29-
data: {propNames: 'width'}
30+
data: {propNames: 'width', componentName: 'Button'}
3031
}
3132
]
3233
},
@@ -36,7 +37,7 @@ ruleTester.run('no-system-props', rule, {
3637
errors: [
3738
{
3839
messageId: 'noSystemProps',
39-
data: {propNames: 'width'}
40+
data: {propNames: 'width', componentName: 'Button'}
4041
}
4142
]
4243
},
@@ -46,7 +47,7 @@ ruleTester.run('no-system-props', rule, {
4647
errors: [
4748
{
4849
messageId: 'noSystemProps',
49-
data: {propNames: 'width'}
50+
data: {propNames: 'width', componentName: 'Button'}
5051
}
5152
]
5253
},
@@ -56,27 +57,27 @@ ruleTester.run('no-system-props', rule, {
5657
errors: [
5758
{
5859
messageId: 'noSystemProps',
59-
data: {propNames: 'width'}
60+
data: {propNames: 'width', componentName: 'Button'}
6061
}
6162
]
6263
},
6364
{
64-
code: `import {Button} from '@primer/components'; <Button width={200} height={200} />`,
65-
output: `import {Button} from '@primer/components'; <Button sx={{width: 200, height: 200}} />`,
65+
code: `import {Button} from '@primer/components'; <Button width={200} height={100} />`,
66+
output: `import {Button} from '@primer/components'; <Button sx={{width: 200, height: 100}} />`,
6667
errors: [
6768
{
6869
messageId: 'noSystemProps',
69-
data: {propNames: 'width, height'}
70+
data: {propNames: 'width, height', componentName: 'Button'}
7071
}
7172
]
7273
},
7374
{
7475
code: `import {Button} from '@primer/components'; <Button width={200} sx={{height: 200}} />`,
75-
output: `import {Button} from '@primer/components'; <Button sx={{width: 200, height: 200}} />`,
76+
output: `import {Button} from '@primer/components'; <Button sx={{height: 200, width: 200}} />`,
7677
errors: [
7778
{
7879
messageId: 'noSystemProps',
79-
data: {propNames: 'width'}
80+
data: {propNames: 'width', componentName: 'Button'}
8081
}
8182
]
8283
},
@@ -86,7 +87,7 @@ ruleTester.run('no-system-props', rule, {
8687
errors: [
8788
{
8889
messageId: 'noSystemProps',
89-
data: {propNames: 'width'}
90+
data: {propNames: 'width', componentName: 'Button'}
9091
}
9192
]
9293
},
@@ -96,7 +97,17 @@ ruleTester.run('no-system-props', rule, {
9697
errors: [
9798
{
9899
messageId: 'noSystemProps',
99-
data: {propNames: 'width'}
100+
data: {propNames: 'width', componentName: 'Button'}
101+
}
102+
]
103+
},
104+
{
105+
code: `import {Button} from '@primer/components'; <Button width={200} sx={{...partialStyles, width: 100}} />`,
106+
output: `import {Button} from '@primer/components'; <Button sx={{...partialStyles, width: 100}} />`,
107+
errors: [
108+
{
109+
messageId: 'noSystemProps',
110+
data: {propNames: 'width', componentName: 'Button'}
100111
}
101112
]
102113
}

‎src/rules/no-system-props.js

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const {isPrimerComponent} = require('../utils/isPrimerComponent')
22
const {pick} = require('@styled-system/props')
3+
const {some, last} = require('lodash')
34

45
// Components for which we allow all styled system props
56
const excludedComponents = new Set(['Box', 'Text'])
@@ -13,7 +14,7 @@ module.exports = {
1314
fixable: 'code',
1415
schema: [],
1516
messages: {
16-
noSystemProps: 'Styled-system props are deprecated ({{ propNames }})'
17+
noSystemProps: 'Styled-system props are deprecated ({{componentName}} called with props: {{ propNames }})'
1718
}
1819
},
1920
create(context) {
@@ -22,13 +23,20 @@ module.exports = {
2223
if (!isPrimerComponent(jsxNode.name, context.getScope(jsxNode))) return
2324
if (excludedComponents.has(jsxNode.name.name)) return
2425

25-
const propsByNameMap = jsxNode.attributes.reduce((prev, cur) => {
26-
prev[cur.name.name] = cur
27-
return prev
26+
// Create an object mapping from prop name to the AST node for that attribute
27+
const propsByNameObject = jsxNode.attributes.reduce((object, cur) => {
28+
// We don't do anything about spreads for now — only named attributes:
29+
if (cur.type === 'JSXAttribute') {
30+
object[cur.name.name] = cur
31+
}
32+
33+
return object
2834
}, {})
2935

30-
let badProps = Object.values(pick(propsByNameMap))
36+
// Create an array of bad prop attribute nodes
37+
let badProps = Object.values(pick(propsByNameObject))
3138

39+
// Filter out our exceptional props
3240
badProps = badProps.filter(prop => {
3341
const excludedProps = excludedComponentProps.get(jsxNode.name.name)
3442
if (!excludedProps) {
@@ -42,25 +50,37 @@ module.exports = {
4250
node: jsxNode,
4351
messageId: 'noSystemProps',
4452
data: {
53+
componentName: jsxNode.name.name,
4554
propNames: badProps.map(a => a.name.name).join(', ')
4655
},
4756
fix(fixer) {
48-
const existingSxProp = jsxNode.attributes.find(attribute => attribute.name.name === 'sx')
57+
const existingSxProp = jsxNode.attributes.find(
58+
attribute => attribute.type === 'JSXAttribute' && attribute.name.name === 'sx'
59+
)
4960
const badPropStylesMap = stylesMapFromPropNodes(badProps, context)
5061
if (existingSxProp && existingSxProp.value.expression.type !== 'ObjectExpression') {
5162
return
5263
}
64+
65+
const stylesToAdd = existingSxProp
66+
? excludeSxEntriesFromStyleMap(badPropStylesMap, existingSxProp)
67+
: badPropStylesMap
68+
5369
return [
70+
// Remove the bad props:
5471
...badProps.map(node => fixer.remove(node)),
55-
existingSxProp
56-
? fixer.replaceText(
57-
existingSxProp,
58-
sxPropTextFromStylesMap(new Map([...badPropStylesMap, ...stylesMapfromSxProp(existingSxProp)]))
59-
)
60-
: fixer.insertTextAfter(
61-
jsxNode.attributes[jsxNode.attributes.length - 1],
62-
sxPropTextFromStylesMap(badPropStylesMap)
63-
)
72+
...(stylesToAdd.size > 0
73+
? [
74+
existingSxProp
75+
? // Update an existing sx prop:
76+
fixer.insertTextAfter(
77+
last(existingSxProp.value.expression.properties),
78+
`, ${objectEntriesStringFromStylesMap(stylesToAdd)}`
79+
)
80+
: // Insert new sx prop
81+
fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(badPropStylesMap))
82+
]
83+
: [])
6484
]
6585
}
6686
})
@@ -71,13 +91,31 @@ module.exports = {
7191
}
7292

7393
const sxPropTextFromStylesMap = styles => {
74-
return `sx={{${[...styles].map(([name, value]) => `${name}: ${value}`).join(', ')}}}`
94+
return `sx={{${objectEntriesStringFromStylesMap(styles)}}}`
7595
}
7696

77-
const stylesMapfromSxProp = sxProp => {
78-
return new Map(sxProp.value.expression.properties.map(prop => [prop.key.name, prop.value.raw]))
97+
const objectEntriesStringFromStylesMap = styles => {
98+
return [...styles].map(([name, value]) => `${name}: ${value}`).join(', ')
7999
}
80100

101+
// Given an array of styled prop attributes, return a mapping from attribute to expression
81102
const stylesMapFromPropNodes = (badProps, context) => {
82103
return new Map(badProps.map(a => [a.name.name, a.value.raw || context.getSourceCode().getText(a.value.expression)]))
83104
}
105+
106+
// Given a style map and an existing sx prop, return a style map containing
107+
// only the entries that aren't already overridden by an sx object entry
108+
const excludeSxEntriesFromStyleMap = (stylesMap, sxProp) => {
109+
if (
110+
!sxProp.value ||
111+
sxProp.value.type !== 'JSXExpressionContainer' ||
112+
sxProp.value.expression.type != 'ObjectExpression'
113+
) {
114+
return stylesMap
115+
}
116+
return new Map(
117+
[...stylesMap].filter(([key, _value]) => {
118+
return !some(sxProp.value.expression.properties, p => p.type === 'Property' && p.key.name === key)
119+
})
120+
)
121+
}

0 commit comments

Comments
 (0)