Enable rules from `eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y` #1118

beaesguerra · 2025-02-05T23:12:26Z

Summary:

This work is part of the implementation of ADR#781 Enabling more lint rules for accessibility

These changes includes:

Updating the eslint-plugin-jsx-a11y dependency
Configuring rules and settings (including custom component mapping, attribute mapping, polymorphic components) for eslint-plugin-jsx-a11y based on the proposed rules and settings in the ADR and the POC
Updating the README to include instructions for using @khanacademy/eslint-config/a11y

Issue: FEI-1133

Implementation Plan:

Create eslint config for a11y #1114 Set up eslint a11y config and enable aphrodite-add-style-variable-name
Use this new config in WB, perseus, webapp and address lint errors from the aphrodite-add-style-variable-name rule
(this PR) Update the a11y.js config with the config based on the accessibility linting rules ADR
Use the updated config in projects and address existing errors by disabling the rules per line. Teams can address existing errors as they work in the area and new errors can be prevented with these lint rules!

Test plan:

Testing the new config locally:

Run yarn pack in packages/eslint-config-khan. This will create a .tgz file in the directory
In another project, install the package: ex: yarn add -D -W /Users/beaesguerra/khan/wonder-stuff/packages/eslint-config-khan/khanacademy-eslint-config-v5.1.0.tgz
- Note: you might need to remove the node_modules first to make sure it installed the correct package. You can check by checking node_modules/@khanacademy/eslint-config/a11y.js in the project to see if the new config is there
Make sure the project already extends @khanacademy/eslint-config/a11y in the project's eslint config file
Run yarn lint/pnpm lint in the project and restart the ESLint server. It should show errors from the jsx-a11y plugin

Note: Before merging and releasing these changes, I'll test these changes locally with the different projects and evaluate the errors to make sure the config changes are still relevant and helpful!

…ps://github.com/Khan/webapp/pull/26840/files#diff-20e55c78ab86b0eb87c6f756500b87f451b52fab251d6f327ce572ec88e8ae3b and ADR: https://khanacademy.atlassian.net/wiki/x/IoBVyg

…llow the naming convention StyledElement

…eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y`

changeset-bot · 2025-02-05T23:12:30Z

🦋 Changeset detected

Latest commit: 9d50ac1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@khanacademy/eslint-config	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2025-02-05T23:14:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.85%. Comparing base (046f326) to head (9d50ac1).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1118   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files          97       97           
  Lines        1392     1392           
  Branches      358      359    +1     
=======================================
  Hits         1390     1390           
  Misses          1        1           
  Partials        1        1

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 046f326...9d50ac1. Read the comment docs.

github-actions · 2025-02-05T23:15:32Z

Size Change: 0 B

Total Size: 4.63 kB

ℹ️ View Unchanged

Filename	Size
`packages/wonder-stuff-core/dist/browser/es/index.js`	1.85 kB
`packages/wonder-stuff-sentry/dist/browser/es/index.js`	1.65 kB
`packages/wonder-stuff-testing/dist/browser/es/index.js`	1.12 kB

_{compressed-size-action}

beaesguerra · 2025-02-05T23:29:57Z

packages/eslint-config-khan/README.md

-For regulard repos, the settings will look like this:
-
-```
-For monorepos the `"import/resolver"` settings will look like this:


This was duplicated from line 12 and not needed (I think!)

khan-actions-bot · 2025-02-05T23:39:33Z

Gerald

Required Reviewers

@Khan/frontend-infra for changes to package.json, yarn.lock, .changeset/quiet-snails-cheer.md, packages/eslint-config-khan/README.md, packages/eslint-config-khan/a11y.js, packages/eslint-config-khan/package.json

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

marcysutton

This looks great to me, thanks for all the thorough comments and rule settings! It will be good to hear @kevinb-khan's take on it too.

kevinb-khan

LGTM. I didn't look at the specific rules though. 🎉

kevinb-khan · 2025-02-08T02:23:15Z

packages/eslint-config-khan/a11y.js

+                // Mapping for common wrappers for html elements when we can use `addStyle`
+                StyledA: "a",
+                StyledButton: "button",
+                StyledImg: "img",
+                StyledSvg: "svg",
+                StyledUl: "ul",
+                StyledOl: "ol",
+                StyledLi: "li",
+                StyledSpan: "span",
+                StyledDiv: "div",
+                StyledSection: "section",
+                StyledHeader: "header",
+                StyledFooter: "footer",
+                StyledBlockquote: "blockquote",
+                StyledForm: "form",
+                StyledOutput: "output",
+                StyledIframe: "iframe",
+                StyledHr: "hr",
+                StyledP: "p",
+                StyledFieldset: "fieldset",
+                StyledLegend: "legend",
+                StyledCaption: "caption",
+                StyledPre: "pre",
+                StyledSup: "sup",
+                StyledMark: "mark",
+                StyledTable: "table",
+                StyledTr: "tr",
+                StyledTd: "td",
+                StyledTh: "th",
+                StyledDl: "dl",
+                StyledDt: "dt",
+                StyledDd: "dd",


