Skip to content

Comments

Improve docs on endpoints#4445

Closed
stephane-vanraes wants to merge 0 commit intosveltejs:masterfrom
stephane-vanraes:master
Closed

Improve docs on endpoints#4445
stephane-vanraes wants to merge 0 commit intosveltejs:masterfrom
stephane-vanraes:master

Conversation

@stephane-vanraes
Copy link
Contributor

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

This PR includes a small improvement to the docs regarding endpoints, currently it is not easy to figure out what the input parameters to the functions are as it involves a lot of clicking around. The change aims to have the RequestEvent and RequestOutputHandler types directly linked from the text instead.

@changeset-bot
Copy link

changeset-bot bot commented Mar 24, 2022

⚠️ No Changeset found

Latest commit: 62764fd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

### Endpoints

Endpoints are modules written in `.js` (or `.ts`) files that export [request handler](/docs/types#sveltejs-kit-requesthandler) functions corresponding to HTTP methods. Their job is to make it possible to read and write data that is only available on the server (for example in a database, or on the filesystem).
Endpoints are modules written in `.js` (or `.ts`) files that export [request handler](/docs/types#sveltejs-kit-requesthandler) functions corresponding to HTTP methods, which take a [RequestEvent](/docs/types#additional-types-requestevent) as input and return a [RequestHandlerOutput](/docs/types#sveltejs-kit-requesthandleroutput). Their job is to make it possible to read and write data that is only available on the server (for example in a database, or on the filesystem).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Endpoints are modules written in `.js` (or `.ts`) files that export [request handler](/docs/types#sveltejs-kit-requesthandler) functions corresponding to HTTP methods, which take a [RequestEvent](/docs/types#additional-types-requestevent) as input and return a [RequestHandlerOutput](/docs/types#sveltejs-kit-requesthandleroutput). Their job is to make it possible to read and write data that is only available on the server (for example in a database, or on the filesystem).
Endpoints are modules written in `.js` (or `.ts`) files that export [request handler](/docs/types#sveltejs-kit-requesthandler) functions corresponding to HTTP methods, which take a [`RequestEvent`](/docs/types#additional-types-requestevent) as input and return [`RequestHandlerOutput`](/docs/types#sveltejs-kit-requesthandleroutput). Their job is to make it possible to read and write data that is only available on the server (for example in a database, or on the filesystem).

That said, I'm not sure whether this change is necessary, because if you click "request handler", you're brought to its type, where you can see what it accepts and returns.

@benmccann
Copy link
Member

Addressing @Conduitry's question above, I think this is a reasonable change for a couple reasons:

  • if you want to know what the input/output is it will save you a click vs having to click first to the RequestHandler and then to the input/output
  • the type for RequestHandler is somewhat difficult to read. @stephane-vanraes reported confusion on Discord and that he was unable to understand what the input was from the RequestHandler type. If you want to find out what the input of the RequestHandler is you need to ignore the first eight lines, which mostly say that the type uses generics and aren't very meaningful given our usage of generated types
  • we might not want to do it everywhere to avoid verbosity, but this is a key heavily used feature of SvelteKit and it'd be nice to help people get started with the basics a bit more easily

Frankly, our display of types that use a lot of generics is not very good. We could customize this to be easier to read by generating the docs ourselves (like in #4005), but it would be quite a bit of work whereas this is a pretty easy change to implement

If there's one other place I think it might be worth linking directly to the input/output it would be https://kit.svelte.dev/docs/loading since it's the most used part of SvelteKit and most of the points above apply there as well. I updated the PR to update that page as well so that whatever we decide here we're doing things consistently. I don't think I'd probably do it beyond that

@stephane-vanraes
Copy link
Contributor Author

stephane-vanraes commented Mar 25, 2022

I think the main concern with legibility is that in the current form the signatures are hard to read for those not used to TypeScript.
Understanding this:

interface Something {
  count: number,
  name: string
}

is still fairly straightfoward and doable for non-TS developers.

But the entire construction with the generics and extends could very well be a completely different language (which technically it is).

@ignatiusmb
Copy link
Member

The types documentation absolutely needs some love, especially the public types. Most of the types are just code blocks of its type signature without any description, and I think we can mitigate a lot of these problems by just having some explanation about the code blocks. Explaining the type would also reduce the burden of having to manually update the links in case the name changes because they are already automatically linked in the code blocks when generated.

This feels like it belongs in the RequestHandler (and Load) type docs itself as it is quite unlikely that the input and output signature are used separately—not to mention that the input RequestEvent type is private, so you can't even use it individually.

Anyone using these types (with TS) needs to understand how the whole RequestHandler type behaves, they will definitely come across generics sooner or later, though the formatting could arguably be nicer. On the contrary, JSDoc users would usually use /** @type {import('./[id]').RequestHandler} */ and ignore the generics altogether, and most IDE will also give autocompletion inside ({ ... }), so many users can get away with just adding the JSDoc and not needing to refer the docs as often.

@Rich-Harris
Copy link
Member

Agree that the type is cryptic, but also agree that the changes don't really belong anywhere else. What about #4475 as an alternative?

https://kit-svelte-dev-git-link-inline-codespans-svelte.vercel.app/docs/types#sveltejs-kit-requesthandler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants