-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Remove unused internal utilities #17380
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
ae28cfb
to
549f9fb
Compare
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.
Approved, but you'll need to merge from master and decide whether to keep findDeclaration
or replace a new usage that I added with find(symbol.declarations, ...
@@ -26,20 +26,6 @@ namespace ts { | |||
return undefined; | |||
} | |||
|
|||
export function findDeclaration<T extends Declaration>(symbol: Symbol, predicate: (node: Declaration) => node is T): T | undefined; |
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.
please don't remove this one. I use it in an upcoming change and would have to put it back.
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.
would it be fast enough to have getDeclarationOfKind
delegate to findDeclaration
?
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.
Actually, the rewrite at the bottom of the change to find(symbol.declarations, ...
is good enough since my new usage requires a cast as well.
(I also just merged the new usage, so you will see it when you merge from master.)
src/compiler/transformers/jsx.ts
Outdated
@@ -114,7 +114,7 @@ namespace ts { | |||
compilerOptions.reactNamespace, | |||
tagName, | |||
objectProperties, | |||
filter(map(children, transformJsxChildToExpression), isDefined), | |||
children && mapDefined(children, transformJsxChildToExpression), |
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.
Pretty much all of our other array utils in core.ts test for the array possibly being undefined. Should we not also do that for mapDefined
?
@rbuckton Good to go? |
* Remove unused internal utilities * Handle undefined input to `mapDefined`
Many of the functions in
utilities.ts
are not used.