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

Provide interfaces for options with JSDocs #1586

Closed
Shinigami92 opened this issue Nov 21, 2022 · 3 comments
Closed

Provide interfaces for options with JSDocs #1586

Shinigami92 opened this issue Nov 21, 2022 · 3 comments
Assignees
Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@Shinigami92
Copy link
Member

Shinigami92 commented Nov 21, 2022

Clear and concise description of the problem

As a user I want to have JSDoc tooltips when hovering over option properties in my IDE so I can directly see what the parameter is aiming for.

Suggested solution

Provide interfaces for e.g. ranges.
Maybe even extract types for every ModuleMethodOption so I as a user can see e.g. default values for each specific property.

Alternative

No response

Additional context

#1486 (comment)


Automatisating the VitePress generated docs is a non-goal for the first PR. Everything that broke after this change for the docs need to be handled in a separate PR. That is so we can focus on IDE UX support in the first place.

@Shinigami92 Shinigami92 added the s: pending triage Pending Triage label Nov 21, 2022
@Shinigami92 Shinigami92 added c: feature Request for new feature and removed s: pending triage Pending Triage labels Nov 21, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Nov 21, 2022
@ST-DDT ST-DDT added the p: 1-normal Nothing urgent label Nov 21, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Nov 21, 2022

Just repeating this here so it doesn't get lost:

Please note, that I will only accept it for the entire project, if there is a test that ensures that the text is the same as the param's and optionally the respective parts are generated automatically (When using inline types).


I think one of the reasons, why we don't use named types, is that IDEs don't show their actual content and you have to click on it to get all properties. (It may potentially also requires some extra work to work with typedoc's @param detection/filtering, not a blocker though). So we should use these named types for only a few select params/options i.e. NumberOrRange.

grafik

@ST-DDT
Copy link
Member

ST-DDT commented Dec 8, 2022

Team decision

We discussed adding jsdocs to each option param directly in addition to the @param ones (basically a copy of that)

int({
  /**
   * The min value.
   */
  min: number,
  /**
   * The max value.
   */
  max: number
})

We decided not to use explicit types (e.g. AlphaOptions), because

  • some methods have and require different types+descriptions for their attributes (exclude digits vs alpha chars)
  • @param will no longer be checked
  • It reduces the readability of the source code

@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Dec 8, 2022
@xDivisionByZerox
Copy link
Member

I will close this since this issue has been solved by #1644 as far as I can see. If this is not correct please reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants