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 TypeScript types to status code docs #20

Merged
merged 3 commits into from Apr 20, 2020

Conversation

ZPiDER666
Copy link
Contributor

Improved status code example (TS, idiomatic)

Improved status code example (TS, idiomatic)
README.md Outdated
import { Status as status } from 'https://deno.land/std/http/http_status.ts';
const handler = (request) => {
return request.response.code(status.Teapot);
const handler = (request:pogo.Request, h:pogo.Toolkit) => {
Copy link
Owner

Choose a reason for hiding this comment

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

These classes, Request and Toolkit, aren't actually exposed by main.js. So this doesn't work, does it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does for me, but i am using TS and the library is not currently, so there may be an interfacing issue. in any case i prefer to do this over :any

Copy link
Owner

Choose a reason for hiding this comment

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

Some kind of TypeScript magic, fascinating. Alright, I'll accept this. When the TypeScript conversion in #14 is merged, do you think it would be reasonable to remove these and use the contextually inferred types? I'm not super familiar with TS conventions yet.

I'd like to keep the docs simple and approachable, maybe even limited to plain JS syntax in most cases (aside from specific examples of TypeScript usage). But I acknowledge that since type inference doesn't work well at the moment, adding types here makes sense until Pogo is written in TypeScript.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely, simple docs are a big plus as long as there are enough concrete examples. my initial problem was that i did not know the signature of the handler method and therefore had not much of an idea what to do. concrete type mentions can help with that as it points you into a direction.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks. One last question: why did you switch from request.response to h.response()? They're both fine, to be clear. The latter is a bit more functionally pure, which is nice, but since it introduces an additional concept, I figured the former might be more approachable. I'm merging this as-is since it's not super important, but curious to hear your thought process.

@sholladay sholladay changed the title Update README.md Add TyoeScript types to status code documentation Apr 20, 2020
@sholladay sholladay changed the title Add TyoeScript types to status code documentation Add TypeScript types to status code documentation Apr 20, 2020
@sholladay sholladay changed the title Add TypeScript types to status code documentation Add TypeScript types to status code docs Apr 20, 2020
@sholladay sholladay merged commit e0157b3 into sholladay:master Apr 20, 2020
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.

None yet

2 participants