jsx-eslint / eslint-plugin-jsx-a11y

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

Add support for ESLint 9 #978

Closed jacobcarpenter closed 6 days ago

jacobcarpenter commented 5 months ago

ESLint 9 is released, but it looks like eslint-plugin-jsx-a11y does not yet support it.

There are breaking API changes: https://eslint.org/docs/latest/use/migrate-to-9.0.0#breaking-changes-for-plugin-developers

There is also a new default config format ("flat config") that probably also needs some adjustment in the exported recommended rules; though, support for flat config could be considered a separate issue.

ljharb commented 5 months ago

Yes, that's right.

In eslint 9, RuleTester only supports flat config, so it makes things more complicated.

I think we need flat config support before we have eslint 9 support.

ljharb commented 5 months ago

See #891.

guswnsxodlf commented 3 months ago

I'm using the plugin with the flat config. Hope it fully supports eslint flat config asap! 👍🏻

import pluginJsxA11y from 'eslint-plugin-jsx-a11y';

export default [
  ...
  {
    plugins: {
      'jsx-a11y': pluginJsxA11y,
    },
    rules: pluginJsxA11y.configs.recommended.rules,
  },
  ...
];
Scc33 commented 2 months ago

Any update on adopting this?

ljharb commented 2 months ago

993 adds support for flat config, so in theory, we're now unblocked from supporting eslint 9.

pauliesnug commented 2 months ago

In addition to ESLint v9 support, we should add TypeScript definitions in the package for typechecking the plugin.

ljharb commented 2 months ago

@pauliesnug that's got nothing to do with eslint 9. we could certainly do that - to help plugin developers, but it seems like you mean so that users have types for the flat config objects?

pauliesnug commented 2 months ago

@pauliesnug that's got nothing to do with eslint 9. we could certainly do that - to help plugin developers, but it seems like you mean so that users have types for the flat config objects?

Yes, and for importing the plugin itself.


// @ts-check

// Not only does this throw a typing error, but also a default import error
import jsxA11y from 'eslint-plugin-jsx-a11y';

export default [jsxA11y()];
yuheiy commented 2 months ago

Since eslint@9 was not included in peerDependencies, I submitted a PR (https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/pull/994) to fix it.

ljharb commented 2 months ago

@yuheiy as i commented there; that's nowhere near sufficient to get this support shipped, but please leave the PR open.

ljharb commented 2 months ago

@pauliesnug please note that TS's module system is horrifically broken unless you enable synthetic imports and esModuleInterop - you shouldn't see an issue with the default import that way.

eagerestwolf commented 1 month ago

@pauliesnug that's got nothing to do with eslint 9. we could certainly do that - to help plugin developers, but it seems like you mean so that users have types for the flat config objects?

Yes, and for importing the plugin itself.

// @ts-check

// Not only does this throw a typing error, but also a default import error
import jsxA11y from 'eslint-plugin-jsx-a11y';

export default [jsxA11y()];

You are getting an import error because eslint-plugin-jsx-a11y doesn't ship type definitions. It has absolutely nothing to do with TypeScript's module system, which works perfectly fine so long as library developers ship standard modules with types. Otherwise, yes it misbehaves. To make the import error go away just create a .d.ts file (traditionally, global.d.ts) with the following content.

declare module "eslint-plugin-jsx-a11y";

Note that you will continue to get type errors everytime you do anything with jsxA11y because it's an untyped module. TypeScript doesn't know what it is, so it assumes it's any (which is bad). You're getting a syntax error because what you imported is an object, not a function.

ljharb commented 1 month ago

