-
-
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
perf: Reduce the use of temporary objects #16959
Conversation
printJoin( | ||
nodes: Array<t.Node> | undefined | null, | ||
opts: PrintJoinOptions = {}, | ||
statement?: boolean, | ||
indent?: boolean, | ||
separator?: PrintJoinOptions["separator"], | ||
printTrailingSeparator?: boolean, | ||
addNewlines?: PrintJoinOptions["addNewlines"], | ||
iterator?: PrintJoinOptions["iterator"], | ||
trailingCommentsLineOffset?: number, | ||
) { |
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.
Their order is similar to the frequency of use.
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.
There are quite a few boolean options here, maybe we can pack them into bit flags.
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 was originally working on bit flags, but then realized the code needed was pretty much the same either way.
I'll switch it over to bit flags and see if that makes it a bit cleaner. :)
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.
Seems a bit difficult since indent===null
would make sense.
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58329 |
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 am all for changes in this PR. The temporary object style was prevailing before we migrated to TS since at that moment the IDE can't infer the parameter names.
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.
More of this! :)
Fixes #1, Fixes #2
#16958 (review)
main
PR