From a688a55ee6da91b6aee7d71d5a9c5cb24c7701e9 Mon Sep 17 00:00:00 2001 From: Frederick Engelhardt <31741411+FrederickEngelhardt@users.noreply.github.com> Date: Wed, 11 Sep 2024 11:42:13 -0700 Subject: [PATCH] Feature/add support for custom image module with image alias (#93) Co-authored-by: Eli Zibin --- .babelrc | 7 +- .changeset/funny-socks-collect.md | 5 + README.md | 29 +- __tests__/index-test.js | 8 +- ...ccessibility-ignores-invert-colors-test.js | 398 ++++++++++++++---- flow/eslint.js | 5 +- ...lid-accessibility-ignores-invert-colors.js | 145 +++++-- src/util/isTouchable.js | 3 + 8 files changed, 450 insertions(+), 150 deletions(-) create mode 100644 .changeset/funny-socks-collect.md diff --git a/.babelrc b/.babelrc index caa9a7c..9f5a8d6 100644 --- a/.babelrc +++ b/.babelrc @@ -1,14 +1,13 @@ { "presets": [ [ - "@babel/preset-env", { + "@babel/preset-env", + { "targets": { "node": "4" } } ] ], - "plugins": [ - "@babel/transform-flow-strip-types", - ] + "plugins": ["@babel/transform-flow-strip-types"] } diff --git a/.changeset/funny-socks-collect.md b/.changeset/funny-socks-collect.md new file mode 100644 index 0000000..aabef8d --- /dev/null +++ b/.changeset/funny-socks-collect.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-react-native-a11y': minor +--- + +Allow aliasing Images diff --git a/README.md b/README.md index 15eb164..0a13c13 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,7 @@ Eslint-plugin-react-native-a11y is a collection of React Native specific ESLint ## Setup ### Pre-Requisites + Before starting, check you already have ESLint as a `devDependency` of your project. > Projects created using `react-native init` will already have this, but for Expo depending on your template you may need to follow ESLint's [installation instructions](https://eslint.org/docs/user-guide/getting-started#installation-and-usage). @@ -30,12 +31,12 @@ yarn add eslint-plugin-react-native-a11y --dev This plugin exposes four recommended configs. -Name|Description --|- -basic|Only use basic validation rules common to both iOS & Android -ios|Use all rules from "basic", plus iOS-specific extras -android|Use all rules from "basic", plus Android-specific extras -all|Use all rules from "basic", plus iOS-specific extras, plus Android-specific extras +| Name | Description | +| ------- | ---------------------------------------------------------------------------------- | +| basic | Only use basic validation rules common to both iOS & Android | +| ios | Use all rules from "basic", plus iOS-specific extras | +| android | Use all rules from "basic", plus Android-specific extras | +| all | Use all rules from "basic", plus iOS-specific extras, plus Android-specific extras | If your project only supports a single platform, you may get the best experience using a platform-specific config. This will both avoid reporting issues which do not affect your platform and also results in slightly faster linting for larger projects. @@ -48,10 +49,7 @@ Add the config you want to use to the `extends` section of your ESLint config us module.exports = { root: true, - extends: [ - '@react-native-community', - 'plugin:react-native-a11y/ios' - ] + extends: ['@react-native-community', 'plugin:react-native-a11y/ios'], }; ``` @@ -62,12 +60,10 @@ Alternatively if you do not want to use one of the pre-defined configs — or wa module.exports = { root: true, - extends: [ - '@react-native-community', - ], + extends: ['@react-native-community'], rules: { - 'react-native-a11y/rule-name': 2 - } + 'react-native-a11y/rule-name': 2, + }, }; ``` @@ -76,6 +72,7 @@ For more information on configuring behaviour of an individual rule, please refe ## Supported Rules ### Basic + - [has-accessibility-hint](docs/rules/has-accessibility-hint.md): Enforce `accessibilityHint` is used in conjunction with `accessibilityLabel` - [has-accessibility-props](docs/rules/has-accessibility-props.md): Enforce that `` components only have either the `accessibilityRole` prop or both `accessibilityTraits` and `accessibilityComponentType` props set - [has-valid-accessibility-actions](docs/rules/has-valid-accessibility-actions.md): Enforce both `accessibilityActions` and `onAccessibilityAction` props are valid @@ -89,9 +86,11 @@ For more information on configuring behaviour of an individual rule, please refe - [has-valid-accessibility-descriptors](docs/rules/has-valid-accessibility-descriptors.md): Ensures that Touchable* components have appropriate props to communicate with assistive technologies ### iOS + - [has-valid-accessibility-ignores-invert-colors](docs/rules/has-valid-accessibility-ignores-invert-colors.md): Enforce that certain elements use `accessibilityIgnoresInvertColors` to avoid being inverted by device color settings. ### Android + - [has-valid-accessibility-live-region](docs/rules/has-valid-accessibility-live-region.md): Enforce `accessibilityLiveRegion` prop values must be valid - [has-valid-important-for-accessibility](docs/rules/has-valid-important-for-accessibility.md): Enforce `importantForAccessibility` property value is valid diff --git a/__tests__/index-test.js b/__tests__/index-test.js index 81c091b..da90c5a 100644 --- a/__tests__/index-test.js +++ b/__tests__/index-test.js @@ -22,8 +22,12 @@ describe('all rule files should be exported by the plugin', () => { }); describe('configurations', () => { - it("should export a 'recommended' configuration", () => { - assert(plugin.configs.recommended); + const configs = ['basic', 'ios', 'android', 'all']; + + configs.forEach((name) => { + it(`should export a '${name}' configuration`, () => { + assert(plugin.configs[name]); + }); }); }); diff --git a/__tests__/src/rules/has-valid-accessibility-ignores-invert-colors-test.js b/__tests__/src/rules/has-valid-accessibility-ignores-invert-colors-test.js index 1a55eeb..682b07a 100644 --- a/__tests__/src/rules/has-valid-accessibility-ignores-invert-colors-test.js +++ b/__tests__/src/rules/has-valid-accessibility-ignores-invert-colors-test.js @@ -15,7 +15,6 @@ import rule from '../../../src/rules/has-valid-accessibility-ignores-invert-colo // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- - const ruleTester = new RuleTester(); const typeError = { @@ -29,99 +28,320 @@ const missingPropError = { type: 'JSXElement', }; -ruleTester.run('has-valid-accessibility-ignores-invert-colors', rule, { - valid: [ - { code: ';' }, - { code: '' }, - { code: '' }, - { - code: '', - }, - { - code: '', - }, - { - code: '', - }, - { - code: '', - }, - { - code: '', - }, - { - code: '', - options: [ - { - invertableComponents: ['FastImage'], - }, - ], - }, - { - code: `const invertColors = true; +describe('verifyReactNativeImage', () => { + it('returns true when importing a named export of Image from react-native', () => { + const output = rule.functions + .verifyReactNativeImage(`import { Text, Image, View } from 'react-native'; + const Component = () => ( + + + + );`); - const Component = () => ( - - );`, - }, - { - code: ``, - }, - ].map(parserOptionsMapper), - invalid: [ - { - code: '', - errors: [typeError], - }, - { - code: '', - errors: [typeError], - }, - { - code: '', - errors: [typeError], - }, - { - code: '', - errors: [typeError], - }, - { - code: ` - - `, - errors: [typeError, missingPropError], - }, - { - code: '', - errors: [typeError], + expect(output.enableLinting).toBe(true); + }); + + it('returns false when importing a named export of a Image from any other library', () => { + const output = rule.functions + .verifyReactNativeImage(`import { Text, Image, View } from './custom-image-component/Image'; + const Component = () => ( + + + + );`); + expect(output.enableLinting).toBe(false); + }); + + /** + * Super edge case if someone wants to alias ReactNative.Image as another component like RNImage and imports an Image from './any-library' + */ + it('returns true when provided a named export of Image that is aliased as something from react-native', () => { + const output = rule.functions + .verifyReactNativeImage(`import React from 'react' + import {Image as RNImage} from 'react-native' + + const CustomImage = () => { + return + } + + export default CustomImage`); + + expect(output.enableLinting).toBe(true); + }); +}); + +const validCustomImportTests = [ + { + title: 'does not throw an error with custom Image components', + code: `import { Text, Image, View } from './custom-image-component/Image'; + const Component = () => ( + + + + );`, + parserOptions: { + sourceType: 'module', }, - { - code: '', - errors: [typeError], + }, +]; + +const validCases = [ + ...validCustomImportTests, + { code: ';' }, + { code: '' }, + { code: '' }, + { + code: '', + }, + { + code: '', + }, + { + code: '', + }, + { + code: '', + }, + { + code: '', + }, + { + code: '', + options: [ + { + invertableComponents: ['FastImage'], + }, + ], + }, + { + code: `const invertColors = true; + + const Component = () => ( + + );`, + }, + { + code: ``, + }, +]; + +const invalidCustomImport = [ + { + title: + 'throws a missing prop error for custom components alongside passing Image that is imported from react-native', + code: `import { + Image, + Button, + FlatList, + Platform, + ScrollView, + View, + } from 'react-native'; + import FastImage from './components/FastImage' + const Component = () => ( + + + + + );`, + errors: [missingPropError], + options: [ + { + invertableComponents: ['FastImage'], + }, + ], + parserOptions: { + sourceType: 'module', }, - { - code: '', - errors: [missingPropError], + }, + { + title: + 'throws a missingPropError for invertibleComponents and type error for Image when it is imported from react-native', + code: `import { + Image, + Button, + FlatList, + Platform, + ScrollView, + View, + } from 'react-native'; + import FastImage from './components/FastImage' + const Component = () => ( + + + + + );`, + errors: [missingPropError, typeError], + options: [ + { + invertableComponents: ['FastImage'], + }, + ], + parserOptions: { + sourceType: 'module', }, - { - code: '', - errors: [missingPropError], + }, + { + title: 'can throw multiple errors for custom and normal Image components', + code: `import { + Image, + Button, + FlatList, + Platform, + ScrollView, + View, + } from 'react-native'; + import FastImage from './components/FastImage' + const Component = () => ( + + + + + + + + + );`, + errors: [ + missingPropError, + typeError, + typeError, + missingPropError, + typeError, + typeError, + ], + options: [ + { + invertableComponents: ['FastImage'], + }, + ], + parserOptions: { + sourceType: 'module', }, - { - code: '', - errors: [missingPropError], + }, + { + title: 'should fail', + code: `import React from 'react' + import { Image } from 'react-native'; + + export const RNImage = (props) => + `, + errors: [missingPropError], + parserOptions: { + sourceType: 'module', }, - { - code: '', - errors: [missingPropError], - options: [ - { - invertableComponents: ['FastImage'], - }, - ], + }, + { + title: + 'supports linting on Custom Invertable ImageComponents without react-native imported', + code: `import { FastImage } from './fast-image' + + const Component = (props) => ( + <> + + + + ); + `, + errors: [missingPropError, typeError], + parserOptions: { + sourceType: 'module', }, - ].map(parserOptionsMapper), + options: [ + { + invertableComponents: ['FastImage'], + }, + ], + }, +]; + +const invalidCases = [ + { + code: '', + errors: [typeError], + }, + { + code: '', + errors: [typeError], + }, + { + code: '', + errors: [typeError], + }, + { + code: '', + errors: [typeError], + }, + { + code: ` + + `, + errors: [typeError, missingPropError], + }, + { + code: '', + errors: [typeError], + }, + { + code: '', + errors: [typeError], + }, + { + code: '', + errors: [missingPropError], + }, + { + code: '', + errors: [missingPropError], + }, + { + code: '', + errors: [missingPropError], + }, + { + code: '', + errors: [missingPropError], + options: [ + { + invertableComponents: ['FastImage'], + }, + ], + }, + ...invalidCustomImport, +]; + +/** + * Solution to rule tester's dynamic title issue + */ +RuleTester.describe = function (text, method) { + RuleTester.testId = 0; + RuleTester.it.title = text; + + method.bind({ testId: 0 }); + + describe(`${RuleTester.it.title}`, method); +}; + +RuleTester.it = function (text, method) { + const computedTitle = eval(`${RuleTester.it.title}Cases`)[RuleTester.testId] + .title; + + if (computedTitle) { + describe(computedTitle, () => { + it(text, method); + }); + } else { + it(text, method); + } + + RuleTester.testId += 1; +}; + +ruleTester.run('has-valid-accessibility-ignores-invert-colors', rule, { + valid: validCases.map(parserOptionsMapper), + invalid: invalidCases.map(parserOptionsMapper), }); diff --git a/flow/eslint.js b/flow/eslint.js index 62c4eb2..1b48a06 100644 --- a/flow/eslint.js +++ b/flow/eslint.js @@ -9,5 +9,8 @@ export type ESLintReport = { export type ESLintContext = { id: string, options: Array, - report: ESLintReport => void, + report: (ESLintReport) => void, + getSourceCode: () => { + text: string, + }, }; diff --git a/src/rules/has-valid-accessibility-ignores-invert-colors.js b/src/rules/has-valid-accessibility-ignores-invert-colors.js index 46ec24c..0c69392 100644 --- a/src/rules/has-valid-accessibility-ignores-invert-colors.js +++ b/src/rules/has-valid-accessibility-ignores-invert-colors.js @@ -32,57 +32,124 @@ const checkParent = ({ openingElement, parent }) => { return true; }; +type VerifyRNImageRes = { + enableLinting: boolean, + elementsToCheck: string[], +}; + +/** + * @description varifies that the Image asset is imported from 'react-native' otherwise exits linting + */ +const verifyReactNativeImage = (text: string): VerifyRNImageRes => { + const res: VerifyRNImageRes = { + enableLinting: true, + elementsToCheck: defaultInvertableComponents, + }; + + // Escape hatch for tests + if (!text.match(new RegExp(/import/, 'g'))) { + return res; + } + + // Flow has issues with String.raw + // $FlowFixMe + const namedSelector = String.raw`(import\s{)(.*)(\bImage\b)(.*)(}\sfrom\s'react-native')`; + // $FlowFixMe + const es6moduleSelector = String.raw`(?<=Image as )(.*?)(?=} from 'react-native')`; + + const imageSourceReactNativeRegExp = new RegExp(`${namedSelector}`, 'gs'); + const imageSourceReactNativeAliasRegExp = new RegExp( + `${es6moduleSelector}`, + 'gs' + ); + + const matchedImage = text.match(imageSourceReactNativeRegExp) || []; + const matchedAliasImage = text.match(imageSourceReactNativeAliasRegExp) || []; + + res.enableLinting = + matchedImage.length === 1 || matchedAliasImage.length === 1; + + if (matchedAliasImage.length === 1) { + res.elementsToCheck = [matchedAliasImage[0].trim()]; + } + + return res; +}; + module.exports = { meta: { docs: {}, schema: [schema], }, + functions: { + verifyReactNativeImage, + }, - create: ({ options, report }: ESLintContext) => ({ - JSXElement: (node: JSXElement) => { - // $FlowFixMe - const { children, openingElement, parent } = node; - - if ( - hasProp(openingElement.attributes, propName) && - !isNodePropValueBoolean(getProp(openingElement.attributes, propName)) - ) { - report({ - node, - message: - 'accessibilityIgnoresInvertColors prop is not a boolean value', - }); - } else { - const elementsToCheck = defaultInvertableComponents; - if (options.length > 0) { - const { invertableComponents } = options[0]; - if (invertableComponents) { - elementsToCheck.push(...invertableComponents); - } - } + create: ({ options, report, getSourceCode }: ESLintContext) => { + /** + * Checks to see if there are valid imports and if so verifies that those imports related to 'react-native' or if a custom module exists + * */ + const { text } = getSourceCode(); + const { enableLinting, elementsToCheck } = verifyReactNativeImage(text); + + // Add in any other invertible components to check for + if (options.length > 0) { + const { invertableComponents } = options[0]; + if (invertableComponents) { + elementsToCheck.push(...invertableComponents); + } + } + + // Exit process if there is nothing to check + if (!enableLinting && options.length === 0) { + return {}; + } - const type = elementType(openingElement); + return { + JSXElement: (node: JSXElement) => { + // $FlowFixMe + const { children, openingElement, parent } = node; if ( - elementsToCheck.indexOf(type) > -1 && - !hasValidIgnoresInvertColorsProp(openingElement) && - children.length === 0 + hasProp(openingElement.attributes, propName) && + !isNodePropValueBoolean(getProp(openingElement.attributes, propName)) ) { - let shouldReport = true; - - if (parent.openingElement) { - shouldReport = checkParent(parent); + report({ + node, + message: + 'accessibilityIgnoresInvertColors prop is not a boolean value', + }); + } else { + if (options.length > 0) { + const { invertableComponents } = options[0]; + if (invertableComponents) { + elementsToCheck.push(...invertableComponents); + } } - if (shouldReport) { - report({ - node, - message: - 'Found an element which will be inverted. Add the accessibilityIgnoresInvertColors prop', - }); + const type = elementType(openingElement); + + if ( + elementsToCheck.indexOf(type) > -1 && + !hasValidIgnoresInvertColorsProp(openingElement) && + children.length === 0 + ) { + let shouldReport = true; + + if (parent.openingElement) { + shouldReport = checkParent(parent); + } + + if (shouldReport) { + report({ + node, + message: + 'Found an element which will be inverted. Add the accessibilityIgnoresInvertColors prop', + }); + } } } - } - }, - }), + }, + }; + }, }; diff --git a/src/util/isTouchable.js b/src/util/isTouchable.js index 63da9ce..fdbbb4b 100644 --- a/src/util/isTouchable.js +++ b/src/util/isTouchable.js @@ -24,6 +24,9 @@ export default function isTouchable( id: '', options: [], report: () => {}, + getSourceCode: () => ({ + text: '', + }), } ) { const { options } = context;