So many. It would be nice to have a wonder-blocks package that exports this. Is that something that's on the roadmap?

Do you mean a WB package that exports this StyledX component mapping? Would it be useful for other cases? If not, another option would be to move this mapping to another file in this package and import it from there so the config file isn't as long! What do you think?

Or did you mean it'd be nice to have a WB package that exports the StyledX components already (as we briefly chatted about 😄)?

I was thinking of a WB package that does a bunch of exports like:

export const StyleP = addStyle("p"); export const styledTable = addStyle("table"); ...

This suggestion is only tangentially related to this PR. It's so that people don't need to remember to use addStyle to get these variants. Eventually we could even add lint rules to disallow non-styled variants.

Thanks for clarifying! I created https://khanacademy.atlassian.net/browse/WB-1879 for this 😄

kevinb-khan · 2025-02-08T02:27:20Z

packages/eslint-config-khan/a11y.js

 const ERROR = "error";
+const OFF = "off";


I wondering if switching to tseslint.config means we'll get property type checking for our configs. No action req'd, just something we may want to think about for the future. See https://typescript-eslint.io/packages/typescript-eslint#config for details.

Thanks for sharing, that would be helpful down the line, yes! I was following the format from our default config file 😅

…th AccordionSectionHeader tag prop in WB

…h projects

beaesguerra · 2025-02-10T23:41:36Z

I tested the config changes locally in webapp, wonder-blocks and perseus and evaluated the errors! I found a few more ways to configure the settings so that the lint rules are more effective, the config is updated now (see last 2 commits) 😄

marcysutton · 2025-02-11T05:21:40Z

packages/eslint-config-khan/a11y.js

+                // for labels to be assosciated with the control for the
+                // jsx-a11y/label-has-associated-control rule.
+                SingleSelect: "select",
+                MultiSelect: "select",


Nice. Good find!!

marcysutton

This is awesome!! I tested it out locally in Wonder Blocks and it worked great. One minor change in the repro steps was that I had to use lowercase -w with pnpm.

Once I got that and the correct tarball path, it installed properly and caught non-semantic elements with addStyle. Great work, @beaesguerra!

## Summary: This work is part of the implementation of [ADR#781 Enabling more lint rules for accessibility](https://khanacademy.atlassian.net/wiki/x/IoBVyg) These changes include: - Updating `@khanacademy/eslint-config` to the latest version to include new jsx-a11y configuration and settings - Addressed existing errors by adding `eslint-disable-next-line` - View the PR commits for a breakdown of errors by rule - Used `npx [email protected] .` to easily disable lint errors per rule - New potential a11y issues can be prevented with these lint rules! Note: In a follow up PR, I'll address the low hanging issues in WB and create tickets for more complex issues that were found A breakdown of the existing jsx-a11y errors: <img width="1027" alt="wb-linting-errors-before" src="https://github.com/user-attachments/assets/cbe4be7f-6868-48af-8474-bad0297cf8b2" /> Implementation Plan: - Khan/wonder-stuff#1114 Set up eslint a11y config and enable aphrodite-add-style-variable-name - Use this new config in WB, perseus, webapp and address lint errors from the aphrodite-add-style-variable-name rule - #2459 - Khan/perseus#2193 - Khan/webapp#29212 - Khan/wonder-stuff#1118 Update the eslint-config/a11y config with the config based on the accessibility linting rules ADR - (includes this PR) Use the updated config in projects and address existing errors by disabling the rules per line. Issue: FEI-1133 ## Test plan: - Run `pnpm lint` and make sure there are no linting errors Author: beaesguerra Reviewers: beaesguerra, jandrade Required Reviewers: Approved By: jandrade Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Chromatic - Build and test on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2471

## Summary: This work is part of the implementation of [ADR#781 Enabling more lint rules for accessibility](https://khanacademy.atlassian.net/wiki/x/IoBVyg) These changes include: - Updating `@khanacademy/eslint-config` to the latest version to include new jsx-a11y configuration and settings - Updating `esling-plugin-jsx-a11y` to the latest version so it is up to date - Addressed existing errors by adding `eslint-disable-next-line` - View the PR commits for a breakdown of errors by rule - Used `npx [email protected] .` to disable lint errors per rule - Provided custom component mapping for jsx-a11y in the eslint config since Perseus has custom components New potential a11y issues can be prevented with these lint rules and existing errors can be addressed over time by the team! The lint error messages have a link to the [rule docs](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/main/docs/rules) for more details! A breakdown of the existing jsx-a11y errors: <img width="1019" alt="perseus-linting-errors-before" src="https://github.com/user-attachments/assets/8191e551-4147-43cb-bbcf-79b0e5684c26" /> ### Implementation Plan: - Khan/wonder-stuff#1114 Set up eslint a11y config and enable aphrodite-add-style-variable-name - Use this new config in WB, perseus, webapp and address lint errors from the aphrodite-add-style-variable-name rule - Khan/wonder-blocks#2459 - #2193 - Khan/webapp#29212 - Khan/wonder-stuff#1118 Update the eslint-config/a11y config with the config based on the accessibility linting rules ADR - (includes this PR) Use the updated config in projects and address existing errors by disabling the rules per line. Issue: FEI-1133 ## Test plan: - Run `yarn lint` and make sure there are no linting errors Author: beaesguerra Reviewers: catandthemachines, beaesguerra, #frontend-infra-web Required Reviewers: Approved By: catandthemachines Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2232

