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 comments to exported TypeScript types #1278

Merged
merged 17 commits into from
Aug 16, 2020

Conversation

pmmmwh
Copy link
Contributor

@pmmmwh pmmmwh commented May 21, 2020

This PR address the last missing bit of #758 by adding doc comments for all public exported types.

Most of the wordings are borrowed from the README.

Questions

  1. Is there a way to config VS Code/whatever to conform to sindresorhus/typescript-definition-style-guide#documentation? It is a bit painful (and difficult for me to check consistency) when I constantly have to tab/indent comment blocks by hand 🤦
  2. How should I document merged types like isStream, resolveBodyOnly, responseType, NormalizedOptions, etc.?

Checklist

  • I have read the documentation.
  • I have included a pull request description of my changes.
  • I have included some tests.
  • If it's a new feature, I have included documentation updates.

Fixes #758

@pmmmwh pmmmwh changed the title Typescript docs Add doc comments to Typescript types May 21, 2020
@pmmmwh pmmmwh changed the title Add doc comments to Typescript types Add doc comments to exported Typescript types May 21, 2020
@pmmmwh
Copy link
Contributor Author

pmmmwh commented May 21, 2020

Hmm ... I'm not sure why tests are failing. All code added in this PR are within comment blocks.

@sindresorhus
Copy link
Owner

Hmm ... I'm not sure why tests are failing.

Just ignore it.

@sindresorhus
Copy link
Owner

Let me know when this is done and ready for review.

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Jun 10, 2020

Let me know when this is done and ready for review.

I think it is now mostly ready. There are still some parts missing, but they are either:

  • Undocumented in the README,
  • Or I don't really have an idea of whether they need to be documented, or how they should be documented.

A specific case I have some doubts is for function overloads, like GotPaginate or even Got/GotStream. I'm not sure if TS can correctly pick up the documentation for both overloads without duplicating them ...

@pmmmwh pmmmwh marked this pull request as ready for review June 10, 2020 11:30
@sindresorhus
Copy link
Owner

I'm not sure if TS can correctly pick up the documentation for both overloads without duplicating them ...

I'm not sure either, but you can test by requiring Got and opening it up in VSCode to see.

@sindresorhus
Copy link
Owner

Can you also update the contribution guidelines to mention that doc changes should also be applied to the doc comments, not just readme.

@szmarczak
Copy link
Collaborator

I'm not sure either, but you can test by requiring Got and opening it up in VSCode to see.

I just checked. TS doesn't assume it's the same function with a different name.

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Jul 28, 2020

I'm not sure either, but you can test by requiring Got and opening it up in VSCode to see.

I just checked. TS doesn't assume it's the same function with a different name.

Should I duplicate the docs or leave it alone? I think this issue also exists for re-exports/types that are used in interfaces as values.

(Sorry I haven't been able to dug into this ...)

@szmarczak
Copy link
Collaborator

Should I duplicate the docs or leave it alone?

@sindresorhus Maybe let's duplicate them?

@sindresorhus
Copy link
Owner

Yeah, I guess we have to duplicate them.

@sindresorhus
Copy link
Owner

Relevant microsoft/TypeScript#407

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Aug 13, 2020

I finally got time to "finish" this 🤦

I've duplicated all the documentation as necessary, and have checked that most types should be documented.

I've left out some types that are quite sparse/would rarely be used:

  • OptionsOfTextResponseBody and its counterparts plus GotRequestFunction
  • Hook function types (could copy them from the Hooks interface) if needed
  • Cookie Jars (no applicable documentation as the existing ones only describes the signatures)
  • Parts of the retry.calculateDelay API (no applicable documentation)
  • NormalizedOptions and Defaults

Please tell me if you want me to fill in any gaps listed above or need to change anything!

@sindresorhus sindresorhus changed the title Add doc comments to exported Typescript types Add documentation comments to exported TypeScript types Aug 16, 2020
@sindresorhus sindresorhus merged commit eaf1e02 into sindresorhus:master Aug 16, 2020
@sindresorhus
Copy link
Owner

Thank you for finishing this :)

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.

Make the TypeScript types strict and better
3 participants