Skip to content

fix the issue that @property types are not recoganized#9621

Merged
zhengbli merged 2 commits intomicrosoft:masterfrom
zhengbli:fixJSDocPropertyTag
Jul 13, 2016
Merged

fix the issue that @property types are not recoganized#9621
zhengbli merged 2 commits intomicrosoft:masterfrom
zhengbli:fixJSDocPropertyTag

Conversation

@zhengbli
Copy link
Copy Markdown
Contributor

Fixes #9620

if (declaration.kind === SyntaxKind.ExportAssignment) {
return links.type = checkExpression((<ExportAssignment>declaration).expression);
}
if (declaration.kind === SyntaxKind.JSDocPropertyTag && (<JSDocPropertyTag>declaration).typeExpression) {
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy Jul 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not this be in getTypeForVariableLikeDeclarationFromJSDocComment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That method is originally for declarations written in the code whose type annotation in another seperated jsdoc comment, so other methods who call it all assumes VariableLikeDeclaration type of the declaration. It seems to be a lot of change to make all of them accept VariableLikeDeclaration | JSDocPropertyTag. What would be the benefit of doing that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing specifically. it is just where all our get-type-from-jsdoc code.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Jul 11, 2016

👍

@zhengbli zhengbli merged commit 9eeb69d into microsoft:master Jul 13, 2016
@zhengbli zhengbli deleted the fixJSDocPropertyTag branch July 13, 2016 18:32
zhengbli added a commit that referenced this pull request Jul 13, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants