-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
fix: Remove type-only import x = y.z
#17025
fix: Remove type-only import x = y.z
#17025
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58578 |
ReferencedIdentifier(path, state) { | ||
if (t.isTSQualifiedName(path.parent) && path.parent.right === path.node) { | ||
return; | ||
} | ||
if (path.parentPath.isTSImportEqualsDeclaration()) return; | ||
state.references.push(path); | ||
}, |
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 didn't use childPath.skip()
to make it easier to re-land #16923 in the future.
@@ -1,2 +1 @@ | |||
import * as joint from '@joint/core'; | |||
export var JGraph = joint.dia.Graph; |
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 wrong, what if joint.dia.Graph
is a value and not a type?
import * as x from "x";
export import foo = x.foo;
should behave the same as
import { foo } from "x";
export { foo };
Failure looks relevant :) |
How does this PR interact with the |
Oh I see. It seems like TypeScript simply does not remove it when the equivalent TS option ( |
This seems a bit dangerous and I'm not sure if this will cause a regression.