Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add polymorphic component check in getElementType #449

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Add polymorphic component check in getElementType #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de463b3
handle polymorphic components
kendallgassner Jul 10, 2023
61a2d19
1:1 mapping
kendallgassner Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 28 additions & 29 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,16 @@ _Note: This is experimental and subject to change._

The `react` config includes rules which target specific HTML elements. You may provide a mapping of custom components to an HTML element in your `eslintrc` configuration to increase linter coverage.

For each component, you may specify a `default` and/or `props`. `default` may make sense if there's a 1:1 mapping between a component and an HTML element. However, if the HTML output of a component is dependent on a prop value, you can provide a mapping using the `props` key. To minimize conflicts and complexity, this currently only supports the mapping of a single prop type.
By default, these eslint rules will check the "as" prop for underlying element changes. If your repo uses a different prop name for polymorphic components provide the prop name in your `eslintrc` configuration under `polymorphicPropName`.

```json
{
"settings": {
"github": {
"polymorphicPropName": "asChild",
"components": {
"Box": {"default": "p"},
"Link": {"props": {"as": {"undefined": "a", "a": "a", "button": "button"}}}
"Box": "p",
"Link": "a"
}
}
}
Expand All @@ -66,9 +67,7 @@ This config will be interpreted in the following way:

- All `<Box>` elements will be treated as a `p` element type.
- `<Link>` without a defined `as` prop will be treated as a `a`.
- `<Link as='a'>` will treated as an `a` element type.
- `<Link as='button'>` will be treated as a `button` element type.
- `<Link as='summary'>` will be treated as the raw `Link` type because there is no configuration set for `as='summary'`.

### Rules

Expand All @@ -82,29 +81,29 @@ This config will be interpreted in the following way:
🔧 Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix).\
❌ Deprecated.

| Name                                        | Description | 💼 | 🔧 | ❌ |
| :------------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------- | :- | :- | :- |
| [a11y-aria-label-is-well-formatted](docs/rules/a11y-aria-label-is-well-formatted.md) | [aria-label] text should be formatted as you would visual text. | ⚛️ | | |
| [a11y-no-generic-link-text](docs/rules/a11y-no-generic-link-text.md) | disallow generic link text | | | ❌ |
| [a11y-no-visually-hidden-interactive-element](docs/rules/a11y-no-visually-hidden-interactive-element.md) | Ensures that interactive elements are not visually hidden | ⚛️ | | |
| [array-foreach](docs/rules/array-foreach.md) | enforce `for..of` loops over `Array.forEach` | ✅ | | |
| [async-currenttarget](docs/rules/async-currenttarget.md) | disallow `event.currentTarget` calls inside of async functions | 🔍 | | |
| [async-preventdefault](docs/rules/async-preventdefault.md) | disallow `event.preventDefault` calls inside of async functions | 🔍 | | |
| [authenticity-token](docs/rules/authenticity-token.md) | disallow usage of CSRF tokens in JavaScript | 🔐 | | |
| [get-attribute](docs/rules/get-attribute.md) | disallow wrong usage of attribute names | 🔍 | 🔧 | |
| [js-class-name](docs/rules/js-class-name.md) | enforce a naming convention for js- prefixed classes | 🔐 | | |
| [no-blur](docs/rules/no-blur.md) | disallow usage of `Element.prototype.blur()` | 🔍 | | |
| [no-d-none](docs/rules/no-d-none.md) | disallow usage the `d-none` CSS class | 🔐 | | |
| [no-dataset](docs/rules/no-dataset.md) | enforce usage of `Element.prototype.getAttribute` instead of `Element.prototype.datalist` | 🔍 | | |
| [no-dynamic-script-tag](docs/rules/no-dynamic-script-tag.md) | disallow creating dynamic script tags | ✅ | | |
| [no-implicit-buggy-globals](docs/rules/no-implicit-buggy-globals.md) | disallow implicit global variables | ✅ | | |
| [no-inner-html](docs/rules/no-inner-html.md) | disallow `Element.prototype.innerHTML` in favor of `Element.prototype.textContent` | 🔍 | | |
| [no-innerText](docs/rules/no-innerText.md) | disallow `Element.prototype.innerText` in favor of `Element.prototype.textContent` | 🔍 | 🔧 | |
| [no-then](docs/rules/no-then.md) | enforce using `async/await` syntax over Promises | ✅ | | |
| [no-useless-passive](docs/rules/no-useless-passive.md) | disallow marking a event handler as passive when it has no effect | 🔍 | 🔧 | |
| [prefer-observers](docs/rules/prefer-observers.md) | disallow poorly performing event listeners | 🔍 | | |
| [require-passive-events](docs/rules/require-passive-events.md) | enforce marking high frequency event handlers as passive | 🔍 | | |
| [role-supports-aria-props](docs/rules/role-supports-aria-props.md) | Enforce that elements with explicit or implicit roles defined contain only `aria-*` properties supported by that `role`. | ⚛️ | | |
| [unescaped-html-literal](docs/rules/unescaped-html-literal.md) | disallow unescaped HTML literals | 🔍 | | |
| Name                                        | Description | 💼 | 🔧 | ❌ |
| :------------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------- | :-- | :-- | :-- |
| [a11y-aria-label-is-well-formatted](docs/rules/a11y-aria-label-is-well-formatted.md) | [aria-label] text should be formatted as you would visual text. | ⚛️ | | |
| [a11y-no-generic-link-text](docs/rules/a11y-no-generic-link-text.md) | disallow generic link text | | | ❌ |
| [a11y-no-visually-hidden-interactive-element](docs/rules/a11y-no-visually-hidden-interactive-element.md) | Ensures that interactive elements are not visually hidden | ⚛️ | | |
| [array-foreach](docs/rules/array-foreach.md) | enforce `for..of` loops over `Array.forEach` | ✅ | | |
| [async-currenttarget](docs/rules/async-currenttarget.md) | disallow `event.currentTarget` calls inside of async functions | 🔍 | | |
| [async-preventdefault](docs/rules/async-preventdefault.md) | disallow `event.preventDefault` calls inside of async functions | 🔍 | | |
| [authenticity-token](docs/rules/authenticity-token.md) | disallow usage of CSRF tokens in JavaScript | 🔐 | | |
| [get-attribute](docs/rules/get-attribute.md) | disallow wrong usage of attribute names | 🔍 | 🔧 | |
| [js-class-name](docs/rules/js-class-name.md) | enforce a naming convention for js- prefixed classes | 🔐 | | |
| [no-blur](docs/rules/no-blur.md) | disallow usage of `Element.prototype.blur()` | 🔍 | | |
| [no-d-none](docs/rules/no-d-none.md) | disallow usage the `d-none` CSS class | 🔐 | | |
| [no-dataset](docs/rules/no-dataset.md) | enforce usage of `Element.prototype.getAttribute` instead of `Element.prototype.datalist` | 🔍 | | |
| [no-dynamic-script-tag](docs/rules/no-dynamic-script-tag.md) | disallow creating dynamic script tags | ✅ | | |
| [no-implicit-buggy-globals](docs/rules/no-implicit-buggy-globals.md) | disallow implicit global variables | ✅ | | |
| [no-inner-html](docs/rules/no-inner-html.md) | disallow `Element.prototype.innerHTML` in favor of `Element.prototype.textContent` | 🔍 | | |
| [no-innerText](docs/rules/no-innerText.md) | disallow `Element.prototype.innerText` in favor of `Element.prototype.textContent` | 🔍 | 🔧 | |
| [no-then](docs/rules/no-then.md) | enforce using `async/await` syntax over Promises | ✅ | | |
| [no-useless-passive](docs/rules/no-useless-passive.md) | disallow marking a event handler as passive when it has no effect | 🔍 | 🔧 | |
| [prefer-observers](docs/rules/prefer-observers.md) | disallow poorly performing event listeners | 🔍 | | |
| [require-passive-events](docs/rules/require-passive-events.md) | enforce marking high frequency event handlers as passive | 🔍 | | |
| [role-supports-aria-props](docs/rules/role-supports-aria-props.md) | Enforce that elements with explicit or implicit roles defined contain only `aria-*` properties supported by that `role`. | ⚛️ | | |
| [unescaped-html-literal](docs/rules/unescaped-html-literal.md) | disallow unescaped HTML literals | 🔍 | | |

<!-- end auto-generated rules list -->
13 changes: 5 additions & 8 deletions lib/rules/a11y-no-visually-hidden-interactive-element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const {generateObjSchema} = require('eslint-plugin-jsx-a11y/lib/util/schemas')

const defaultClassName = 'sr-only'
const defaultcomponentName = 'VisuallyHidden'
const defaultHtmlPropName = 'as'

const schema = generateObjSchema({
className: {type: 'string'},
Expand All @@ -18,12 +17,11 @@ const schema = generateObjSchema({
*/
const INTERACTIVELEMENTS = ['a', 'button', 'summary', 'select', 'option', 'textarea']

const checkIfInteractiveElement = (context, htmlPropName, node) => {
const checkIfInteractiveElement = (context, node) => {
const elementType = getElementType(context, node.openingElement)
const asProp = getPropValue(getProp(node.openingElement.attributes, htmlPropName))

for (const interactiveElement of INTERACTIVELEMENTS) {
if ((asProp ?? elementType) === interactiveElement) {
if (elementType === interactiveElement) {
return true
}
}
Expand All @@ -32,14 +30,14 @@ const checkIfInteractiveElement = (context, htmlPropName, node) => {

// if the node is visually hidden recursively check if it has interactive children
const checkIfVisuallyHiddenAndInteractive = (context, options, node, isParentVisuallyHidden) => {
const {className, componentName, htmlPropName} = options
const {className, componentName} = options
if (node.type === 'JSXElement') {
const classes = getPropValue(getProp(node.openingElement.attributes, 'className'))
const isVisuallyHiddenElement = node.openingElement.name.name === componentName
const hasSROnlyClass = typeof classes !== 'undefined' && classes.includes(className)
let isHidden = false
if (hasSROnlyClass || isVisuallyHiddenElement || !!isParentVisuallyHidden) {
if (checkIfInteractiveElement(context, htmlPropName, node)) {
if (checkIfInteractiveElement(context, node)) {
return true
}
isHidden = true
Expand Down Expand Up @@ -69,11 +67,10 @@ module.exports = {
const config = options[0] || {}
const className = config.className || defaultClassName
const componentName = config.componentName || defaultcomponentName
const htmlPropName = config.htmlPropName || defaultHtmlPropName

return {
JSXElement: node => {
if (checkIfVisuallyHiddenAndInteractive(context, {className, componentName, htmlPropName}, node, false)) {
if (checkIfVisuallyHiddenAndInteractive(context, {className, componentName}, node, false)) {
context.report({
node,
message:
Expand Down
28 changes: 9 additions & 19 deletions lib/utils/get-element-type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,18 @@ For now, we only support the mapping of one prop type to an element type, rather
*/
function getElementType(context, node) {
const {settings} = context
const rawElement = elementType(node)
if (!settings) return rawElement

const componentMap = settings.github && settings.github.components
if (!componentMap) return rawElement
const component = componentMap[rawElement]
if (!component) return rawElement
let element = component.default ? component.default : rawElement
// check if the node contains a polymorphic prop
const polymorphicPropName = settings?.github?.polymorphicPropName ?? 'as'
const rawElement = getPropValue(getProp(node.attributes, polymorphicPropName)) ?? elementType(node)

if (component.props) {
const props = Object.entries(component.props)
for (const [key, value] of props) {
const propMap = value
const propValue = getPropValue(getProp(node.attributes, key))
const mapValue = propMap[propValue]
// if a component configuration does not exists, return the raw element
if (!settings?.github?.components?.[rawElement]) return rawElement

if (mapValue) {
element = mapValue
}
}
}
return element
const defaultComponent = settings.github.components[rawElement]

// check if the default component is also defined in the configuration
return defaultComponent ? defaultComponent : defaultComponent
}

module.exports = {getElementType}
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"lint:eslint-docs": "npm run update:eslint-docs -- --check",
"lint:js": "eslint .",
"pretest": "mkdir -p node_modules/ && ln -fs $(pwd) node_modules/",
"test": "npm run eslint-check && npm run lint && mocha tests/**/*.js tests/",
"test": "mocha tests/**/*.js tests/",
"update:eslint-docs": "eslint-doc-generator"
},
"repository": {
Expand Down Expand Up @@ -65,4 +65,4 @@
"mocha": "^10.0.0",
"npm-run-all": "^4.1.5"
}
}
}
21 changes: 3 additions & 18 deletions tests/a11y-no-generic-link-text.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ ruleTester.run('a11y-no-generic-link-text', rule, {
settings: {
github: {
components: {
Link: {
props: {as: {undefined: 'a'}},
},
Link: 'a',
},
},
},
Expand All @@ -41,9 +39,7 @@ ruleTester.run('a11y-no-generic-link-text', rule, {
settings: {
github: {
components: {
ButtonLink: {
default: 'a',
},
ButtonLink: 'a',
},
},
},
Expand All @@ -54,25 +50,14 @@ ruleTester.run('a11y-no-generic-link-text', rule, {
settings: {
github: {
components: {
Link: {
props: {as: {undefined: 'a'}},
},
Link: 'a',
},
},
},
},
{
code: '<Test as="a" href="#">Read more</Test>',
errors: [{message: errorMessage}],
settings: {
github: {
components: {
Test: {
props: {as: {a: 'a'}},
},
},
},
},
},
{
code: "<Box><a href='#'>Click here</a></Box>;",
Expand Down