From 0073358adc97bead34aa930361561d9b61c1f30f Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Mon, 26 Aug 2024 13:49:37 +0000 Subject: [PATCH 1/2] Add new `size` and `weight` allowed props for `Text` component --- .../no-unnecessary-components.test.js | 10 ++++++++ src/rules/no-unnecessary-components.js | 24 +++++++++++-------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/rules/__tests__/no-unnecessary-components.test.js b/src/rules/__tests__/no-unnecessary-components.test.js index 67e327ff..c6dda7d2 100644 --- a/src/rules/__tests__/no-unnecessary-components.test.js +++ b/src/rules/__tests__/no-unnecessary-components.test.js @@ -65,6 +65,16 @@ ruleTester.run('unnecessary-components', rule, { filename, }, ]), + { + name: `Text with weight prop`, + code: `${prcImport}${jsx(`Hello World`)}`, + filename, + }, + { + name: `Text with size prop`, + code: `${prcImport}${jsx(`Hello World`)}`, + filename, + }, ], invalid: Object.entries(components).flatMap(([component, {messageId, replacement}]) => [ { diff --git a/src/rules/no-unnecessary-components.js b/src/rules/no-unnecessary-components.js index c5701d6a..0a152be0 100644 --- a/src/rules/no-unnecessary-components.js +++ b/src/rules/no-unnecessary-components.js @@ -12,11 +12,13 @@ const components = { replacement: 'div', messageId: 'unecessaryBox', message: 'Prefer plain HTML elements over `Box` when not using `sx` for styling.', + allowedProps: ['sx'], // + styled-system props }, Text: { replacement: 'span', messageId: 'unecessarySpan', message: 'Prefer plain HTML elements over `Text` when not using `sx` for styling.', + allowedProps: ['sx', 'size', 'weight'], // + styled-system props }, } @@ -68,19 +70,20 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ const isPrimer = skipImportCheck || isPrimerComponent(name, context.sourceCode.getScope(openingElement)) if (!isPrimer) return - // Validate the attributes and ensure an `sx` prop is present or spreaded in + // Validate the attributes and ensure an allowed prop is present or spreaded in /** @type {typeof attributes[number] | undefined | null} */ let asProp = undefined for (const attribute of attributes) { - // If there is a spread type, check if the type of the spreaded value has an `sx` property + // If there is a spread type, check if the type of the spreaded value has an allowed property if (attribute.type === 'JSXSpreadAttribute') { const services = ESLintUtils.getParserServices(context) const typeChecker = services.program.getTypeChecker() const spreadType = services.getTypeAtLocation(attribute.argument) - if (typeChecker.getPropertyOfType(spreadType, 'sx') !== undefined) return + for (const allowedProp of componentConfig.allowedProps) + if (typeChecker.getPropertyOfType(spreadType, allowedProp) !== undefined) return - // Check if the spread type has a string index signature - this could hide an `sx` property + // Check if the spread type has a string index signature - this could hide an allowed property if (typeChecker.getIndexTypeOfType(spreadType, IndexKind.String) !== undefined) return // If there is an `as` inside the spread object, we can't autofix reliably @@ -89,12 +92,13 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ continue } - // Has sx prop, so should keep using this component - if ( - attribute.name.type === 'JSXIdentifier' && - (attribute.name.name === 'sx' || isStyledSystemProp(attribute.name.name)) - ) - return + // Has an allowed prop, so should keep using this component + for (const allowedProp of componentConfig.allowedProps) + if ( + attribute.name.type === 'JSXIdentifier' && + (attribute.name.name === allowedProp || isStyledSystemProp(attribute.name.name)) + ) + return // If there is an `as` prop we will need to account for that when autofixing if (attribute.name.type === 'JSXIdentifier' && attribute.name.name === 'as') asProp = attribute From 8c88de2dcf025173ecc4539470e05ce98df6e590 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Mon, 26 Aug 2024 13:56:22 +0000 Subject: [PATCH 2/2] Refactor using `getPropertiesOfType` --- src/rules/no-unnecessary-components.js | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/rules/no-unnecessary-components.js b/src/rules/no-unnecessary-components.js index 0a152be0..da02b655 100644 --- a/src/rules/no-unnecessary-components.js +++ b/src/rules/no-unnecessary-components.js @@ -12,13 +12,13 @@ const components = { replacement: 'div', messageId: 'unecessaryBox', message: 'Prefer plain HTML elements over `Box` when not using `sx` for styling.', - allowedProps: ['sx'], // + styled-system props + allowedProps: new Set(['sx']), // + styled-system props }, Text: { replacement: 'span', messageId: 'unecessarySpan', message: 'Prefer plain HTML elements over `Text` when not using `sx` for styling.', - allowedProps: ['sx', 'size', 'weight'], // + styled-system props + allowedProps: new Set(['sx', 'size', 'weight']), // + styled-system props }, } @@ -70,6 +70,9 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ const isPrimer = skipImportCheck || isPrimerComponent(name, context.sourceCode.getScope(openingElement)) if (!isPrimer) return + /** @param {string} name */ + const isAllowedProp = name => componentConfig.allowedProps.has(name) || isStyledSystemProp(name) + // Validate the attributes and ensure an allowed prop is present or spreaded in /** @type {typeof attributes[number] | undefined | null} */ let asProp = undefined @@ -80,25 +83,23 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ const typeChecker = services.program.getTypeChecker() const spreadType = services.getTypeAtLocation(attribute.argument) - for (const allowedProp of componentConfig.allowedProps) - if (typeChecker.getPropertyOfType(spreadType, allowedProp) !== undefined) return // Check if the spread type has a string index signature - this could hide an allowed property if (typeChecker.getIndexTypeOfType(spreadType, IndexKind.String) !== undefined) return + const spreadPropNames = typeChecker.getPropertiesOfType(spreadType).map(prop => prop.getName()) + + // If an allowed prop gets spread in, this is a valid use of the component + if (spreadPropNames.some(isAllowedProp)) return + // If there is an `as` inside the spread object, we can't autofix reliably - if (typeChecker.getPropertyOfType(spreadType, 'as') !== undefined) asProp = null + if (spreadPropNames.includes('as')) asProp = null continue } // Has an allowed prop, so should keep using this component - for (const allowedProp of componentConfig.allowedProps) - if ( - attribute.name.type === 'JSXIdentifier' && - (attribute.name.name === allowedProp || isStyledSystemProp(attribute.name.name)) - ) - return + if (attribute.name.type === 'JSXIdentifier' && isAllowedProp(attribute.name.name)) return // If there is an `as` prop we will need to account for that when autofixing if (attribute.name.type === 'JSXIdentifier' && attribute.name.name === 'as') asProp = attribute