Skip to content

fix(types): fix types of flat configs #3882

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CHC383
Copy link

@CHC383 CHC383 commented Jan 25, 2025

Fixes #3878

This is another attempt to fix #3878 from a different direction than #3879

@CHC383
Copy link
Author

CHC383 commented Jan 25, 2025

@ljharb Since #3879 is blocked, I tried to fix the typing issue from a different angle: creating a new flatConfigs object outside of the original configs. This eventually will become a breaking change but not at the moment, as the original configs.flat is still kept and only marked as deprecated through JSDoc.

If we run out of ideas on #3879, maybe we can go with this one?

Copy link

codecov bot commented Jan 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.71%. Comparing base (e6b5b41) to head (cd3665a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3882      +/-   ##
==========================================
+ Coverage   97.66%   97.71%   +0.04%     
==========================================
  Files         136      133       -3     
  Lines        9978     9971       -7     
  Branches     3703     3703              
==========================================
- Hits         9745     9743       -2     
+ Misses        233      228       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2025

I agree this is semver-minor, but I have no desire to go with that model. I want a single configs object with eslintrc configs, and a flat on it that holds all the flat configs. If the only unsolvable issue with that is that you might need to use NonNullable<> or !, I think that's acceptable (i don't think it's unsolvable, to be clear, but i'm not willing to use what i think is a worse object layout)

@CHC383
Copy link
Author

CHC383 commented Jan 25, 2025

I agree this is semver-minor, but I have no desire to go with that model. I want a single configs object with eslintrc configs, and a flat on it that holds all the flat configs. If the only unsolvable issue with that is that you might need to use NonNullable<> or !, I think that's acceptable (i don't think it's unsolvable, to be clear, but i'm not willing to use what i think is a worse object layout)

Yeah you are right, I just checked and found out ESLint supports ts config since v9.9.0, before that NonNullable<> or ! are not an option, I guess it is not a big deal then compared to using // @ts-expect-error in the js config.

And just curious, you are also the main maintainer of eslint-plugin-import and eslint-plugin-jsx-a11y, why is this plugin choosing this layout while the other two using the layout similar to the PR? TBH this is the only plugin among the few popular ones I used that use this layout, others either have flatConfigs as a separate object, or have the flat configs in the same level as the legacy configs (all inside the same configs object) and prefix the name with flat.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2025

It's a fair point that they're not all the same; this, however, is what i would have preferred to do on the other two, i just didn't notice the difference.

Prefixing means you can't programmatically compare them, for example - munging string names isn't clean at all. A separate object, at least, is more reasonable, but configs is the only property under which i'd expect to find configs.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[enhancement]: flat configs are possibly undefined
2 participants