From 933e273f623d4fa5ee64809215ba1a9a977ab0cc Mon Sep 17 00:00:00 2001 From: adamviktora Date: Wed, 18 Sep 2024 15:11:49 +0200 Subject: [PATCH 1/3] feat(EmptyState): make titleText optional --- .../emptyStateHeader-move-into-emptyState.md | 3 - ...tyStateHeader-move-into-emptyState.test.ts | 50 +++++---------- .../emptyStateHeader-move-into-emptyState.ts | 61 +++++-------------- ...mptyStateHeaderMoveIntoEmptyStateInput.tsx | 17 ++++++ ...ptyStateHeaderMoveIntoEmptyStateOutput.tsx | 12 ++++ 5 files changed, 58 insertions(+), 85 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.md b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.md index 95d61eae7..152574d9c 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.md +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.md @@ -2,8 +2,6 @@ EmptyStateHeader and EmptyStateIcon are now rendered internally within EmptyState and should only be customized using props. Content passed to the `icon` prop on EmptyState will also be wrapped by EmptyStateIcon automatically. -Additionally, the `titleText` prop is now required on EmptyState. - #### Examples In: @@ -17,4 +15,3 @@ Out: ```jsx %outputExample% ``` - diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.test.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.test.ts index 278e05ac1..5f18c4910 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.test.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.test.ts @@ -15,6 +15,10 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { { code: `import { EmptyState } from '@patternfly/react-core'; `, }, + { + // without an EmptyStateHeader or Title text + code: `import { EmptyState } from "@patternfly/react-core"; Foo bar`, + }, ], invalid: [ { @@ -106,14 +110,13 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { } from "@patternfly/react-core"; export const EmptyStateHeaderMoveIntoEmptyStateInput = () => ( - - - + + ); `, errors: [ { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. You must manually supply a titleText prop to EmptyState, then you can rerun this codemod.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`, type: "JSXElement", }, ], @@ -143,7 +146,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { `, errors: [ { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`, type: "JSXElement", }, ], @@ -175,7 +178,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { `, errors: [ { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`, type: "JSXElement", }, ], @@ -211,7 +214,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { `, errors: [ { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`, type: "JSXElement", }, ], @@ -249,32 +252,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { `, errors: [ { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`, - type: "JSXElement", - }, - ], - }, - { - // without an EmptyStateHeader or titleText - code: `import { EmptyState } from "@patternfly/react-core"; - - export const EmptyStateHeaderMoveIntoEmptyStateInput = () => ( - - Foo bar - - ); - `, - output: `import { EmptyState } from "@patternfly/react-core"; - - export const EmptyStateHeaderMoveIntoEmptyStateInput = () => ( - - Foo bar - - ); - `, - errors: [ - { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. You must manually supply a titleText prop to EmptyState`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`, type: "JSXElement", }, ], @@ -489,7 +467,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { `, errors: [ { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`, type: "JSXElement", }, ], @@ -540,7 +518,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { type: "JSXElement", }, { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`, type: "JSXElement", }, ], @@ -587,7 +565,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, { type: "JSXElement", }, { - message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`, + message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`, type: "JSXElement", }, ], diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts index 67fe9e1ea..f8a48c0fc 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts @@ -17,6 +17,7 @@ import { getFromPackage, getChildrenAsAttributeValueText, getRemoveElementFixes, + childrenIsEmpty, } from "../../helpers"; // https://github.com/patternfly/patternfly-react/pull/9947 @@ -26,29 +27,11 @@ const composeMessage = ( hasChildren?: boolean ) => { let message = - "EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props"; - const missingTitleTextMessage = - ", and the titleText prop is now required on EmptyState."; - - if (hasTitleText) { - message += "."; - } else { - message += missingTitleTextMessage; - } + "EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props."; if (hasTitleText && hasChildren) { message += - " Because the children for EmptyStateHeader are now inaccessible you must remove either the children or the titleText prop"; - } else if (!hasTitleText && !hasChildren) { - message += " You must manually supply a titleText prop to EmptyState"; - } - - const hasHeader = [hasTitleText, hasIcon, hasChildren].some( - (arg) => typeof arg !== "undefined" - ); - - if (hasTitleText === hasChildren && hasHeader) { - message += ", then you can rerun this codemod."; + " Because the children for EmptyStateHeader are now inaccessible you must remove either the children or the titleText prop, then you can rerun this codemod."; } if (hasIcon) { @@ -175,19 +158,6 @@ module.exports = { const titleChild = getChildJSXElementByName(node, "Title"); - if ( - (!header || header.type !== "JSXElement") && - (!titleChild || titleChild.type !== "JSXElement") - ) { - // report without fixer if there is no header/title or the header/title is not a React element, because - // creating a titleText for the EmptyState in this case is difficult - context.report({ - node, - message: composeMessage(), - }); - return; - } - const newEmptyStateProps: string[] = []; const removeElements: JSXElement[] = []; @@ -204,6 +174,10 @@ module.exports = { "EmptyStateIcon" ); + if (!header && !titleChild && !emptyStateIconChild) { + return; + } + let iconProp: string = ""; if (emptyStateIconChild) { @@ -231,19 +205,12 @@ module.exports = { hasTitleText = !!titleTextAttribute; hasIcon ||= !!headerIconAttribute; - hasChildren ||= header.children.length > 0; + hasChildren ||= !childrenIsEmpty(header.children); const message = composeMessage(hasTitleText, hasIcon, hasChildren); - if (!titleTextAttribute && !hasChildren) { - // report without fixer if there is a header, but it doesn't have titleText or children, because creating a - // titleText for the EmptyState in this case is difficult - context.report({ node, message }); - return; - } - if (titleTextAttribute && hasChildren) { - // report without fixer if there is the header has a titleText and children, because creating an accessible + // report without fixer if the header has both titleText and children, because creating an accessible // titleText for the EmptyState in this case is difficult context.report({ node, message }); return; @@ -269,10 +236,12 @@ module.exports = { const titleText = titleTextPropValue || - `titleText=${getChildrenAsAttributeValueText( - context, - header.children - )}`; + (hasChildren + ? `titleText=${getChildrenAsAttributeValueText( + context, + header.children + )}` + : ""); const iconPropValue = getExpression(headerIconAttribute?.value); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateInput.tsx b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateInput.tsx index 04605a858..ef8915e31 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateInput.tsx +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateInput.tsx @@ -26,3 +26,20 @@ export const EmptyStateWithoutHeaderMoveIntoEmptyStateInput = () => ( Body ); + +export const EmptyStateHeaderWithoutTitleTextMoveIntoEmptyStateInput = () => ( + + } + /> + +); + +export const EmptyStateWithoutHeaderAndTitleTextMoveIntoEmptyStateInput = + () => ( + + + Body + + ); diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateOutput.tsx b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateOutput.tsx index 1d4f3f451..1902bb642 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateOutput.tsx +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateOutput.tsx @@ -19,3 +19,15 @@ export const EmptyStateWithoutHeaderMoveIntoEmptyStateInput = () => ( Body ); + +export const EmptyStateHeaderWithoutTitleTextMoveIntoEmptyStateInput = () => ( + + +); + +export const EmptyStateWithoutHeaderAndTitleTextMoveIntoEmptyStateInput = + () => ( + + Body + + ); From 92e0d86e666afafe4384fae33b6a9d7383dbf37d Mon Sep 17 00:00:00 2001 From: adamviktora Date: Wed, 25 Sep 2024 12:32:31 +0200 Subject: [PATCH 2/3] refactor (PR review) --- .../emptyStateHeader-move-into-emptyState.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts index f8a48c0fc..dccbe1407 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts @@ -234,14 +234,16 @@ module.exports = { titleTextAttribute ); - const titleText = - titleTextPropValue || - (hasChildren + const createTitleTextPropFromChildren = () => + hasChildren ? `titleText=${getChildrenAsAttributeValueText( context, header.children )}` - : ""); + : ""; + + const titleText = + titleTextPropValue || createTitleTextPropFromChildren(); const iconPropValue = getExpression(headerIconAttribute?.value); From f8a4c1a3204754d71875eb9354fa6d3f57be6b52 Mon Sep 17 00:00:00 2001 From: adamviktora Date: Wed, 25 Sep 2024 17:01:10 +0200 Subject: [PATCH 3/3] refactor no.2 --- .../emptyStateHeader-move-into-emptyState.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts index dccbe1407..de091b73f 100644 --- a/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts +++ b/packages/eslint-plugin-pf-codemods/src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts @@ -234,16 +234,14 @@ module.exports = { titleTextAttribute ); - const createTitleTextPropFromChildren = () => - hasChildren - ? `titleText=${getChildrenAsAttributeValueText( - context, - header.children - )}` - : ""; - - const titleText = - titleTextPropValue || createTitleTextPropFromChildren(); + const childrenTitleText = hasChildren + ? `titleText=${getChildrenAsAttributeValueText( + context, + header.children + )}` + : ""; + + const titleText = titleTextPropValue || childrenTitleText; const iconPropValue = getExpression(headerIconAttribute?.value);