-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Removes instances of "any" and unnecessary type casts #236
base: master
Are you sure you want to change the base?
Removes instances of "any" and unnecessary type casts #236
Conversation
import { Request } from "express"; | ||
import { UserDocument } from "../models/User"; | ||
|
||
export interface Req extends Request { | ||
user: UserDocument; | ||
} |
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.
I''m not sure if that is the best approach. In TS one usually augments missing changes using the type definition file (declaration merging), as we don't own the Request type:
E.g., for the user
property on the Request
:
https://github.com/microsoft/TypeScript-Node-Starter/pull/227/files
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.
Thanks for the comment. Indeed, module augmentation may be the more idiomatic way to go about it, although I like the explicitness of wrapping ( extending ) a given type. Furthermore, it would be nice if the express's Request
type were generic, similar to how RouteComponentProps accepts a parameter representing the type of the matched path params, however this is a topic for a different discussion. To get back to the main story line, why wasn't #227 merged yet ( #236 could build off of it )?
Seem to repeat many edits from #232 as well, we really need some merges or comments, @peterblazejewicz |
fbc1328
to
1e15e8b
Compare
No description provided.