-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Migrate more MapLikes to Maps #10359
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
Conversation
src/compiler/checker.ts
Outdated
| ? assumeTrue ? TypeFacts.EQUndefinedOrNull : TypeFacts.NEUndefinedOrNull | ||
| : value.kind === SyntaxKind.NullKeyword | ||
| ? assumeTrue ? TypeFacts.EQNull : TypeFacts.NENull | ||
| : assumeTrue ? TypeFacts.EQUndefined : TypeFacts.NEUndefined; |
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.
Not a fan of the ? and : prefix style. Would prefer to keep the existing formatting.
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 can change it back. I primarily made the change to work around an issue with our .tmLanguage syntax coloring as the existing formatting resulted in syntax coloring being disabled/incorrect for the next 30 lines or so in VS Code.
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.
Both are nigh impossible to parse for a human, fwiw :-P
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.
@bterlson, so how's that pattern matching proposal coming along? :)
|
👍 Once comments are addressed. Thanks for cleaning all of this up. Much nicer with these changes. Just confirming that we still use |
|
I left everything as |
This change migrates a number of remaining
MapLike<T>entries toMap<T>, as well as adds a number of additional functions for working with bothMapLike<T>andMap<T>.The changes in the compiler itself add an additional performance improvement, shaving another ~400ms off of the total emit time of Monaco and Angular.
This also disables the tslint restriction on use of the
inoperator, as offline performance tests I've run show that usinginis faster than bothhasOwnPropertyandobj[key]when merely testing for the presence of a key. Theinoperator also works better when paired withdelete, which is faster at removing entries from aMap<T>than setting the entry toundefined.