Skip to content

Commit 5f920de

Browse files
Copilotjoshblack
andcommitted
Add autofix support for conditional and dynamic icon usage
Co-authored-by: joshblack <[email protected]>
1 parent d35c34c commit 5f920de

File tree

2 files changed

+144
-12
lines changed

2 files changed

+144
-12
lines changed

src/rules/__tests__/no-deprecated-octicon.test.js

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,29 +155,75 @@ export default function App() {
155155
],
156156
},
157157

158-
// Complex conditional case - should report but not autofix
158+
// Complex conditional case - now provides autofix
159159
{
160160
code: `import {Octicon} from '@primer/react/deprecated'
161161
import {XIcon, CheckIcon} from '@primer/octicons-react'
162162
export default function App() {
163163
return <Octicon icon={condition ? XIcon : CheckIcon} />
164164
}`,
165-
output: null,
165+
output: `import {Octicon} from '@primer/react/deprecated'
166+
import {XIcon, CheckIcon} from '@primer/octicons-react'
167+
export default function App() {
168+
return condition ? <XIcon /> : <CheckIcon />
169+
}`,
170+
errors: [
171+
{
172+
messageId: 'replaceDeprecatedOcticon',
173+
},
174+
],
175+
},
176+
177+
// Complex conditional case with props - applies props to both components
178+
{
179+
code: `import {Octicon} from '@primer/react/deprecated'
180+
import {XIcon, CheckIcon} from '@primer/octicons-react'
181+
export default function App() {
182+
return <Octicon icon={condition ? XIcon : CheckIcon} size={16} className="test" />
183+
}`,
184+
output: `import {Octicon} from '@primer/react/deprecated'
185+
import {XIcon, CheckIcon} from '@primer/octicons-react'
186+
export default function App() {
187+
return condition ? <XIcon size={16} className="test" /> : <CheckIcon size={16} className="test" />
188+
}`,
166189
errors: [
167190
{
168191
messageId: 'replaceDeprecatedOcticon',
169192
},
170193
],
171194
},
172195

