Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[@babel/traverse] Improve performance of mapped Visitor type #69522

Merged
merged 1 commit into from
May 21, 2024
Merged

[@babel/traverse] Improve performance of mapped Visitor type #69522

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion types/babel__traverse/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ export class Binding {
export type Visitor<S = unknown> =
& VisitNodeObject<S, Node>
& {
[Type in Node["type"]]?: VisitNode<S, Extract<Node, { type: Type }>>;
[N in Node as N["type"]]?: VisitNode<S, N extends { type: N["type"] } ? N : never>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can think of a couple of reasons; one is just that you've switched (I think?) the mapped type to be homomorphic by specifically doing N in Node as N["type"]; this may actually end up being a behavior change just due to how these mappings work...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No obvious outward differences jumped out at me between the two kinds of mapped types at the bottom of this little playground (albeit it's hard to see the whole picture) besides the aforementioned speed. Can you think of what the downstream behavioural changes may be? If so I'm happy to investigate a bit further.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest and say that I'm not an expert here; @weswigham would probably know how to better reason about this as I'm more or less parroting off of others 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract is distributive, so operates over every member of the Node passed in - so for every key in Node["type"] previously it was iterating over every Node type to find and select the desired type (exponential behavior). Now, instead, for each member of Node, we only create a single property and execute a single conditional over that member. We're just iterating over less.

}
& {
[K in keyof t.Aliases]?: VisitNode<S, t.Aliases[K]>;
Expand Down