Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for eslint-define-config #23

Closed
Shinigami92 opened this issue Jan 15, 2024 · 4 comments
Closed

Support for eslint-define-config #23

Shinigami92 opened this issue Jan 15, 2024 · 4 comments

Comments

@Shinigami92
Copy link

Right now, eslint-plugin-i does not support eslint-define-config or typescript types at all.
Does eslint-plugin-i want to support it natively? (This would require shipping .d.ts files), or should I add https://github.com/eslint-types/define-config-plugin-types/tree/main/types/i (alongside https://github.com/eslint-types/define-config-plugin-types/tree/main/types/import)?

Coming from vitejs/vite#15569

@JounQin
Copy link
Member

JounQin commented Jan 15, 2024

Thanks @Shinigami92! ❤️

I'd love to definitely.

But I also have some questions

  1. Do you mean to re-export @eslint-types/import as typings at this repository? It seems we can't, the plugin name is hard coded.
  2. If not, should we mark @eslint-types/import as dependency for reusing some types? Or we just write ourselves?
  3. Do you want to maintain it here? It would be great if you can join the org or project.

@Shinigami92
Copy link
Author

@JounQin could you make a review of eslint-types/define-config-plugin-types#42
I just mostly copied over the files from <root>/types/import and replaced import/ with i/ as well as the github links in the settings type.
Then I ran pnpm run preflight to generate the rest via script.

Can you especially check the settings? Also why do the rules (<root>/types/i/types.d.ts) contain links to https://github.com/import-js/eslint-plugin-import instead of https://github.com/un-es/eslint-plugin-i?

@JounQin
Copy link
Member

JounQin commented Jan 15, 2024

So you've decided to not maintain the typings here right? I have a different opinion on this considering flat config support later and there will be no requirements to use:

/// <reference types="@eslint-types/import" />

I didn't view all through but it should just be fine, because the most functionalities are all the same, ecpect modern deps.

Also why do the rules (/types/i/types.d.ts) contain links to https://github.com/import-js/eslint-plugin-import instead of https://github.com/un-es/eslint-plugin-i?

Because this fork aims to be an alias actually. Imagine a user have a legacy ESLint config using eslint-plugin-import inside, and in project root level it's using eslint-plugin-i, there will be duplicate deps: import + i, this is what I don't want.

The recommended usage is always:

npm install -D eslint-plugin-import@npm:eslint-plugin-i@latest

See also import-js/eslint-import-resolver-typescript#248


Besides, I remembered that, the setting prefix is still import due to strategy as above. Please check #17.

@Shinigami92
Copy link
Author

This MR can be closed. I'm thinking of deprecating eslint-define-config in favor of tseslint.config (https://typescript-eslint.io/getting-started/#step-2-configuration)

@Shinigami92 Shinigami92 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants