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

Add documentation snippets to the k6/* type definitions #1069

Closed
na-- opened this issue Jul 5, 2019 · 22 comments
Closed

Add documentation snippets to the k6/* type definitions #1069

na-- opened this issue Jul 5, 2019 · 22 comments

Comments

@na--
Copy link
Member

na-- commented Jul 5, 2019

As discussed in #929 (comment), docs in the type definitions would be a nice feature to have, since they can be useful when shown in the auto-complete popups. Of course, I doubt we can fit the full docs, but short descriptions should suffice, especially if we can also add links to the full docs for each object and function?

@bookmoons
Copy link
Contributor

Working toward this in k6/description.

@bookmoons
Copy link
Contributor

Here's what vscode looks like with the descriptions.

check-cropped
fail-cropped
group-cropped
sleep-cropped

@ppcano You might be interested in this.

@bookmoons
Copy link
Contributor

Examples of some docs generated from the types, with a few different themes.

cayman
tactile
hacker

@ppcano
Copy link
Contributor

ppcano commented Jul 8, 2019

@bookmoons 👏😍

Please, could you tell us how are you generating the markdown files?

I asked you because we plan to create a new documentation website for k6 (replacing the existing readme website). I talked with @na-- about how we could have "independent" markdown files for the API docs that can be imported into the new documentation website. This is similar to what you have done with the above themes.

Could the TSdocs include a section (like examples) that vscode does not show it but it will be transformed to markdown?

https://github.com/Microsoft/tsdoc#what-are-the-goals

Extensible: Each tool can supplement the core tags with custom tags for specialized scenarios.

@na-- I think we should continue exploring this solution.

@bookmoons
Copy link
Contributor

Did it with API Extractor.

Could the TSdocs include a section (like examples) that vscode does not show it but it will be transformed to markdown?

Haven't seen something that does this but I'm not familiar with everything yet. There's the syntax. Maybe that @packageDocumentation tag.

@bookmoons
Copy link
Contributor

Think I have my head around the doc language and I have a good model down. I'm ready to blast through the rest of these descriptions.

Can I ask if you guys have an idea what you're thinking about the docs? I know you've mentioned the issue with duplicating them, which is such a pain to maintain. I'm wondering whether I should go for as much as possible so they can generate useful docs, or minimal so they're not creating a maintenance load.

I can hold off for now if it needs some percolation time.

@bookmoons
Copy link
Contributor

Poked around the vscode rendering a little. Examples show in intellisense. Even custom tags show in intellisense. It just lists everything. Not sure there's any way right now to define things just for docs.

/**
 * @notATag Custom tag not in standard TSDoc.
 */
export function foo(): void;

foo-cropped

Mentioned this as a possible feature in microsoft/vscode#77261.

@na--
Copy link
Member Author

na-- commented Jul 12, 2019

Now that things have percolated for a few days, I think I have a reasonably good idea about the pros and cons regarding the API docs in the DefinitelyTyped repository.

Pros of having the whole API documentation there:

  • We'll have just a single place that would need to be updated when we change the API or want to improve the docs.
  • Have a unified format for that documentation, as well as a reasonably automated way to convert it to a somewhat readable markdown format.

Cons:

  • The documentation will be in a different repo than the k6 code, which means:

    • for every k6 pull request, we'll have to make another separate pull request to update the docs and type definitions
    • that pull request will be in a repository that we don't control and that other people would have to review our PRs - this loss of control to me is the biggest negative consequence
    • we can't have an automatic versioning of the docs - if the docs were in the k6 repo, we can easily generate very accurate (based on git commits and tags) API docs for each k6 version

    In summary, only this point seems to be bigger hassle than we need, making me think it might be worth researching if there isn't some nice way to host the type definitions in the k6 repo ourselves as well, not only the docs part...

  • Using docstrings for the full documentation gives us a nice way to generate the human-readable docs, but it will also likely restrict us by limiting us to its own specific format and capabilities. Writing the full documentation directly in a human-readable markdown format (with some conventions, of course) seems to be a much more flexible and better way to go about the task, especially if we want to give a lot of examples.

  • The issue you mentioned above, that VSCode will show everything we write is kind of also a big deal-breaker that will limit us severely if we want to have a nice documentation with lots of examples.

  • The TypeScript notations could also be somewhat confusing for people that haven't worked with strong type systems. For example, I'd bet that a lot of JS users would be somewhat confused what <T> and Checkers are in the export function check<T>(val: T, sets: Checkers<T>, tags?: object): boolean; definition here... Having it as a type auto-complete popup would be somewhat OK, but in the actual documentation we can probably explain it in a more universally understandable way if we weren't so constrained to have valid TS type definitions...