(if you're going to make type defs, please do so in DefinitelyTyped, so others can benefit)

pauliesnug commented 1 month ago

(if you're going to make type defs, please do so in DefinitelyTyped, so others can benefit)

good idea, i'll do that until they are shipped alongside the plugin

eagerestwolf commented 1 month ago

@ljharb done https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70203. Once the PR is merged, it should now be possible to do the following (and by it should, I mean I am using this):

import { fixupConfigRules } from "@eslint/compat";
import jsxA11y from "eslint-plugin-jsx-a11y";

export default [
  // Other configs here
  ...fixupConfigRules(jsxA11y.flatConfigs.recommended),
];
eagerestwolf commented 1 month ago

@ljharb @pauliesnug If either of you would like to double check the PR and make sure I typed everything correctly, you are welcome to do so.

eagerestwolf commented 1 month ago

On the topic of having a stopgap soon. Would there be interest in assistance in a rewrite to support ESLint 9? I will say if I would be the one to really help push this forward, I have some ideas as to how that would be accomplished. First, the plugin should probably be written in TypeScript using @typescript-eslint/utils which already has native support for JSX (and typed linting as well, not that it's really super necessary for this project), thus negating the need for the custom JSX library. This would also allow dropping the external types repo, since you could just ship your own types. Additionally, a major rewrite (which would almost certainly result in a V7 release) would be a great time to remove any already deprecated rules. This is how I would tackle the rewrite; but it's your project. I will just say if it's going to in pure JS (or worse Flow), I have no interest in helping.

michaelfaith commented 1 month ago

For what it's worth, I would also be interested in helping with a TS re-write.

ljharb commented 1 month ago

No, I'm not interested in a rewrite, to use TS or anything else :-) TS types can and should be added with jsdoc comments and d.ts files, and a full rewrite just wouldn't add much value.

controversial commented 1 month ago

I’d be interested in using a rewrite of this plugin leveraging typescript-eslint, fwiw, even if it were to become a separate project

jacobcarpenter commented 1 month ago

Honest question: what benefit are you hoping to receive from a rewrite of an eslint plugin from JS with TS JSDoc typing to full Typescript?

As a consumer of the plugin, I don't think you would see any difference.

eagerestwolf commented 1 month ago

@controversial I am considering the idea of a fork (possibly working with the people behind react-eslint).

@jacobcarpenter it's a few things. First, using TypeScript reduces bugs since you aren't passing arbitrary things around. Second, it makes things easier to work on (and contribute to) for those who aren't intimately familiar with the underlying topic (JSX AST parsing in this case). Third, the TypeScript compiler (with the proper flags enabled) can (not necessarily does) output more optimized JavaScript code than a human. Fourth, it makes the code more extensible (and makes the types more reliable). Fifth, TypeScript allows developers to "cherry pick" bits and pieces of code. For example, one could create a plugin (e.g. eslint-plugin-antfu) that pulls rules from plugins to create their own plugin. Finally, ESLint itself is considering a rewrite in TypeScript and/or Rust and adding TypeScript config files.

Think of JavaScript and TypeScript like C++ and Rust. In the case of C++ and Rust, the main issue is memory safety. With JavaScript and TypeScript, the difference is type safety (both JavaScript and TypeScript are memory safe).

jacobcarpenter commented 1 month ago

1 and 2 aren't actually improved by Typescript over TS-compatible JSDoc syntax. You can typecheck the code using tsc if you set it up correctly: https://www.typescriptlang.org/docs/handbook/intro-to-js-ts.html

This post by Rich Harris about SvelteKit's decision to move from full-TS to JSDoc annotated typechecked JS is another good source of food for thought on the subject.

I'd love to see an example of 3! In my view having the package code match the actual source is a benefit over using tsc-emitted code (which complicates debugging/sourcemapping), but I'd love to see some compelling compiler optimizations!

I'm not clear on how 4 and 5 are improved by TS over JS.

It's also important to note that, even when using pure TS, if you're authoring a library that can be consumed by plain JS code, your types are not guarantees like they would be in your Rust example. Even between pure-TS projects, different tsconfig settings around nullability and other options can easily break/violate some clearly asserted type properties.

ljharb commented 1 month ago

Lots of those points are wrong, including that TS reduces bugs (and a full rewrite almost always introduces way more bugs than it prevents), but either way it’s all off topic for this issue. Please file a separate one if you want to continue discussing it.

eagerestwolf commented 1 month ago

You are correct in that sense, but its ultimately a personal preference thing. I guess the main differentiation for me is developer experience, whereas yours is user experience. There is nothing wrong with JSDoc commented pure JS, except that it causes headaches for TS users because TS really doesn't like untyped things. Ultimately, an ideal solution is probably both good JSDoc comments and TS. JSDoc allows JS users to get the advantages of type awareness (not the same as type checking), while TS users get their type safety.

Yes, you are correct that there are some TS options that negate some (not all, don't conflate the 2) type safety features. However, that's intended. One of the nice things about TS is that you, as the developer, decide how you consume and compile your code.

As for Svelte, another reason for not using TS (especially in very large projects) is TS's compiler is not fast (and anyone who says it is is lying to you). That's not the only reason Svelte is going pure JS though. That move is also part of Svelte's push to embrace standards as they are, and not to meddle with them like other frameworks do. That is once of the things that made Svelte popular, and why it's still so loved.

Ultimately, we can agree to disagree. However, there is another alternative to pure TS and pure JS. You could simply pass your pure JS into the TS compiler and it will spit out as .d.ts file using your JSDoc annotations to infer types. With that said, I can tell this conversation is going nowhere. We are running in circles. The people who work on this project don't like TS, and that's fine. However, there are many of us that do. You can continue to work in JS and Flow, and if the community decides to create a TS fork, the community decides to create a TS fork. That's the beauty of open source.

This will be my last comment on this issue. I see no point in discussing it further.

ljharb commented 1 month ago

Consumers can’t tell the difference between a TS project, a JS project with JSdoc types, or a typeless project with DT types.

The contributor experience for the first two is largely identical as well.