-
Notifications
You must be signed in to change notification settings - Fork 12.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
Add 'isNamedDeclaration' helper to reduce casts #22089
Conversation
@@ -4298,6 +4298,11 @@ namespace ts { | |||
return declaration.name || nameForNamelessJSDocTypedef(declaration); | |||
} | |||
|
|||
/** @internal */ | |||
export function isNamedDeclaration(node: Node): node is NamedDeclaration & { name: DeclarationName } { |
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.
Mmm, this implementation doesn't seem to actually check that the name is a DeclarationName
.
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.
Added an assertion -- a name
property should always be a DeclarationName
.
1299534
to
b1c15f4
Compare
b1c15f4
to
52d830f
Compare
src/compiler/utilities.ts
Outdated
/** @internal */ | ||
export function isNamedDeclaration(node: Node): node is NamedDeclaration & { name: DeclarationName } { | ||
const name = (node as NamedDeclaration).name; | ||
Debug.assert(!name || isDeclaration(node) && node.name === name, "A 'name' property should always be a DeclarationName."); |
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 really a hot path, i would rather not do additional work. this is rather simple check. does it have a name
property or not.
96b9248
to
142824b
Compare
Hi. is this new Type Guard not exposed externally as all of the other Type Guards? |
You can send us a PR to remove the |
Especially helps with #22088 because testing
(node as NamedDeclaration).name
doesn't make(node as NamedDeclaration).name
defined in future uses.