beaesguerra added 7 commits February 5, 2025 14:33

[fei-1133] update eslint-plugin-jsx-a11y

538d1b7

[fei-1133] configure rules and settings for jsx-a11y based on poc htt…

2fd4d22

…ps://github.com/Khan/webapp/pull/26840/files#diff-20e55c78ab86b0eb87c6f756500b87f451b52fab251d6f327ce572ec88e8ae3b and ADR: https://khanacademy.atlassian.net/wiki/x/IoBVyg

[fei-1133] only include component mapping for component names that fo…

49d237e

…llow the naming convention StyledElement

[fei-1133] add more component mapping

758f8e3

[fei-1133] use constants for error/off values

3892023

[fei-1133] Update documentation

e5ff416

[fei-1133] docs(changeset): Enable accessibility linting rules from `…

3529c9f

…eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y`

beaesguerra self-assigned this Feb 5, 2025

[fei-1133] organize rules

c58d9e2

beaesguerra commented Feb 5, 2025

View reviewed changes

beaesguerra marked this pull request as ready for review February 5, 2025 23:39

khan-actions-bot requested a review from a team February 5, 2025 23:39

beaesguerra requested a review from kevinb-khan February 5, 2025 23:41

marcysutton approved these changes Feb 7, 2025

View reviewed changes

kevinb-khan approved these changes Feb 8, 2025

View reviewed changes

beaesguerra added 2 commits February 10, 2025 15:26

[fei-1133] Add polymorphicAllowList. Was running into false errors wi…

18851e2

…th AccordionSectionHeader tag prop in WB

[fei-1133] Add SingleSelect and MultiSelect mapping after testing wit…

9d50ac1

…h projects

beaesguerra requested a review from marcysutton February 10, 2025 23:42

marcysutton reviewed Feb 11, 2025

View reviewed changes

marcysutton approved these changes Feb 11, 2025

View reviewed changes

beaesguerra merged commit cd91b93 into main Feb 11, 2025
8 checks passed

beaesguerra deleted the fei-1133 branch February 11, 2025 21:24

beaesguerra mentioned this pull request Feb 11, 2025

Enable jsx-a11y eslint rules Khan/wonder-blocks#2471

Merged

beaesguerra mentioned this pull request Feb 12, 2025

Enable jsx-a11y eslint rules Khan/perseus#2232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable rules from `eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y` #1118

Enable rules from `eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y` #1118

beaesguerra commented Feb 5, 2025 •

edited

Loading

changeset-bot bot commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited

Loading

beaesguerra Feb 5, 2025

khan-actions-bot commented Feb 5, 2025

marcysutton left a comment

kevinb-khan left a comment

kevinb-khan Feb 8, 2025

beaesguerra Feb 10, 2025

kevinb-khan Feb 11, 2025

beaesguerra Feb 11, 2025

kevinb-khan Feb 8, 2025

beaesguerra Feb 10, 2025

beaesguerra commented Feb 10, 2025

marcysutton Feb 11, 2025

marcysutton left a comment

Enable rules from eslint-plugin-jsx-a11y in @khanacademy/eslint-config/a11y #1118

Enable rules from eslint-plugin-jsx-a11y in @khanacademy/eslint-config/a11y #1118

Conversation

beaesguerra commented Feb 5, 2025 • edited Loading

Summary:

Test plan:

changeset-bot bot commented Feb 5, 2025 • edited Loading

🦋 Changeset detected

codecov bot commented Feb 5, 2025 • edited Loading

Codecov Report

github-actions bot commented Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

khan-actions-bot commented Feb 5, 2025

Gerald

marcysutton left a comment

Choose a reason for hiding this comment

kevinb-khan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beaesguerra commented Feb 10, 2025

Choose a reason for hiding this comment

marcysutton left a comment

Choose a reason for hiding this comment

Enable rules from `eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y` #1118

Enable rules from `eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y` #1118

beaesguerra commented Feb 5, 2025 •

edited

Loading

changeset-bot bot commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited

Loading