-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin): adding consistent-type-exports
rule
#3936
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(eslint-plugin): adding consistent-type-exports
rule
#3936
Conversation
Thanks for the PR, @dzearing! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
Codecov Report
@@ Coverage Diff @@
## master #3936 +/- ##
==========================================
+ Coverage 92.69% 93.33% +0.64%
==========================================
Files 333 152 -181
Lines 11400 8014 -3386
Branches 3242 2568 -674
==========================================
- Hits 10567 7480 -3087
+ Misses 363 180 -183
+ Partials 470 354 -116
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…ipt-eslint into feat/consistent-type-exports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic, thanks so much! 👏
Just left a few nits. I'll mark as approved for now, but would be great to address them.
Thank you @JoshuaKGoldberg ! I will address. |
…tility, minor nit cleanup
All feedback has been addressed. Thanks again @JoshuaKGoldberg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fantastic to me, thanks @dzearing!
I don't think either of my comments are blocking, but would be nice-to-haves IMO!
Ready for merge, unless there is any other feedback. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quick eyeball looking at some of the type stuff.
Some queries that I'm not sure how your code will handle.
could you please add the following test cases:
const variable = 1;
class Class {}
enum Enum {}
function Func() {}
namespace ValueNS { export const x = 1 }
export {
variable,
Class,
Enum,
Func,
ValueNS,
}
const variable = 1;
class Class {}
enum Enum {}
function Func() {}
namespace ValueNS { export const x = 1 }
export type { // type-export the values
variable,
Class,
Enum,
Func,
ValueNS,
}
type Alias = 1;
interface IFace {}
namespace TypeNS { export type x = 1 }
export {
Alias,
IFace,
TypeNS,
}
type Alias = 1;
interface IFace {}
namespace TypeNS { export type x = 1 }
export type {
Alias,
IFace,
TypeNS,
}
All added. |
…es, and also where namespace is type only but exported as a non type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect - this LGTM!
Just as a note - TS4.5 will add a new feature which allows you to specify the type
declarator as part of the specifier!
type T = 1;
export { type T }
We don't have support for this syntax in our parser yet (we only support RC versions and it's still 4.5-beta), but once we release support we'll have to update this rule to match! I have noted this in the 4.5 tracking issue - #3950
consistent-type-exports
rule
This rule addresses issue #3597:
It will catch exports that are type-only, but don't use
export type
:...and allow you to auto-fix them:
It will also auto separate types from non-types:
This is particularly important when using
esbuild
to transpile a library into a browser-consumable bundle.Open to feedback! I named it something symmetrical with the current
consistent-type-imports
rule so that it would be easy to find, but would be happy to rename to something better.I did not add any options; if there are compelling reasons to add options, please let me know.