-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Make NodePath<T | U>
distributive
#16439
Make NodePath<T | U>
distributive
#16439
Conversation
0fe41a6
to
c62c6d6
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56735 |
@@ -43,7 +43,7 @@ class NodePath<T extends t.Node = t.Node> { | |||
this.scope = null; | |||
} | |||
|
|||
declare parent: t.ParentMaps[T["type"]]; | |||
declare parent: t.Node; |
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.
Does parent path type narrowing still work in this PR?
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.
Yes, because it's properly defined further down in the NodePath
interface.
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.
Amazing!
I've tried something similar before and failed. :)
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.
Awesome work.
Fixes #1, Fixes #2
This pull request makes it so that the types
NodePath<t.Identifier | t.StringLiteral>
andNodePath<t.Identifier> | NodePath<t.StringLiteral>
are equivalent. This makes ourpath.is*
able to better perform type narrowing:It also makes #16430 slightly faster, because it reduces the number of possible
NodePath<...>
types (becauseNodePath<t.Identifier> | NodePath<t.StringLiteral>
andNodePath<t.Identifier | t.StringLiteral>
are now the same type).Unfortunately it makes it more common for
.ensureBlock()
to not work, sinceasserts
return types do not work on unions (microsoft/TypeScript#44212). I consider this acceptable, sincepath.is*
are much more common.