eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

[Feature Request]: Add TypeScript types

Open segevfiner opened this issue 1 year ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues and my issue is unique
  • [X] 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

segevfiner avatar Aug 08 '24 13:08 segevfiner

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,
);

eglove avatar Aug 08 '24 14:08 eglove

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

ljharb avatar Aug 09 '24 02:08 ljharb

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

eric-net avatar Aug 15 '24 17:08 eric-net

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

voxpelli avatar Sep 23 '24 10:09 voxpelli

@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?

JstnMcBrd avatar Sep 30 '24 16:09 JstnMcBrd

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? 🤔

voxpelli avatar Sep 30 '24 16:09 voxpelli

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?

JstnMcBrd avatar Sep 30 '24 16:09 JstnMcBrd

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 🤔

voxpelli avatar Sep 30 '24 19:09 voxpelli

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.

ljharb avatar Oct 01 '24 00:10 ljharb

I'll open a separate issue for this.

  • #3836

JstnMcBrd avatar Oct 01 '24 02:10 JstnMcBrd