ArnaudBarre / eslint-plugin-react-refresh

Validate that your components can safely be updated with Fast Refresh
MIT License
206 stars 13 forks source link

Bug: ts declaration types are not shipped with the package #38

Closed valerii15298 closed 7 months ago

valerii15298 commented 8 months ago

When I am trying to use it in flat config I get this error: Could not find a declaration file for module 'eslint-plugin-react-refresh' But your package is build with typescript so you can just emit declaration files to the distributed files in order for others to use them.

Screenshot 2024-02-26 at 01 58 52
ArnaudBarre commented 8 months ago

I can do that, but I'm quite unhappy is the low guidance of ESLint core for that. What are types you using to type your config? I don't think this is worth it adding a dependency on @types/eslint or a subpackage of @typescipt-eslint, so maybe having the module typed as { "only-export-components": any } would be a start, but that's bad that you can't get types for the parameters

valerii15298 commented 8 months ago

@ArnaudBarre I completely agree. The thing is I am using ES modules. Probably with commonjs your current types are correct. But with ES Modules there are some breaking changes. Basically typescript with ES Modules does not work with namespaces any more. You can check out this blogpost: https://devblogs.microsoft.com/typescript/typescripts-migration-to-modules/ You can check out my project here: https://github.com/valerii15298/int20h-2024/blob/main/api/eslint.config.js It is a small one, so you can clone install deps and see that there is no any autocomplete for all eslint plugins... Maybe I am doing something wrong though...

I also can create separate reproduction repo specifically for it if you want :)

ArnaudBarre commented 8 months ago

I'll look at it, if you find any plugin that does things right, please share!

ArnaudBarre commented 7 months ago

So I've look at it and both the types shipped by @types/eslint and TSESLint in https://github.com/typescript-eslint/typescript-eslint/pull/7273 makes it totally useless to ship types for plugin, there is absolutely no validation on plugin name or options. This is sad. This is one of the reason I'm working on a new linter. Until ESLint takes really care about typing of their config, I considered this out of scope.

valerii15298 commented 7 months ago

@ArnaudBarre can you share which linter are you working on? I heard that Biome is quite promising...

ArnaudBarre commented 7 months ago

I don't have anything ready for now, I've worked on something on Christmas/January but I have still some rules I want to implement before making the project public. Here are some tweets I made about it:

Biome and OXC linter are two promising projects, but the lack of custom rules and type aware rules is a deal breaker for me.

A project in the same idea but IMO with a public API too low level is https://github.com/johnsoncodehk/typescript-linter

pauliesnug commented 4 months ago

So I've look at it and both the types shipped by @types/eslint and TSESLint in typescript-eslint/typescript-eslint#7273 makes it totally useless to ship types for plugin, there is absolutely no validation on plugin name or options. This is sad. This is one of the reason I'm working on a new linter. Until ESLint takes really care about typing of their config, I considered this out of scope.

@ArnaudBarre The plugin is already written in TypeScript... would it hurt to just include an index.d.ts to make life easier? There are some ESLint addons that add type validation for rules/rule names, like ESLint define config etc. If not, is it alright if I just make a fork?

ArnaudBarre commented 4 months ago

This is not as simple as run tsc --emitDeclaration because eslint is so averse to TS. If you find a way to add typing to this without adding megabytes of dependency, a PR is welcomed.

aryaemami59 commented 2 weeks ago

@ArnaudBarre If you're still open to this, I would love to file a PR to add type definitions.

ArnaudBarre commented 2 weeks ago

I'm open to a PR as long as it doesn't pull a big dependency (or is added as peer dep)

aryaemami59 commented 2 weeks ago

@ArnaudBarre Sounds good I'll get started on it.