From 2f2ad31b2618525e54937a265477a9ffc0b7019f Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Fri, 9 Aug 2024 12:02:17 -0400 Subject: [PATCH 1/4] feat(Button): Extend icon moving to support Icon and react-icon children --- .../src/rules/helpers/index.ts | 1 + .../src/rules/helpers/isReactIcon.ts | 31 ++++++++++++++ .../button-moveIcons-icon-prop.test.ts | 22 ++++++++++ .../button-moveIcons-icon-prop.ts | 42 ++++++++++++++----- .../buttonMoveIconsIconPropInput.tsx | 17 ++++++-- .../buttonMoveIconsIconPropOutput.tsx | 9 +++- 6 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 packages/eslint-plugin-pf-codemods/src/rules/helpers/isReactIcon.ts diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts index 88173d615..2f38f8ec4 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/index.ts @@ -8,6 +8,7 @@ export * from "./helpers"; export * from "./importAndExport"; export * from "./includesImport"; export * from "./interfaces"; +export * from "./isReactIcon"; export * from "./JSXAttributes"; export * from "./JSXElements"; export * from "./nodeMatches"; diff --git a/packages/eslint-plugin-pf-codemods/src/rules/helpers/isReactIcon.ts b/packages/eslint-plugin-pf-codemods/src/rules/helpers/isReactIcon.ts new file mode 100644 index 000000000..66e53e789 --- /dev/null +++ b/packages/eslint-plugin-pf-codemods/src/rules/helpers/isReactIcon.ts @@ -0,0 +1,31 @@ +import { Rule } from "eslint"; +import { JSXElement } from "estree-jsx"; +import { getFromPackage, getDefaultImportsFromPackage } from "./index"; + +/** Returns true if an element is a patternfly/react-icons import, false if it isn't, and undefined + * if no element is passed (for type safety) */ +export function isReactIcon(context: Rule.RuleContext, element?: JSXElement) { + if (!element) { + return; + } + + const openingElementIdentifier = element.openingElement.name; + + // TODO: update this to use the appropriate getNodeName helper once it lands + if (openingElementIdentifier.type !== "JSXIdentifier") { + return; + } + const elementName = openingElementIdentifier.name; + + const pfIconsPackage = "@patternfly/react-icons"; + const { imports: iconImports } = getFromPackage(context, pfIconsPackage); + const iconDefaultImports = getDefaultImportsFromPackage( + context, + pfIconsPackage + ); + const allIconImports = [...iconImports, ...iconDefaultImports]; + + return allIconImports.some( + (iconImport) => iconImport.local.name === elementName + ); +} diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts index 56557f4d8..2ff1480eb 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts @@ -81,5 +81,27 @@ ruleTester.run("button-moveIcons-icon-prop", rule, { }, ], }, + // with Icon component child + { + code: `import { Button, Icon } from '@patternfly/react-core'; `, + output: `import { Button, Icon } from '@patternfly/react-core'; `, + errors: [ + { + message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`, + type: "JSXElement", + }, + ], + }, + // with react-icons icon child + { + code: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; `, + output: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; `, + errors: [ + { + message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`, + type: "JSXElement", + }, + ], + }, ], }); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts index c5fdd20cd..e6bec11e8 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts @@ -6,12 +6,15 @@ import { getAttributeValue, getExpression, getChildrenAsAttributeValueText, + getChildElementByName, + isReactIcon, } from "../../helpers"; // https://github.com/patternfly/patternfly-react/pull/10663 module.exports = { meta: { fixable: "code" }, create: function (context: Rule.RuleContext) { + const source = context.getSourceCode(); const { imports } = getFromPackage(context, "@patternfly/react-core"); const buttonImport = imports.find( @@ -31,12 +34,12 @@ module.exports = { ) { const variantProp = getAttribute(node.openingElement, "variant"); const iconProp = getAttribute(node.openingElement, "icon"); - if (!variantProp || iconProp) { + if (iconProp) { return; } const variantValue = getAttributeValue( context, - variantProp.value + variantProp?.value ); const isEnumValuePlain = @@ -44,28 +47,47 @@ module.exports = { variantValue?.object?.name === buttonVariantEnumImport.local.name && variantValue?.property.name === "plain"; - if (variantValue !== "plain" && !isEnumValuePlain) { - return; - } + + const isPlain = variantValue === "plain" || isEnumValuePlain; + const childrenProp = getAttribute(node, "children"); - let childrenValue; + + let childrenValue: string | undefined; if (childrenProp) { const childrenPropExpression = getExpression( childrenProp?.value ); childrenValue = childrenPropExpression - ? context.getSourceCode().getText(childrenPropExpression) + ? source.getText(childrenPropExpression) : ""; - } else { + } else if (isPlain) { childrenValue = getChildrenAsAttributeValueText( context, node.children ); } - if (!childrenValue) { + + if (!childrenValue && isPlain) { + return; + } + + const iconComponentChild = getChildElementByName(node, "Icon"); + + const jsxElementChildren = node.children.filter( + (child) => child.type === "JSXElement" + ) as JSXElement[]; + const reactIconChild = jsxElementChildren.find((child) => + isReactIcon(context, child) + ); + + const iconChild = iconComponentChild || reactIconChild; + + if (!isPlain && !iconChild) { return; } + const iconChildString = `{${source.getText(iconChild)}}`; + context.report({ node, message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`, @@ -74,7 +96,7 @@ module.exports = { fixes.push( fixer.insertTextAfter( node.openingElement.name, - ` icon=${childrenValue}` + ` icon=${childrenValue || iconChildString}` ) ); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropInput.tsx b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropInput.tsx index d0e84331c..0556e3e98 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropInput.tsx +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropInput.tsx @@ -1,7 +1,18 @@ import { Button } from "@patternfly/react-core"; +import { SomeIcon } from "@patternfly/react-icons"; export const ButtonMoveIconsIconPropInput = () => ( - + <> + + + + ); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropOutput.tsx b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropOutput.tsx index 298b7fa59..1bd4350f7 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropOutput.tsx +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/buttonMoveIconsIconPropOutput.tsx @@ -1,5 +1,12 @@ import { Button } from "@patternfly/react-core"; +import { SomeIcon } from "@patternfly/react-icons"; export const ButtonMoveIconsIconPropInput = () => ( - + <> + + + + ); From 3b28577c0d16d029bc0b71667eb4c605be92053a Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Thu, 22 Aug 2024 14:51:55 -0400 Subject: [PATCH 2/4] fix(Button): fix bugs found during review --- .../button-moveIcons-icon-prop.test.ts | 26 +++++++++++++++++++ .../button-moveIcons-icon-prop.ts | 12 ++++++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts index 2ff1480eb..957aeee01 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.test.ts @@ -9,6 +9,10 @@ ruleTester.run("button-moveIcons-icon-prop", rule, { { code: `import { Button } from '@patternfly/react-core'; `, + }, ], invalid: [ { @@ -103,5 +107,27 @@ ruleTester.run("button-moveIcons-icon-prop", rule, { }, ], }, + // with react-icons icon child and another child + { + code: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; `, + output: `import { Button } from '@patternfly/react-core'; import { SomeIcon } from "@patternfly/react-icons"; `, + errors: [ + { + message: `Icons must now be passed to the \`icon\` prop of Button instead of as children. If you are passing anything other than an icon as children, ignore this rule when running fixes.`, + type: "JSXElement", + }, + ], + }, + // with children prop + { + code: `import { Button } from '@patternfly/react-core'; - + }> + + + ); From 834c357ad6cbcf91a9363c177cbf075dca679194 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Mon, 26 Aug 2024 10:14:17 -0400 Subject: [PATCH 4/4] chore(helpers): Update helper import after merge --- .../v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts index 38e26916b..28e493141 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/buttonMoveIconsIconProp/button-moveIcons-icon-prop.ts @@ -6,7 +6,7 @@ import { getAttributeValue, getExpression, getChildrenAsAttributeValueText, - getChildElementByName, + getChildJSXElementByName, isReactIcon, } from "../../helpers"; @@ -75,7 +75,7 @@ module.exports = { } const iconComponentChild = - hasIconImport && getChildElementByName(node, "Icon"); + hasIconImport && getChildJSXElementByName(node, "Icon"); const jsxElementChildren = node.children.filter( (child) => child.type === "JSXElement"