Update more return types to include undefined #15903
Conversation
src/services/types.ts
Outdated
| @@ -19,34 +19,34 @@ namespace ts { | |||
| getFirstToken(sourceFile?: SourceFile): Node; | |||
| getLastToken(sourceFile?: SourceFile): Node; | |||
| // See ts.forEachChild for documentation. | |||
| forEachChild<T>(cbNode: (node: Node) => T, cbNodeArray?: (nodes: Node[]) => T): T; | |||
| forEachChild<T>(cbNode: (node: Node) => T, cbNodeArray?: (nodes: Node[]) => T): T | undefined; | |||
There was a problem hiding this comment.
Callbacks should also return | undefined.
There was a problem hiding this comment.
@andy-ms
T is inferred from the return type of the callback(s). I don't get why it should be declared to return T | undefined. Can you please explain your thoughts on this?
There was a problem hiding this comment.
Just for documentation: if a callback always returns a truthy value then forEachChild will terminate immediately and return the first result. Since it's always allowed to return undefined, it doesn't hurt to have it there explicitly.
|
Looks like there is something wrong with the line-endings in |
|
@DickvdBrink that seems to be an issue with github. Other PRs also have this |
|
Ah oke, my bad ;) 👍 |
|
@andy-ms any more comments? |
|
@ajafff mind updating the PR. |
|
@mhegazy @andy-ms updated and resolved conflicts. |
This PR updates even more return types where
undefinedcan be returned.The changes in services.ts and jsdoc.ts are not strictly necessary, because the APIs defined there are not public. But it may help you when you want to enable strictNullChecks in this project.
Ref: #15887