kit icon indicating copy to clipboard operation
kit copied to clipboard

maybe improve types documentation a bit

Open Rich-Harris opened this issue 3 years ago • 12 comments

Deployed here: https://kit-svelte-dev-git-better-docs-svelte.vercel.app/docs/types-presentation

Some of the documentation around types is a bit ropey. For example I don't love how this looks:

image

Also, the configuration documentation is kind of all over the place — this stuff really belongs in types/index.d.ts, so that it appears inline when you're actually writing the config, rather than being manually written in a separate documentation file where it's less useful (and manually maintained as we make changes).

In this PR I'm trying some ideas for how we might improve it a bit. Am open to all suggestions. Some of this might need custom CSS to make it look halfway decent.

Rich-Harris avatar Sep 23 '22 21:09 Rich-Harris

🦋 Changeset detected

Latest commit: 0ed36e18c4133ab8f0aa3402990a479bb75a225b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
create-svelte Patch
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Sep 23 '22 21:09 changeset-bot[bot]

Just learning TypeDoc while learning TS. My main motivation to learn TS is in fact having docs for free*. This combo is making me fall in love with TS. I heard JsDocs was gaining some traction in the svelte community as the first step to TS without having to get married to it. Should svelte/kit have both? It seems JsDoc and TsDoc is mostly interoperable (don't have to add the type in the params in TsDoc as one diference), don't know enough to comment on it, but I've seen some TypeDoc magic that I can't wait to see around here. I might be talking nonsense and all of this is already implemented or something else, I just wanted to bump this, I'd loved to see better documentation not only for final users but also for devs of svelte/kit.

EDIT: is this hilarious?

SrGeneroso avatar Sep 24 '22 01:09 SrGeneroso

During the work on #7087 I felt hindered by having to write more detailed docs inside JSDoc. You can't use JSDoc within JSDoc for example (/** @type {..} */ will close the outer JSDoc). Moreover, it could get overwhelming seing an ocean of docs when hovering over a function. In also complicates future enhancements like translation or JS+TS in the docs with a switch.

I therefore think we need a different approach - some kind of mix between manual docs and "include X from the types". Made-up example-MD file that would need to be preprocessed:

## cookies

### set

--definition-- // <- this would extract the type definition from types.d.ts into here
--summary-- // <- this would extract the JSDoc from types.d.ts into here in case you don't want to duplicate things

Lorem ipsum bla whatever you need additionally

dummdidumm avatar Sep 29 '22 16:09 dummdidumm

Reverting part of my previous comment - the API docs should be very concise and more on the short side. The cookie example is the biggest it should probably get. If there's more it should go into a different part of the docs, explaining things in more detail and in the context of usage. What remains is the question about code samples, which I don't have a good answer to. Edit: Seems like there's an example of this in the ambient type definitions already. It's writtin in a way that doesn't break the JSDoc but the docs are borked if you hover over them from your IDE. not sure what's the best solution here.

dummdidumm avatar Oct 01 '22 16:10 dummdidumm

I pushed a POC of a generator enhancement that transforms interfaces into better looking docs similar to what you proposed. I added a border to each of the snippets that present sub headline to better distinguish them from regular code snippets. I also kept a trimmed down version of the whole interface at the start of each section so people can get a quick overview of that type.

I think the result is much better-looking, but only if we have actually some docs in there - so I think if we change this, we should also get down to writing a lot more quick API docs here at the same time.

The final part of the picture (for me at least) would be to restructure the types and modules sections into one "API Reference" section, and we group the modules / interfaces logically into sub pages. Does this make sense? (Related: I think refining the side bar would be good; only showing the headers, not the sub headlines).

dummdidumm avatar Oct 20 '22 13:10 dummdidumm

Pushed up some design tweaks that make type blocks visually distinct from code blocks, and make things more grouped visually.

This is definitely an improvement over the status quo, but there's a bunch of issues (inherited or otherwise) that we should try and address if we can:

  • [x] code block references value imports, not types
image
  • [x] this still looks overwhelming, not sure if there's a better way to present it
image
  • [x] unrelated to this work, but super confusing wording
image
  • [x] properties without descriptions look weird, especially at the end

We need to add loads more docs

image
  • [ ] do I as a user need to see this? seems complicated and irrelevant
image
  • [x] repetition of cancel() looks kinda awkward in situations like this

What if the top was just interface BeforeNavigate extends Navigation {...} or something?

image
  • [x] this is duplicative

Everything in this block is described immediately afterward — it's not clear it serves any real value

image
  • [x] some types don't create links, like ValidatedConfig here

Also, a sequence of properties without descriptions looks super weird

image
  • [x] indentation is wacky. also, what does "dest null" mean?
image
  • [x] bullet points and text are way out of alignment
image
  • [x] these should all link back to the relevant docs sections
image
  • [x] why do some things get property descriptions and others don't?

Also, the configuration docs should be driven from the types. we need to be able to show default values somehow

image
  • [x] references to generics show up as 'any'
image
  • [x] wonky margins around code blocks
image
  • [x] double line breaks are unnecessary

(assuming we don't get rid of the contents of these interfaces altogether)

image
  • [x] nested properties don't look great
image

Rich-Harris avatar Nov 23 '22 18:11 Rich-Harris

This is a very thorough analysis! I agree with most of that. I think the two biggest things we can do are

  • doc this shit up: everything should have some jsdoc so it doesn't look that empty
  • rethink modules and types sections: I think we should merge them to an API section. If we think it gets too big that way, we could split each into a separate page, which would also help with "what ends/starts where?". I quite like Angular's approach here, with an overview page from which you can jump to the details of what you need https://angular.io/api

dummdidumm avatar Nov 23 '22 18:11 dummdidumm

Another issue — probably shouldn't block merging this PR, but worth opening as follow-up — type variables are ignored when converting JSDoc to TypeScript:

image image

Rich-Harris avatar Nov 24 '22 19:11 Rich-Harris

Don't know if you already saw this, so I'm posting it again: IMO it makes sense to rethink modules and types sections in general: I think we should merge them to an API section. Right now things are split across these two sections (for example $app/forms appears in both), which is weird. If we think it gets too big that way, we could split each into a separate page, which would also help with "what ends/starts where?". I quite like Angular's approach here, with an overview page from which you can jump to the details of what you need https://angular.io/api

dummdidumm avatar Nov 24 '22 20:11 dummdidumm

Yeah, I had that in the back of my mind as I was fixing up the smaller things. It's definitely an option, though for the SubmitFunction in particular I found it made more sense just to move it into @sveltejs/kit (notwithstanding the fact that it makes this a breaking change).

It is a big page. I definitely wouldn't rule out us subdividing it further later like Angular does. But I think it's worth avoiding if we can — Angular's API is just monstrously large, and if SvelteKit ends up as unwieldy as that then I plan to retire.

Until then I think the modules/types distinction makes sense, given that all the types are exported from @sveltejs/kit or ./$types. I think combining them at this stage would reduce clarity

Rich-Harris avatar Nov 24 '22 20:11 Rich-Harris

There's definitely room to add more descriptions, but that doesn't need to hold up this PR any further. I think the big thing remaining is to sort out the configuration docs:

  • [x] they're still written as though we can't document nested properties — needs tweaking
  • [ ] we have both https://kit-svelte-8bwewn5rw-svelte.vercel.app/docs/configuration and https://kit-svelte-8bwewn5rw-svelte.vercel.app/docs/types#sveltejs-kit-kitconfig. There Can Be Only One. I'm not sure how best to shuffle things around though
  • [x] defaults aren't present for all values yet. (Also, it might be nice if we had some iconography for params/defaults/returns or at least a slightly improved design, rather than bullet points)
  • [ ] if we retire /docs/configuration then it would be good to generate hash links for properties, at least at the top level

Rich-Harris avatar Nov 24 '22 20:11 Rich-Harris

I vote for keeping the configuration section. It can be autogenerated, but I think it's better to have it separate, since it's rather big and semantically doesn't fit into types/modules for me.

dummdidumm avatar Nov 25 '22 11:11 dummdidumm

Another thing to fix in a follow-up PR

image

Rich-Harris avatar Nov 27 '22 23:11 Rich-Harris

The create-svelte tests are failing — something to do with moving SubmitFunction form $app/forms to @sveltejs/kit I assume. @dummdidumm any idea why this is happening?

Rich-Harris avatar Nov 28 '22 16:11 Rich-Harris

No idea why this fails ... I need to investigate more thoroughly tomorrow. So far it looks like this only fails for .svelte files, so this might be some bug inside language tools.

dummdidumm avatar Nov 28 '22 17:11 dummdidumm

No idea why this fails ... I need to investigate more thoroughly tomorrow. So far it looks like this only fails for .svelte files, so this might be some bug inside language tools.

I lied, this also happens inside TS files. Still no idea why. Update: I think it happens if the imported type is a function interface (or type) - looks like a bug inside TypeScript. It only happens if you have an ambient file which you export from the main types (so they are available to all), while at the same time importing a function type or interface from those types into one of the ambient modules.

dummdidumm avatar Nov 29 '22 16:11 dummdidumm

I was trying to configure the trailingSlash option, as introduced in #733, but found it's missing in the current config and found this to be the PR where it got removed. I couldn't really figure out why, so was wondering if it was intentional? If so, is the feature no longer supported?

realvasko avatar Feb 02 '23 17:02 realvasko

Never mind, saw that it's now part of the page options.

realvasko avatar Feb 03 '23 10:02 realvasko