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

feat: expose RuleConfig etc. #235

Closed
wants to merge 1 commit into from

Conversation

antfu
Copy link
Collaborator

@antfu antfu commented Sep 28, 2023

Would be easier to add custom types

@Shinigami92
Copy link
Collaborator

TBH I'm not a fan of this 🤔
eslint-define-config is not meant to be a dependency to define custom rules, and originally there are even typescript types in @types/eslint itself
we just nearly copied these over so we do not have direct dependencies in the src code

Please have a look at https://github.com/DefinitelyTyped/DefinitelyTyped/blob/d6df71f271f38735049115008ff6b5de169c9450/types/eslint/index.d.ts#L893

These should be changed/used originally, but eslint-define-config cant do that, because then it would require a peer dev dependency or fallback to any

@antfu
Copy link
Collaborator Author

antfu commented Sep 29, 2023

Well, it's just a re-export of a few simple types. I see no harm in having them, but I am not blocked by it either. Totally up to you to decide then.

@Shinigami92
Copy link
Collaborator

Well, it's just a re-export of a few simple types. I see no harm in having them, but I am not blocked by it either. Totally up to you to decide then.

It's definitely not that I don't want to export them just to make others life harder
It's more the: that seems to be the wrong way

Either eslint or @types/eslint should export these, so even I could reuse them in this repo

But I'm even the not sure if I e.g. need to put @types/eslint from devDeps to direct deps 🤔
I just don't know what that would do to the project relying on eslint-define-config

Sidenote: I spend today in train, because I drive home
I don't promise to work much this weekend, because I want to relax from the week and spend time for gaming as well as friends

@Shinigami92
Copy link
Collaborator

I could answer yet in #233 yet because I need to write a bit more and prepared for that, but tldr I would love to get rid at all of eslint-define-config
But I think this can be targeted for eslint v9 FlatConfig-only or even eslint v10

A bit like the move from cjs to esm

@Shinigami92
Copy link
Collaborator

@antfu is there any reason now to have exported this? Or did the augmented global properties support solved it?

@antfu
Copy link
Collaborator Author

antfu commented Oct 26, 2023

I found that ESLint exposes that as well. I'm more curious about why you don't use the one from ESLint directly tho. But indeed this is not necessary.

@antfu antfu closed this Oct 26, 2023
@Shinigami92
Copy link
Collaborator

I found that ESLint exposes that as well. I'm more curious about why you don't use the one from ESLint directly tho. But indeed this is not necessary.

I think there types were not so precise, but also I didn't wanted to rely on their types, because then I would need to rely on @types/eslint. Otherwise the types would be broken / fallback to any, if @types/eslint is not installed on the dev's side.
However, @sxzz did use types from eslint here:

import type { ESLint, Linter } from 'eslint';

But I never used FlatConfig yet 🤷, so I don't have knowledge about that if it even works.

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

Successfully merging this pull request may close these issues.

2 participants