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

Remove type annotations from JSDoc? #17

Open
diskdance opened this issue Dec 21, 2022 · 9 comments
Open

Remove type annotations from JSDoc? #17

diskdance opened this issue Dec 21, 2022 · 9 comments

Comments

@diskdance
Copy link
Contributor

diskdance commented Dec 21, 2022

Given that JSDoc type annotations are simply ignored in .d.ts files, I believe they are redundant and should be removed.

@AnYiEE
Copy link
Contributor

AnYiEE commented Jun 23, 2023

I disagree. Due to maintenance costs, sometimes the type annotation is directly set as "any". In such cases, it is meaningful to have detailed explanations or examples in JSDoc that reflect the types being used.

@diskdance
Copy link
Contributor Author

I disagree. Due to maintenance costs, sometimes the type annotation is directly set as "any". In such cases, it is meaningful to have detailed explanations or examples in JSDoc that reflect the types being used.

Thank you for your advice, but I didn't say to remove all JSDoc comments, but only type annotations (i.e. {string} in @param {string} p1 - A string param.) because it has no actual effect. Detailed explanations and examples are kept as-is.

@AnYiEE
Copy link
Contributor

AnYiEE commented Jun 24, 2023

Thank you for your advice, but I didn't say to remove all JSDoc comments, but only type annotations (i.e. {string} in @param {string} p1 - A string param.) because it has no actual effect. Detailed explanations and examples are kept as-is.

However, due to maintenance costs, if there is a complex mixed object, its type is often directly set as "any".

JSDoc {Object} in @param {Object} p1 - A object param is meaningful to let people know that this is an object, so they will go back to check the documentation or source code - what exactly is the object here. Even if the type of typescript is any.

@diskdance
Copy link
Contributor Author

diskdance commented Jun 24, 2023

However, due to maintenance costs, if there is a complex mixed object, its type is often directly set as "any".

I hope you don't mind me being too straightforward here, but this is a misuse of any -- the best practice is always to describe the shape of the object as accurately as possible (as I have done in @types/oojs-ui), rather then rely on a feature that is meant to turn off all type checking features.

If I were a maintainer and saw a {Object} in JSDoc, I would then browse the source code to figure out its properties. This is what I have done hundreds of times when typing OOUI and it is totally feasible.

It is maintainers' responsibility to type them, not users.

@AnYiEE
Copy link
Contributor

AnYiEE commented Jun 24, 2023

It is maintainers' responsibility to type them, not users.

Well said, then you go ahead and do it. Until the types of all objects and arrays are accurately described, I oppose removing comments prematurely.

@diskdance
Copy link
Contributor Author

@AnYiEE Could you please give a full list of typings you think inaccurate "due to maintenance costs"? In such case problems can be solved faster, thanks!

@AnYiEE
Copy link
Contributor

AnYiEE commented Jun 24, 2023

@AnYiEE Could you please give a full list of typings you think inaccurate "due to maintenance costs"? In such case problems can be solved faster, thanks!

Maybe you can search globally for the keyword any. I recommend basing it on the source code in this pull request, as they have been renovated based on the latest upstream source code. Thanks for your work.

@diskdance
Copy link
Contributor Author

After #24 I believe it is sufficient to have type annotations removed.

@Derugon
Copy link
Collaborator

Derugon commented Jan 22, 2024

At the current state, some type annotations have been removed, some have been modified (to be more specific), and others have been left as they are in the original JSdoc. Sometimes there are also multiple conventions used (e.g. JSdoc vs TS syntax of objects).

If, for the reason discussed above, some annotations should be kept as is, I suggest either to:

  • remove most of annotations and keep those of objects whose structure has not yet been typed, or
  • keep the annotations and synchronize them with TS types whenever possible.

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

No branches or pull requests

3 participants