jsx-eslint / eslint-plugin-jsx-a11y

Static AST checker for a11y rules on JSX elements.
MIT License
3.42k stars 637 forks source link

Upgrade `axe-core` for more features (it's pinned right now) #989

Closed norgeous closed 5 months ago

norgeous commented 5 months ago

Would it be possible to unpin axe-core dep?

https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/package.json#L82

Currently it is pinned to =4.7.0 in eslint-plugin-jsx-a11y

The latest available is 4.9.1 (at time of writing)

https://github.com/dequelabs/axe-core/blob/develop/CHANGELOG.md

Personally, I am interested in any version above 4.7.1, as that adds support for CSS4 colors such as lch, oklch, lab, oklab

Thanks!

norgeous commented 5 months ago

related: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/792

ljharb commented 5 months ago

The reason we're pinned to v4.7.0 is due to failing tests; see https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/commit/8d8f0169dbaaa28143cf936cba3046c6e53fa134 and https://github.com/dequelabs/axe-core/issues/4127

If indeed v4.9.0 fixed that, then a PR to unpin it would be great!

norgeous commented 5 months ago

It seems the tests (npm run test:ci) are compatible with ^4.9.1 🎉

Screenshot 2024-06-05 143640

Screenshot 2024-06-05 143035

But im using node v22.2.0... does that all seem correct?

Or do i need to run the tests with some other specific node version?

ljharb commented 5 months ago

You'd need to run the tests with node 4 or 6, yes.

norgeous commented 5 months ago

After a bit of dicking about with versions of node, npm and eslint

npm i -g npm@4; npm i -g npm@6; npm -i eslint@3

I did some local testing:

node version npm version eslint version axe-core version test result note
v6.17.1 v6.14.16 3.19.0 =4.7.0 ✅66 same as main branch (just checking)
v6.17.1 v6.14.16 3.19.0 =4.7.1 ❌8 ✅58 TypeError: Object.values is not a function
v6.17.1 v6.14.16 3.19.0 ^4.9.1 ✅66 all passing!

i guess, i will open that PR

ljharb commented 5 months ago

node versions are what matters, not npm versions, but let's look at the tests on the PR :-)

ljharb commented 5 months ago

Fixed in #990.