jsx-eslint/eslint-plugin-react

[Feature Request]: Add TypeScript types

segevfiner opened this issue · 10 comments

segevfiner commented

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

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

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,
);
  • 👍1
ljharb commented

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

voxpelli commented

Here's a PR that adds this through type generation based on the JSDocs: #3830

  • 👀2
JstnMcBrd commented

@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

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

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

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

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

I'll open a separate issue for this.

  • 👍1