So, in short, I think minimal docs (at most, a few short sentences per method, with a link to the actual docs) in the DefinitelyTyped repo is the way to go for now, despite causing minor duplication of the docs. We can revisit that in the future, if the VSCode issue is addressed and if we can bring those type definitions in our repo... @mstoykov , @ppcano , @bookmoons , @robingustafsson - thoughts?

@ppcano
Copy link
Contributor

ppcano commented Jul 12, 2019

LGTM.

with a link to the actual docs

I am not sure if this makes sense, but we can test it and see.

@bookmoons
Copy link
Contributor

It seems sensible.

For whatever it's worth, TSDoc and VSCode both seem unreceptive. VSCode has already closed the issue. microsoft/tsdoc#170 microsoft/vscode#77261

Proposed format:

  • 1 line summary.
  • Link to docs.
  • Params with short descriptions.
  • Return value with short description.
  • No @remarks with an extended description.
  • No @examples with code examples.
/**
 * Initiate a WebSocket connection to a remote host.
 * https://docs.k6.io/docs/connect-url-params-func
 * 
 * @param url - Request URL (e.g. "ws://echo.websocket.org").
 * @param params - Additional request parameters.
 * @param callback - Called when the connection is initiated.
 * @returns HTTP Response object.
 */
export function connect(url: string, params: Params | null, callback: Callback): Response;

Here's how it comes out. The link is clickable.

connect-cropped

Full docs for comparison. Extended description of the behavior is left out. Longer description of the callback param left out.

@na--
Copy link
Member Author

na-- commented Jul 12, 2019

The proposed format LGTM

@bookmoons
Copy link
Contributor

Alright, thanks guys. Heading that way.

@bookmoons
Copy link
Contributor

In the docs there are these little gold icons on some functions. eg createHash has one, createHMAC doesn't.

icons-cropped

What do those mean? Is that something I should be including?

@bookmoons
Copy link
Contributor

In k6/http the docs say Response.timings.looking_up is currently unavailable:
https://docs.k6.io/docs/response-k6http

Should I maybe take that out of the types?

@na--
Copy link
Member Author

na-- commented Jul 22, 2019

What do those mean? Is that something I should be including?

Apparently those just denote which items in the menu list are functions/methods and we forgot to mark some of the newer pages. Unfortunately, I have absolutely no idea how to change that, I can't find such an option in the interface 😕 We need to get out of readme.io and start hosting the docs as a static site we generate...

In k6/http the docs say Response.timings.looking_up is currently unavailable:
https://docs.k6.io/docs/response-k6http

Should I maybe take that out of the types?

Yeah, that was actually one of the reasons for the type definition fixes: #929 (comment) I just forgot to make sure it was taken out when I skimmed over the new definitions 😊

@bookmoons
Copy link
Contributor

Copy that.

@na--
Copy link
Member Author

na-- commented Jul 22, 2019

btw current idea is to bring back looking_up after the current DNS issues are fixed (#1011 (comment)), so I'll hopefully review your PR about them today or tomorrow at the latest 😄

@bookmoons
Copy link
Contributor

Opened DefinitelyTyped/DefinitelyTyped#37045 to remove looking_up.

@bookmoons
Copy link
Contributor

k6/description has this ready to submit. I'll get it open tomorrow after that little change is merged.

@bookmoons
Copy link
Contributor

looking_up is removed. Opened DefinitelyTyped/DefinitelyTyped#37056 to add docs.

Some details:


Function types pull information from different doc comments. I settled on this pattern to get them to render right everywhere. Type params and release tag on the interface. Params and return on the signature. Summary line duplicated.

/**
 * Check procedure.
 * @typeParam VT - Value type.
 * @public
 */
export interface Checker<VT> {
    /**
     * Check procedure.
     * @param val - Value to check.
     * @returns Whether check passed.
     */
    (val: VT): boolean;
}

I linked all HTML elements to the Element page. They're not all listed but I guess it's the closest thing.


Tried to include an API Extractor config file to support validating doc comments. It works nicely for that. But the repo doesn't like the config file. Tests fail with the file there.

I have a question open about this:
https://stackoverflow.com/questions/57143743/how-to-ignore-files

@bookmoons
Copy link
Contributor

bookmoons commented Jul 23, 2019

These docs are merged and published. DefinitelyTyped/DefinitelyTyped#37056

@na--
Copy link
Member Author

na-- commented Jul 24, 2019

Thanks, @bookmoons, closing this now.

@na-- na-- closed this as completed Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants