eslint-plugin-jsx-a11y
eslint-plugin-jsx-a11y copied to clipboard
feat: support new config system
Supporting eslint's new config system of eslint.
Note that the legacy config system always has require()d plugins and sharable configs, whereas the new system is available with both of ESM and CJS.
Thus conditional export is introduced to keep compatibility while providing a new config/plugin at the same time.
plugin: protocol(e.g. plugin:react-hooks/recommended) is not valid any more. Thus the new plugin doesn't have configs.
Fixes #978.
Codecov Report
Merging #891 (fe5bb49) into main (7b3492b) will increase coverage by
0.00%. The diff coverage isn/a.
:exclamation: Current head fe5bb49 differs from pull request most recent head 3e48857. Consider uploading reports for the commit 3e48857 to get more accurate results
@@ Coverage Diff @@
## main #891 +/- ##
=======================================
Coverage 99.29% 99.29%
=======================================
Files 104 103 -1
Lines 1555 1571 +16
Branches 523 514 -9
=======================================
+ Hits 1544 1560 +16
Misses 11 11
However, why can't the existing "main" Just Work with the new config system? What's in
legacy.jsthat's different fromindex.js?
I have never said the existing "main" just doesn't work. It works because the only difference is the existence of configs property, and the new config system would ignore it when the plugin object still has it. But my point was that it's not the intended specification, and can confuse users, leading to weird usage.
I'm not sure why a user would be confused, and if they are, I think that's on eslint to fix, not individual plugins in the ecosystem.
I'm not sure why a user would be confused, and if they are, I think that's on eslint to fix, not individual plugins in the ecosystem.
Okay. I respect that. I'd patch as long as this is resolved.
Pushed a new commit :)
why do you need to bring in your own globals? isn't that what env is for?
hmm, i'm having second thoughts on the files config. @jjangga0214, if files: ['**/*.{js,mjs,cjs,jsx,mjsx,ts,tsx,mtsx}'] is included by default in the legacy config system, will that break anything for them?
if files: ['**/*.{js,mjs,cjs,jsx,mjsx,ts,tsx,mtsx}'] is included by default in the legacy config system, will that break anything for them?
Well, my opinion is yes, if so. But we know the assumption is not true, so what did make you think about the imagination?
I didn't understand your reply.
What I realized is that it's critical for all of these extensions to be included, and highly unlikely devs will remember to include them all, so it's valuable to provide them by default. However, I want the plugin object to be able to be usable in both config systems.
It seems there's confusion between us?
if files: ['**/*.{js,mjs,cjs,jsx,mjsx,ts,tsx,mtsx}'] is included by default in the legacy config system,
To my understanding, files: ['**/*.{js,mjs,cjs,jsx,mjsx,ts,tsx,mtsx}'] is NOT included by default in the legacy config system. Am I wrong?
@jjangga0214 what i mean is, in the legacy config, is the presence of a "files" property problematic? or is it ignored?
I don't understand the point. Legacy config should not be used in the new config. They're just different and incompatible. The exception is when the legacy config only has .rules.
Other fields like .plugins(conflict. Legacy: string[], New: PluginObject[]),.extends(ignored), .overrides (ignored), .parser (ignored), .parserOptions (ignored), .env (ignored) and more, are incompatible. For almost all plugins in the ecosystem, including eslint-plugin-jsx-a11y, their presets have at least .plugins or .extends.
So I still don't grasp what your concern is. I think I may understand if you show me an example of your thoughts.
"Different and incompatible" is what I'm trying to confirm.
If in fact the same object can't serve in both configs, then that's a massive design mistake on eslint's side - and I'd prefer to impose the cost on the users of the new config, since the legacy config is the most important (and will remain so for a good number of years).
I'd prefer to impose the cost on the users of the new config
Does this mean you're not supposed to support the new config system, which also means you'd not merge this PR?
@jjangga0214 oh no, not at all! it just means the main entrypoint of the package will be the legacy config, and flat config users will need to import, eg, eslint-plugin-jsx-a11y/flat or something.
However, if the same object can be used in both config systems, then the main entrypoint can support both, which is the ideal.
I've just rebased this, and if tests pass, and we can confirm that the main entrypoint works in both configs, then it'd be good to go :-)
@ljharb is this being maintained? the last update was four months ago when you said it would be merged
@pauliesnug of course it is. I'm waiting for confirmation that the plugin works in both config formats.
@ljharb @pauliesnug
oh no, not at all! it just means the main entrypoint of the package will be the legacy config, and flat config users will need to import, eg, eslint-plugin-jsx-a11y/flat or something.
Okay, I now understand it.
if the same object can be used in both config systems,
Unfortunately, no.
https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/1adec3517fc2c9797212ca4d38858deed917e7be/src/index.js#L47-L54
For example, as I've said before, plugins as string[] is incompatible, and parserOptions should be under languageOptions. The only compatible property is rules.
Note that the spec of plugin itself(not config) is compatible, by the way.
So in conclusion, we should provide a different object by a different entry point.
Any idea about the name of it?
(You already mentioned eslint-plugin-jsx-a11y/flat. Is it a suggestion or just an example?)
Just an example, but yes, it'd need to be some other entry point.
Any idea about the name of it?
FWIW 'flat/recommended' is what eslint-plugin-jsdoc names it https://www.npmjs.com/package/eslint-plugin-jsdoc#flat-config
+1 on this. This would be useful for importing recommended/strict settings via CLI (e.g. eslint --config node_modules/eslint-plugin-jsx-a11y/recommended.js). Which is currently impossible since the current index.js contains additional configurations.
Is this still being worked on? Otherwise I can always start a PR
@soerenmartius please don't open a new PR; if you want to rebase this PR and keep working on it, please comment a branch link and i'll pull in your changes.
@soerenmartius please don't open a new PR; if you want to rebase this PR and keep working on it, please comment a branch link and i'll pull in your changes.
Thanks - I am on PTO this and next week but will try to contribute once I am back unless you have time to finish this until then! Thanks
Is this still being worked on? I might be able to help out, if nobody's working on it. Really eager for the v9 & flat config support.
@michaelfaith if you have updates, please comment a link to a branch or sha (not a new PR) and i'll pull in the changes.
@ljharb I know you mentioned not creating a new PR, but I went in a totally different direction with this, in order to address some of your concerns, so I opened a new PR: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/pull/993
Totally cool if you'd rather bring those changes into this PR, but figured I'd leave that up to you. (But note, this PR shouldn't close the v9 issue, since it's just focused on flat config support). With my changes, we can use the same main entry point for both legacy and flat experiences, without the need for a separate entry point. This was inspired by how the jsdoc and a couple of other plugins have provided support for both.
I also added a new examples folder in the root that has three different (vite-based) React apps using the built plugin, testing usage in both legacy (eslintrc) and flat (esm and cjs) configs.
Closing in favor of #993.