Skip to content
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

support for @property on classes (including tests) #91

Merged

Conversation

aidinabedi
Copy link
Contributor

@aidinabedi aidinabedi commented Aug 23, 2019

We need support for class properties and since #73 has not been updated since May. I decided to take it upon myself to fulfill the requirement regarding tests, as stated in #73 (comment).

So this PR is based on the work done by @HackbrettXXX, with the addition of test cases. See output: https://travis-ci.org/englercj/tsd-jsdoc/builds/575963771#L251

@englercj
Copy link
Owner

Thanks @aidinabedi for adding tests. Glad you did, because it looks like PropTest3 and PropTest6 are not valid Typescript.

They both contain:

{
    myProp: boolean;
    myProp: boolean;
}

@aidinabedi
Copy link
Contributor Author

It's not the job of tsd-jsdoc to output valid TypeScript in that test. The output is intentionally meant to be duplicate properties because I wrote "invalid" jsdoc comments, i.e. describing the same property twice. Which I could just as well do using other jsdoc tags (without the PR). I wanted to have a test to make sure the behavior is still the same, and tsd-jsdoc isn't trying to be smart in this "special" case. To quote the readme:

This library provides very little validation beyond what JSDoc provides. Meaning if you have invalid JSDoc comments, this will likely output an invalid TypeScript Definition File.

@aidinabedi
Copy link
Contributor Author

@englercj I've added additional duplicates to illustrate that the behavior is the same, whether you use @property or regular @type tags.

@englercj
Copy link
Owner

Fair enough.

@englercj englercj merged commit 89c5655 into englercj:master Aug 24, 2019
@HackbrettXXX
Copy link
Contributor

Thanks for providing the tests @aidinabedi. Couldn't find the time myself.

@aidinabedi
Copy link
Contributor Author

@HackbrettXXX No problem!

@aidinabedi aidinabedi mentioned this pull request Sep 1, 2019
@aidinabedi aidinabedi deleted the feature/class-properties-with-tests branch April 5, 2020 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants