-
Notifications
You must be signed in to change notification settings - Fork 168
feat: add new method on Request class #35
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
Conversation
hey @shinspiegel , i'm thinking maybe it should be interface FormDataBody {
get(key:string): string
getFile(key:string): FormFile
}
interface FormFile {
filename: string
contentType: string
content: Uint8Array
}
interface Request {
...
decodeBody(type: 'text'): Promise<string>
decodeBody(type: 'json'): Promise<any>
decodeBody(type: 'form-data'): Promise<FormDataBody>
} what do you think? |
|
@ije I think it would be best to only use the |
@shadowtime2000 yes you are right! |
Hi, I was trying to overload the method, but the intellisense keeps getting a error. This is the right way of doing this in typescript? async decodeBody(type: "text" | "json" | "form-data"): Promise<string | any | FormDataBody> {
...
} Or this is the right way? async decodeBody(type: "text"): Promise<string>
async decodeBody(type: "json"): Promise<any>
async decodeBody(type: "form-data"): Promise<FormDataBody> {
...
} Or there is other way of doing? I was reading the Deno.Land docs to parse the form data, but I didn't find how to properly use it. I'll keep digging tomorrow. Ps.: |
@shinspiegel well done! thank you!
|
Hi, sorry for late response (couple of busy days) I did try to use the TS the way show below, and for some reason the Deno compiler just doesn't get it. I'm doing something wrong? (I'm new to TS, trying to catch up) class Request {
...
decodeBody(type: 'text'): Promise<string>
decodeBody(type: 'json'): Promise<any>
decodeBody(type: 'form-data'): Promise<FormDataBody>
async decodeBody(type: string): Promise<any> {
// decode
}
...
} The VSCode still gives me error on this one. Can I use the code from other source like multipart/form-data body please ref deno.land/x/[email protected]/multipart.ts? Or should I create a complete separated file with the complete (and a little bit changed) version of theirs? |
hi @shinspiegel thanks for your great work, it's ok with oak, maybe we can implement the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
api.ts
Outdated
@@ -84,6 +85,51 @@ export class Request extends ServerRequest implements APIRequest { | |||
await this.send(JSON.stringify(data, replacer, space), 'application/json; charset=utf-8') | |||
} | |||
|
|||
async decodeBody(type: "text"): Promise<string> | |||
async decodeBody(type: "json"): Promise<any> | |||
async decodeBody(type: "form-data"): Promise<FormDataBody> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here should be:
decodeBody(type: 'text'): Promise<string>
decodeBody(type: 'json'): Promise<any>
decodeBody(type: 'form-data'): Promise< FormDataBody >
async decodeBody(type: string): Promise<any> {
if (type === "text") {
...
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, now the Deno compiler and the VSCode intellisense aren't angry with me 😄
I guess it's done... |
@shinspiegel it looks amazing, thanks |
decodeBody<T = any>(type: 'json'): Promise<T> so no need for a cast to have the right type. |
I think if the JSON is invalid should not return null but throw an exception because it's to the developer using it handle this case. |
A unhandle exception would broke the application, stopping from working, right?
This looks like a great idea! |
Yes, but how I know if the response body is null or if it's an invalid JSON, without exception? |
@shinspiegel sorry, you can not validate the types by
you should create your own validate function: type User = {
id: number
name: string
}
function isUser(v: any): v is User {
return typeof v === 'object' && v !== null && typeof v.id === 'number' && typeof v.name === 'string'
}
const body = await req.decodeBody('json')
if (isUser(body)) {
// body type is User
} else {
req.status(400).send("bad request")
} |
@ije what about the thowing exception on the JSON parse, should I improve it? |
@shinspiegel i think we should remove the try {
const data = await req.decodeBody('json')
req.status(200).send("hello " + data.name)
} catch(e) {
req.status(400).send("bad request")
} |
@shinspiegel do you have time to implement a |
Sure I'll make a new branch and start my next free night. That will be a excellent opportunity to learn a thing or two.
I'll change the try/catch block on the same branch/issue. |
This is a starting point for the discussion to this method.
In this way even if the JSON information is broken or it is a invalid will return
null
. What do you think?I can reduce the code the code to a single line or anything like that, but I think it's cleaner to see the intend in this way.
What I can to to improve it?