jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.99k stars 2.77k forks source link

[Feature Request]: Add TypeScript types #3797

Closed segevfiner closed 1 month ago

segevfiner commented 2 months ago

Is there an existing issue for this?

Description Overview

It is possible to have the ESLint config be type checked using JSDoc comments and the // @ts-check annotation comment: e.g.

// @ts-check

/** @type {import("eslint").Linter.Config[]} */
export default [
    // ...
]

But this package doesn't export types ATM. It would be nice if it did export types so it works without errors using this. Most/all other plugins I used do have types.

Expected Behavior

For the package to have TypeScript types

eslint-plugin-react version

v7.35.0

eslint version

v9.8.0

node version

v18.20.4

eglove commented 2 months ago

typescript-eslint provides a typed wrapper for configs. https://typescript-eslint.io/getting-started

// @ts-check

import eslint from '@eslint/js';
import tseslint from 'typescript-eslint';

export default tseslint.config(
  eslint.configs.recommended,
  ...tseslint.configs.recommended,
);
ljharb commented 2 months ago

I'll probably add types eventually, but in the meantime, a DT package would be the way to go.

eric-net commented 2 months ago

https://typescript-eslint.io/users/configs/

voxpelli commented 1 month ago

Here's a PR that adds this through type generation based on the JSDocs: https://github.com/jsx-eslint/eslint-plugin-react/pull/3830

JstnMcBrd commented 4 weeks ago

@voxpelli thanks for adding this! I submitted a request for types back in #3776, so it's great to see it happen!

One issue though - I updated to v7.37.0, but TypeScript is still complaining there are no type declarations.

image

Are we sure the types are being generated + added to the package build correctly? I noticed there is no "types" field in package.json - could that be the problem? Usually packages declare their type exports like this:

"main": "index.js",
"types": "index.d.ts",

If this is a bug, should we open a new issue for this?

voxpelli commented 4 weeks ago

I noticed there is no "types" field in package.json - could that be the problem

Oh, I forgot to add that? 🫣

Yeah, would be great to add that, can you validate that it works when that's been added?


Though I did add a test that should have validated that the types are working properly when used in a module, is that test then broken? 🤔

JstnMcBrd commented 4 weeks ago

can you validate that it works when that's been added?

I would, but I can't find the type file to point to - there's no index.d.ts. There is a lib/types.d.ts file, but it doesn't seem to define the export types, and it hasn't been modified in 6 months.

Here is what I see in my node_modules.

image

It's possible the type declarations are being excluded when publishing to npm. Or maybe I just don't know where to look.

You can see what has been published by browsing the package files here. Do you see your type files?

voxpelli commented 4 weeks ago

Looks like the building of the types and the publishing failed: https://github.com/jsx-eslint/eslint-plugin-react/actions/runs/11062995632/job/30739408890

Yet it did publish 🤔

ljharb commented 4 weeks ago

I published manually, locally. The types built fine, afaik.

Indeed, the publish workflow failed and I couldn't reproduce that locally.

If there's a bug then it'd be great to get that fixed.

JstnMcBrd commented 4 weeks ago

I'll open a separate issue for this.