173-
// Dynamic icon access - should report but not autofix
196+
// Dynamic icon access - now provides autofix
174197
{
175198
code: `import {Octicon} from '@primer/react/deprecated'
176199
export default function App() {
177200
const icons = { x: XIcon }
178201
return <Octicon icon={icons.x} />
179202
}`,
180-
output: null,
203+
output: `import {Octicon} from '@primer/react/deprecated'
204+
export default function App() {
205+
const icons = { x: XIcon }
206+
return React.createElement(icons.x, {})
207+
}`,
208+
errors: [
209+
{
210+
messageId: 'replaceDeprecatedOcticon',
211+
},
212+
],
213+
},
214+
215+
// Dynamic icon access with props
216+
{
217+
code: `import {Octicon} from '@primer/react/deprecated'
218+
export default function App() {
219+
const icons = { x: XIcon }
220+
return <Octicon icon={icons.x} size={16} className="btn-icon" />
221+
}`,
222+
output: `import {Octicon} from '@primer/react/deprecated'
223+
export default function App() {
224+
const icons = { x: XIcon }
225+
return React.createElement(icons.x, {size: 16, className: "btn-icon"})
226+
}`,
181227
errors: [
182228
{
183229
messageId: 'replaceDeprecatedOcticon',

src/rules/no-deprecated-octicon.js

Lines changed: 94 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,12 @@ module.exports = {
4242
}
4343

4444
let iconName = null
45-
let isDynamic = false
45+
let isConditional = false
46+
let isMemberExpression = false
47+
let conditionalExpression = null
48+
let memberExpression = null
4649

47-
// Analyze the icon prop to determine the icon name
50+
// Analyze the icon prop to determine the icon name and type
4851
if (iconProp.value?.type === 'JSXExpressionContainer') {
4952
const expression = iconProp.value.expression
5053

@@ -53,20 +56,25 @@ module.exports = {
5356
iconName = expression.name
5457
} else if (expression.type === 'ConditionalExpression') {
5558
// Conditional case: icon={condition ? XIcon : YIcon}
56-
// For now, we'll skip auto-fixing complex conditionals
57-
isDynamic = true
59+
isConditional = true
60+
conditionalExpression = expression
5861
} else if (expression.type === 'MemberExpression') {
5962
// Dynamic lookup: icon={icons.x}
60-
isDynamic = true
63+
isMemberExpression = true
64+
memberExpression = expression
6165
}
6266
}
6367

64-
if (!iconName && !isDynamic) {
68+
if (!iconName && !isConditional && !isMemberExpression) {
6569
return
6670
}
6771

72+
// Get all props except the icon prop to preserve them
73+
const otherProps = openingElement.attributes.filter(attr => attr !== iconProp)
74+
const propsText = otherProps.map(attr => sourceCode.getText(attr)).join(' ')
75+
6876
// For simple cases, we can provide an autofix
69-
if (iconName && !isDynamic) {
77+
if (iconName) {
7078
context.report({
7179
node: openingElement,
7280
messageId: 'replaceDeprecatedOcticon',
@@ -111,8 +119,86 @@ module.exports = {
111119
}
112120
},
113121
})
122+
} else if (isConditional) {
123+
// Handle conditional expressions: icon={condition ? XIcon : YIcon}
124+
// Transform to: condition ? <XIcon otherProps /> : <YIcon otherProps />
125+
context.report({
126+
node: openingElement,
127+
messageId: 'replaceDeprecatedOcticon',
128+
*fix(fixer) {
129+
const test = sourceCode.getText(conditionalExpression.test)
130+
const consequentName = conditionalExpression.consequent.type === 'Identifier'
131+
? conditionalExpression.consequent.name
132+
: sourceCode.getText(conditionalExpression.consequent)
133+
const alternateName = conditionalExpression.alternate.type === 'Identifier'
134+
? conditionalExpression.alternate.name
135+
: sourceCode.getText(conditionalExpression.alternate)
136+
137+
const propsString = propsText ? ` ${propsText}` : ''
138+
let replacement = `${test} ? <${consequentName}${propsString} /> : <${alternateName}${propsString} />`
139+
140+
// If it has children, we need to include them in both branches
141+
if (node.children && node.children.length > 0) {
142+
const childrenText = node.children.map(child => sourceCode.getText(child)).join('')
143+
replacement = `${test} ? <${consequentName}${propsString}>${childrenText}</${consequentName}> : <${alternateName}${propsString}>${childrenText}</${alternateName}>`
144+
}
145+
146+
yield fixer.replaceText(node, replacement)
147+
},
148+
})
149+
} else if (isMemberExpression) {
150+
// Handle member expressions: icon={icons.x}
151+
// Transform to: React.createElement(icons.x, otherProps)
152+
context.report({
153+
node: openingElement,
154+
messageId: 'replaceDeprecatedOcticon',
155+
*fix(fixer) {
156+
const memberText = sourceCode.getText(memberExpression)
157+
158+
// Build props object
159+
let propsObject = '{}'
160+
if (otherProps.length > 0) {
161+
const propStrings = otherProps.map(attr => {
162+
if (attr.type === 'JSXSpreadAttribute') {
163+
return `...${sourceCode.getText(attr.argument)}`
164+
} else {
165+
const name = attr.name.name
166+
const value = attr.value
167+
if (!value) {
168+
return `${name}: true`
169+
} else if (value.type === 'Literal') {
170+
return `${name}: ${JSON.stringify(value.value)}`
171+
} else if (value.type === 'JSXExpressionContainer') {
172+
return `${name}: ${sourceCode.getText(value.expression)}`
173+
}
174+
return `${name}: ${sourceCode.getText(value)}`
175+
}
176+
})
177+
propsObject = `{${propStrings.join(', ')}}`
178+
}
179+
180+
let replacement = `React.createElement(${memberText}, ${propsObject})`
181+
182+
// If it has children, include them as additional arguments
183+
if (node.children && node.children.length > 0) {
184+
const childrenArgs = node.children.map(child => {
185+
if (child.type === 'JSXText') {
186+
return JSON.stringify(child.value.trim()).replace(/\n\s*/g, ' ')
187+
} else {
188+
return sourceCode.getText(child)
189+
}
190+
}).filter(child => child !== '""') // Filter out empty text nodes
191+
192+
if (childrenArgs.length > 0) {
193+
replacement = `React.createElement(${memberText}, ${propsObject}, ${childrenArgs.join(', ')})`
194+
}
195+
}
196+
197+
yield fixer.replaceText(node, replacement)
198+
},
199+
})
114200
} else {
115-
// For complex cases, just report without autofix
201+
// For other complex cases, just report without autofix
116202
context.report({
117203
node: openingElement,
118204
messageId: 'replaceDeprecatedOcticon',

0 commit comments

Comments
 (0)