-
-
Notifications
You must be signed in to change notification settings - Fork 449
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(lint/useExportType): add rule #29
Conversation
See the ongoing discussion in rome#4751. |
d987fd4
to
06470f1
Compare
06470f1
to
afcdb41
Compare
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
afcdb41
to
4f73eb0
Compare
4f73eb0
to
a3b1f33
Compare
a3b1f33
to
69b7d7e
Compare
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
69b7d7e
to
292f9db
Compare
This is already done by https://biomejs.dev/linter/rules/use-grouped-type-import/ , do we need the fix in this rule too? Or, is there something I am missing? |
useGroupedTypeImport factorize We discussed it in the corresponding rome issue. We could implement two rules and deprecate useGroupedTypeImport:
Alternatively, we could perform the factorization of I am a bit shared about which approach to choose. We could even adopt both. |
Sorry I made a mistake, thank you for clarifying. I asked because today my team needed Is there an issue to track |
No, once this PR merged, I will start working on. Have you any thoughts about which approach to use? |
I think the best course of action is to use a lint rule, mainly because there's a reason if TypeScript decided to do this: import {type something} from "something.js" import {} from "something.js"; Hence, if there's a case where users need to have this case enabled, they should be free to turn it off using a suppression comment. |
142a6ff
to
1c41acb
Compare
1c41acb
to
18f498e
Compare
I think this doesn't hold for exports. Thus, I decided to factorize type-only exports when possible. Ready for review. |
18f498e
to
8daabba
Compare
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/semantic_analyzers/nursery/use_export_type.rs
Outdated
Show resolved
Hide resolved
|
||
i Safe fix: Use inline type exports. | ||
|
||
18 │ export·{·type·Interface,·type·TypeAlias,·Enum,·func·as·f,·Class·}; |
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.
Can a bundler really three shake this? Or can typescript really do that?
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.
I think so. Since the export is declared as a type, the bundler/compiler knows that the entity is not used a runtime. Thus, it can remove this specific export. Otherwise, it has to check if the definition is a type. Many bundlers, such as ESbuil assume that you use the isolated module mode because they are not able to do multi-file analysis. Thus, they are not able to understand when an imported value is only a type. Requiring type-only exports and imports help them to detect types and runtime values.
8daabba
to
53d25fb
Compare
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.
Let's 🚢 it, people want it!
Summary
This rule implements some part of consistent-type-exports.
In contrast to the ESLint rule:
we use
export { type }
by default. ESLint provides an option for this and split an export between a value export and aexport type
.For example, given the following code:
ESlint proposes the following fix:
While this rules proposes the following fix:
The ESLint rule is aware of which re-export name is a type. Biome has no information on re-exported names.
ESLint doesn't factorize (group) an export that contains only type-only exports.
For example, Biome proposes the following fix, while ESLint doesn't:
Test Plan
Tests included.