From 3d77c845a98b6fc8cf10c810996278c02e308f35 Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Mon, 9 Jan 2023 13:16:45 -0800 Subject: [PATCH] [Refactor] use fromEntries, flatMap, etc; better use iteration methods --- __mocks__/genInteractives.js | 35 +++++----- .../__util__/ruleOptionsMapperFactory.js | 9 +-- .../rules/aria-unsupported-elements-test.js | 4 +- package.json | 3 + src/rules/alt-text.js | 12 ++-- src/rules/anchor-is-valid.js | 20 +++--- src/rules/interactive-supports-focus.js | 7 +- src/rules/media-has-caption.js | 6 +- src/util/getSuggestion.js | 14 ++-- src/util/hasAccessibleChild.js | 4 +- src/util/isInteractiveElement.js | 65 +++++-------------- src/util/isInteractiveRole.js | 21 +++--- src/util/isNonInteractiveElement.js | 53 ++++----------- src/util/isNonInteractiveRole.js | 21 +++--- 14 files changed, 110 insertions(+), 164 deletions(-) diff --git a/__mocks__/genInteractives.js b/__mocks__/genInteractives.js index 0f1ef30b8..11f32ec9b 100644 --- a/__mocks__/genInteractives.js +++ b/__mocks__/genInteractives.js @@ -4,6 +4,8 @@ import { dom, roles } from 'aria-query'; import includes from 'array-includes'; +import fromEntries from 'object.fromentries'; + import JSXAttributeMock from './JSXAttributeMock'; import JSXElementMock from './JSXElementMock'; @@ -115,13 +117,7 @@ const nonInteractiveElementsMap: {[string]: Array<{[string]: string}>} = { ul: [], }; -const indeterminantInteractiveElementsMap = domElements.reduce( - (accumulator: { [key: string]: Array }, name: string): { [key: string]: Array } => ({ - ...accumulator, - [name]: [], - }), - {}, -); +const indeterminantInteractiveElementsMap: { [key: string]: Array } = fromEntries(domElements.map((name: string) => [name, []])); Object.keys(interactiveElementsMap) .concat(Object.keys(nonInteractiveElementsMap)) @@ -138,22 +134,25 @@ const interactiveRoles = [] // aria-activedescendant, thus in practice we treat it as a widget. 'toolbar', ) - .filter((role) => !roles.get(role).abstract) - .filter((role) => roles.get(role).superClass.some((klasses) => includes(klasses, 'widget'))); + .filter((role) => ( + !roles.get(role).abstract + && roles.get(role).superClass.some((klasses) => includes(klasses, 'widget')) + )); const nonInteractiveRoles = roleNames - .filter((role) => !roles.get(role).abstract) - .filter((role) => !roles.get(role).superClass.some((klasses) => includes(klasses, 'widget'))) - // 'toolbar' does not descend from widget, but it does support - // aria-activedescendant, thus in practice we treat it as a widget. - .filter((role) => !includes(['toolbar'], role)); + .filter((role) => ( + !roles.get(role).abstract + && !roles.get(role).superClass.some((klasses) => includes(klasses, 'widget')) + + // 'toolbar' does not descend from widget, but it does support + // aria-activedescendant, thus in practice we treat it as a widget. + && !includes(['toolbar'], role) + )); export function genElementSymbol(openingElement: Object): string { return ( openingElement.name.name + (openingElement.attributes.length > 0 - ? `${openingElement.attributes - .map((attr) => `[${attr.name.name}="${attr.value.value}"]`) - .join('')}` + ? `${openingElement.attributes.map((attr) => `[${attr.name.name}="${attr.value.value}"]`).join('')}` : '' ) ); @@ -172,7 +171,7 @@ export function genInteractiveElements(): Array { } export function genInteractiveRoleElements(): Array { - return [...interactiveRoles, 'button article', 'fakerole button article'].map((value): JSXElementMockType => JSXElementMock( + return interactiveRoles.concat('button article', 'fakerole button article').map((value): JSXElementMockType => JSXElementMock( 'div', [JSXAttributeMock('role', value)], )); diff --git a/__tests__/__util__/ruleOptionsMapperFactory.js b/__tests__/__util__/ruleOptionsMapperFactory.js index 1e1b628b0..5e223d94f 100644 --- a/__tests__/__util__/ruleOptionsMapperFactory.js +++ b/__tests__/__util__/ruleOptionsMapperFactory.js @@ -2,6 +2,10 @@ * @flow */ +import entries from 'object.entries'; +import flatMap from 'array.prototype.flatmap'; +import fromEntries from 'object.fromentries'; + type ESLintTestRunnerTestCase = { code: string, errors: ?Array<{ message: string, type: string }>, @@ -21,10 +25,7 @@ export default function ruleOptionsMapperFactory(ruleOptions: Array = []) code, errors, // Flatten the array of objects in an array of one object. - options: (options || []).concat(ruleOptions).reduce((acc, item) => [{ - ...acc[0], - ...item, - }], [{}]), + options: [fromEntries(flatMap((options || []).concat(ruleOptions), (item) => entries(item)))], parserOptions, settings, }; diff --git a/__tests__/src/rules/aria-unsupported-elements-test.js b/__tests__/src/rules/aria-unsupported-elements-test.js index b29a617c5..1e1b94212 100644 --- a/__tests__/src/rules/aria-unsupported-elements-test.js +++ b/__tests__/src/rules/aria-unsupported-elements-test.js @@ -50,7 +50,7 @@ const ariaValidityTests = domElements.map((element) => { // Generate invalid test cases. const invalidRoleValidityTests = domElements - .filter((element) => Boolean(dom.get(element).reserved)) + .filter((element) => dom.get(element).reserved) .map((reservedElem) => ({ code: `<${reservedElem} role {...props} />`, errors: [errorMessage('role')], @@ -61,7 +61,7 @@ const invalidRoleValidityTests = domElements }); const invalidAriaValidityTests = domElements - .filter((element) => Boolean(dom.get(element).reserved)) + .filter((element) => dom.get(element).reserved) .map((reservedElem) => ({ code: `<${reservedElem} aria-hidden aria-role="none" {...props} />`, errors: [errorMessage('aria-hidden')], diff --git a/package.json b/package.json index 0deb55dda..c493b9338 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "@babel/runtime": "^7.20.7", "aria-query": "^5.1.3", "array-includes": "^3.1.6", + "array.prototype.flatmap": "^1.3.1", "ast-types-flow": "^0.0.7", "axe-core": "^4.6.2", "axobject-query": "^3.1.1", @@ -81,6 +82,8 @@ "jsx-ast-utils": "^3.3.3", "language-tags": "=1.0.5", "minimatch": "^3.1.2", + "object.entries": "^1.1.6", + "object.fromentries": "^2.0.6", "semver": "^6.3.0" }, "peerDependencies": { diff --git a/src/rules/alt-text.js b/src/rules/alt-text.js index 83c8b5588..495774563 100644 --- a/src/rules/alt-text.js +++ b/src/rules/alt-text.js @@ -12,6 +12,8 @@ import { getPropValue, getLiteralPropValue, } from 'jsx-ast-utils'; +import flatMap from 'array.prototype.flatmap'; + import { generateObjSchema, arraySchema } from '../util/schemas'; import getElementType from '../util/getElementType'; import hasAccessibleChild from '../util/hasAccessibleChild'; @@ -204,12 +206,10 @@ export default { // Elements to validate for alt text. const elementOptions = options.elements || DEFAULT_ELEMENTS; // Get custom components for just the elements that will be tested. - const customComponents = elementOptions - .map((element) => options[element]) - .reduce( - (components, customComponentsForElement) => components.concat(customComponentsForElement || []), - [], - ); + const customComponents = flatMap( + elementOptions, + (element) => options[element], + ); const typesToValidate = new Set( [].concat( customComponents, diff --git a/src/rules/anchor-is-valid.js b/src/rules/anchor-is-valid.js index 4e3520628..7972db758 100644 --- a/src/rules/anchor-is-valid.js +++ b/src/rules/anchor-is-valid.js @@ -64,16 +64,12 @@ export default ({ const propOptions = options.specialLink || []; const propsToValidate = ['href'].concat(propOptions); - const values = propsToValidate - .map((prop) => getProp(node.attributes, prop)) - .map((prop) => getPropValue(prop)); + const values = propsToValidate.map((prop) => getPropValue(getProp(node.attributes, prop))); // Checks if any actual or custom href prop is provided. - const hasAnyHref = values - .filter((value) => value === undefined || value === null).length !== values.length; + const hasAnyHref = values.some((value) => value != null); // Need to check for spread operator as props can be spread onto the element // leading to an incorrect validation error. - const hasSpreadOperator = attributes - .filter((prop) => prop.type === 'JSXSpreadAttribute').length > 0; + const hasSpreadOperator = attributes.some((prop) => prop.type === 'JSXSpreadAttribute'); const onClick = getProp(attributes, 'onClick'); // When there is no href at all, specific scenarios apply: @@ -99,10 +95,12 @@ export default ({ // Hrefs have been found, now check for validity. const invalidHrefValues = values - .filter((value) => value !== undefined && value !== null) - .filter((value) => (typeof value === 'string' && ( - !value.length || value === '#' || /^\W*?javascript:/.test(value) - ))); + .filter((value) => ( + value != null + && (typeof value === 'string' && ( + !value.length || value === '#' || /^\W*?javascript:/.test(value) + )) + )); if (invalidHrefValues.length !== 0) { // If an onClick is found it should be a button, otherwise it is an invalid link. if (onClick && activeAspects.preferButton) { diff --git a/src/rules/interactive-supports-focus.js b/src/rules/interactive-supports-focus.js index f13cbf45e..2dec63006 100644 --- a/src/rules/interactive-supports-focus.js +++ b/src/rules/interactive-supports-focus.js @@ -36,9 +36,10 @@ import getTabIndex from '../util/getTabIndex'; // ---------------------------------------------------------------------------- const schema = generateObjSchema({ - tabbable: enumArraySchema([...roles.keys()] - .filter((name) => !roles.get(name).abstract) - .filter((name) => roles.get(name).superClass.some((klasses) => includes(klasses, 'widget')))), + tabbable: enumArraySchema([...roles.keys()].filter((name) => ( + !roles.get(name).abstract + && roles.get(name).superClass.some((klasses) => includes(klasses, 'widget')) + ))), }); const domElements = [...dom.keys()]; diff --git a/src/rules/media-has-caption.js b/src/rules/media-has-caption.js index 0b9ef8386..d20003deb 100644 --- a/src/rules/media-has-caption.js +++ b/src/rules/media-has-caption.js @@ -10,6 +10,8 @@ import type { JSXElement, JSXOpeningElement, Node } from 'ast-types-flow'; import { getProp, getLiteralPropValue } from 'jsx-ast-utils'; +import flatMap from 'array.prototype.flatmap'; + import type { ESLintConfig, ESLintContext, ESLintVisitorSelectorConfig } from '../../flow/eslint'; import { generateObjSchema, arraySchema } from '../util/schemas'; import getElementType from '../util/getElementType'; @@ -26,8 +28,8 @@ const schema = generateObjSchema({ const isMediaType = (context, type) => { const options = context.options[0] || {}; - return MEDIA_TYPES.map((mediaType) => options[mediaType]) - .reduce((types, customComponent) => types.concat(customComponent), MEDIA_TYPES) + return MEDIA_TYPES + .concat(flatMap(MEDIA_TYPES, (mediaType) => options[mediaType])) .some((typeToCheck) => typeToCheck === type); }; diff --git a/src/util/getSuggestion.js b/src/util/getSuggestion.js index 7c58c11ee..2dd8d3c5e 100644 --- a/src/util/getSuggestion.js +++ b/src/util/getSuggestion.js @@ -1,4 +1,5 @@ import editDistance from 'damerau-levenshtein'; +import fromEntries from 'object.fromentries'; // Minimum edit distance to be considered a good suggestion. const THRESHOLD = 2; @@ -8,12 +9,13 @@ const THRESHOLD = 2; * to return. */ export default function getSuggestion(word, dictionary = [], limit = 2) { - const distances = dictionary.reduce((suggestions, dictionaryWord) => { - const distance = editDistance(word.toUpperCase(), dictionaryWord.toUpperCase()); - const { steps } = distance; - suggestions[dictionaryWord] = steps; // eslint-disable-line - return suggestions; - }, {}); + const distances = fromEntries( + dictionary.map((dictionaryWord) => { + const distance = editDistance(word.toUpperCase(), dictionaryWord.toUpperCase()); + const { steps } = distance; + return [dictionaryWord, steps]; + }), + ); return Object.keys(distances) .filter((suggestion) => distances[suggestion] <= THRESHOLD) diff --git a/src/util/hasAccessibleChild.js b/src/util/hasAccessibleChild.js index 6fe255b20..df8cd6a01 100644 --- a/src/util/hasAccessibleChild.js +++ b/src/util/hasAccessibleChild.js @@ -8,10 +8,10 @@ export default function hasAccessibleChild(node: JSXElement, elementType: (JSXOp return node.children.some((child: Node) => { switch (child.type) { case 'Literal': - return Boolean(child.value); + return !!child.value; // $FlowFixMe JSXText is missing in ast-types-flow case 'JSXText': - return Boolean(child.value); + return !!child.value; case 'JSXElement': return !isHiddenFromScreenReader( elementType(child.openingElement), diff --git a/src/util/isInteractiveElement.js b/src/util/isInteractiveElement.js index 3b826975d..3475d678c 100644 --- a/src/util/isInteractiveElement.js +++ b/src/util/isInteractiveElement.js @@ -12,6 +12,7 @@ import { elementAXObjects, } from 'axobject-query'; import includes from 'array-includes'; +import flatMap from 'array.prototype.flatmap'; import attributesComparator from './attributesComparator'; const domKeys = [...dom.keys()]; @@ -39,8 +40,8 @@ const interactiveRoles = new Set(roleKeys const role = roles.get(name); return ( !role.abstract - // The `progressbar` is descended from `widget`, but in practice, its - // value is always `readonly`, so we treat it as a non-interactive role. + // The `progressbar` is descended from `widget`, but in practice, its + // value is always `readonly`, so we treat it as a non-interactive role. && name !== 'progressbar' && role.superClass.some((classes) => includes(classes, 'widget')) ); @@ -50,50 +51,23 @@ const interactiveRoles = new Set(roleKeys 'toolbar', )); -const nonInteractiveElementRoleSchemas = elementRoleEntries - .reduce(( - accumulator, - [ - elementSchema, - roleSet, - ], - ) => { - if ([...roleSet].every((role): boolean => nonInteractiveRoles.has(role))) { - accumulator.push(elementSchema); - } - return accumulator; - }, []); +const nonInteractiveElementRoleSchemas = flatMap( + elementRoleEntries, + ([elementSchema, roleSet]) => ([...roleSet].every((role): boolean => nonInteractiveRoles.has(role)) ? [elementSchema] : []), +); -const interactiveElementRoleSchemas = elementRoleEntries - .reduce(( - accumulator, - [ - elementSchema, - roleSet, - ], - ) => { - if ([...roleSet].some((role): boolean => interactiveRoles.has(role))) { - accumulator.push(elementSchema); - } - return accumulator; - }, []); +const interactiveElementRoleSchemas = flatMap( + elementRoleEntries, + ([elementSchema, roleSet]) => ([...roleSet].some((role): boolean => interactiveRoles.has(role)) ? [elementSchema] : []), +); const interactiveAXObjects = new Set([...AXObjects.keys()] .filter((name) => AXObjects.get(name).type === 'widget')); -const interactiveElementAXObjectSchemas = [...elementAXObjects] - .reduce(( - accumulator, - [ - elementSchema, - AXObjectSet, - ], - ) => { - if ([...AXObjectSet].every((role): boolean => interactiveAXObjects.has(role))) { - accumulator.push(elementSchema); - } - return accumulator; - }, []); +const interactiveElementAXObjectSchemas = flatMap( + [...elementAXObjects], + ([elementSchema, AXObjectSet]) => ([...AXObjectSet].every((role): boolean => interactiveAXObjects.has(role)) ? [elementSchema] : []), +); function checkIsInteractiveElement(tagName, attributes): boolean { function elementSchemaMatcher(elementSchema) { @@ -104,21 +78,18 @@ function checkIsInteractiveElement(tagName, attributes): boolean { } // Check in elementRoles for inherent interactive role associations for // this element. - const isInherentInteractiveElement = interactiveElementRoleSchemas - .some(elementSchemaMatcher); + const isInherentInteractiveElement = interactiveElementRoleSchemas.some(elementSchemaMatcher); if (isInherentInteractiveElement) { return true; } // Check in elementRoles for inherent non-interactive role associations for // this element. - const isInherentNonInteractiveElement = nonInteractiveElementRoleSchemas - .some(elementSchemaMatcher); + const isInherentNonInteractiveElement = nonInteractiveElementRoleSchemas.some(elementSchemaMatcher); if (isInherentNonInteractiveElement) { return false; } // Check in elementAXObjects for AX Tree associations for this element. - const isInteractiveAXElement = interactiveElementAXObjectSchemas - .some(elementSchemaMatcher); + const isInteractiveAXElement = interactiveElementAXObjectSchemas.some(elementSchemaMatcher); if (isInteractiveAXElement) { return true; } diff --git a/src/util/isInteractiveRole.js b/src/util/isInteractiveRole.js index e7d64e55e..74bfb623b 100644 --- a/src/util/isInteractiveRole.js +++ b/src/util/isInteractiveRole.js @@ -3,11 +3,13 @@ import { roles as rolesMap } from 'aria-query'; import type { Node } from 'ast-types-flow'; import { getProp, getLiteralPropValue } from 'jsx-ast-utils'; import includes from 'array-includes'; +import flatMap from 'array.prototype.flatmap'; const roles = [...rolesMap.keys()]; -const interactiveRoles = roles - .filter((name) => !rolesMap.get(name).abstract) - .filter((name) => rolesMap.get(name).superClass.some((klasses) => includes(klasses, 'widget'))); +const interactiveRoles = roles.filter((name) => ( + !rolesMap.get(name).abstract + && rolesMap.get(name).superClass.some((klasses) => includes(klasses, 'widget')) +)); // 'toolbar' does not descend from widget, but it does support // aria-activedescendant, thus in practice we treat it as a widget. @@ -38,15 +40,10 @@ const isInteractiveRole = ( let isInteractive = false; const normalizedValues = String(value).toLowerCase().split(' '); - const validRoles = normalizedValues.reduce(( - accumulator: Array, - name: string, - ) => { - if (includes(roles, name)) { - accumulator.push(name); - } - return accumulator; - }, []); + const validRoles = flatMap( + normalizedValues, + (name: string) => (includes(roles, name) ? [name] : []), + ); if (validRoles.length > 0) { // The first role value is a series takes precedence. isInteractive = includes(interactiveRoles, validRoles[0]); diff --git a/src/util/isNonInteractiveElement.js b/src/util/isNonInteractiveElement.js index c70503ac6..78aabaa53 100644 --- a/src/util/isNonInteractiveElement.js +++ b/src/util/isNonInteractiveElement.js @@ -13,6 +13,8 @@ import { } from 'axobject-query'; import type { Node } from 'ast-types-flow'; import includes from 'array-includes'; +import flatMap from 'array.prototype.flatmap'; + import attributesComparator from './attributesComparator'; const roleKeys = [...roles.keys()]; @@ -56,50 +58,23 @@ const interactiveRoles = new Set(roleKeys 'toolbar', )); -const nonInteractiveElementRoleSchemas = elementRoleEntries - .reduce(( - accumulator, - [ - elementSchema, - roleSet, - ], - ) => { - if ([...roleSet].every((role): boolean => nonInteractiveRoles.has(role))) { - accumulator.push(elementSchema); - } - return accumulator; - }, []); +const nonInteractiveElementRoleSchemas = flatMap( + elementRoleEntries, + ([elementSchema, roleSet]) => ([...roleSet].every((role): boolean => nonInteractiveRoles.has(role)) ? [elementSchema] : []), +); -const interactiveElementRoleSchemas = elementRoleEntries - .reduce(( - accumulator, - [ - elementSchema, - roleSet, - ], - ) => { - if ([...roleSet].some((role): boolean => interactiveRoles.has(role))) { - accumulator.push(elementSchema); - } - return accumulator; - }, []); +const interactiveElementRoleSchemas = flatMap( + elementRoleEntries, + ([elementSchema, roleSet]) => ([...roleSet].some((role): boolean => interactiveRoles.has(role)) ? [elementSchema] : []), +); const nonInteractiveAXObjects = new Set([...AXObjects.keys()] .filter((name) => includes(['window', 'structure'], AXObjects.get(name).type))); -const nonInteractiveElementAXObjectSchemas = [...elementAXObjects] - .reduce(( - accumulator, - [ - elementSchema, - AXObjectSet, - ], - ) => { - if ([...AXObjectSet].every((role): boolean => nonInteractiveAXObjects.has(role))) { - accumulator.push(elementSchema); - } - return accumulator; - }, []); +const nonInteractiveElementAXObjectSchemas = flatMap( + [...elementAXObjects], + ([elementSchema, AXObjectSet]) => ([...AXObjectSet].every((role): boolean => nonInteractiveAXObjects.has(role)) ? [elementSchema] : []), +); function checkIsNonInteractiveElement(tagName, attributes): boolean { function elementSchemaMatcher(elementSchema) { diff --git a/src/util/isNonInteractiveRole.js b/src/util/isNonInteractiveRole.js index a05794a28..54aff8550 100644 --- a/src/util/isNonInteractiveRole.js +++ b/src/util/isNonInteractiveRole.js @@ -9,11 +9,13 @@ import { import type { Node } from 'ast-types-flow'; import { getProp, getLiteralPropValue } from 'jsx-ast-utils'; import includes from 'array-includes'; +import flatMap from 'array.prototype.flatmap'; const roles = [...rolesMap.keys()]; -const nonInteractiveRoles = roles - .filter((name) => !rolesMap.get(name).abstract) - .filter((name) => !rolesMap.get(name).superClass.some((klasses) => includes(klasses, 'widget'))); +const nonInteractiveRoles = roles.filter((name) => ( + !rolesMap.get(name).abstract + && !rolesMap.get(name).superClass.some((klasses) => includes(klasses, 'widget')) +)); /** * Returns boolean indicating whether the given element has a role @@ -47,15 +49,10 @@ const isNonInteractiveRole = ( let isNonInteractive = false; const normalizedValues = String(role).toLowerCase().split(' '); - const validRoles = normalizedValues.reduce(( - accumulator: Array, - name: string, - ) => { - if (includes(roles, name)) { - accumulator.push(name); - } - return accumulator; - }, []); + const validRoles = flatMap( + normalizedValues, + (name: string) => (includes(roles, name) ? [name] : []), + ); if (validRoles.length > 0) { // The first role value is a series takes precedence. isNonInteractive = includes(nonInteractiveRoles, validRoles[0]);