-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix-#3: refactored-backend-with-typescript #408
fix-#3: refactored-backend-with-typescript #408
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hey @vamsidhar-914! Thanks for sticking to the guidelines! High five! 🙌🏻 |
@krishnaacharyaa for tests I might need some help.I didnt touched tests for refactoring |
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.
HI @vamsidhar-914 appreciate the efforts, kindly don't make any changes to the codebase, just the refactoring part to the ts and don't remove the codes or comment if you get errors, kindly address all the reviews :)
fullName: string; | ||
email: string; | ||
password: string; | ||
avatar?: string; |
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.
Give here password string | undefined, so that it will let you define undefined
import mongoose from 'mongoose'; | ||
|
||
export interface IUser extends Document { | ||
_id: string; |
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.
Use types instead of interface and remove the I prefix in the naming
|
||
user.refreshToken = refreshToken; | ||
|
||
await user.save(); | ||
user.password = undefined; | ||
// user.password = undefined; | ||
|
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.
Make the password field nullable in the type, so that you don't have to comment it for the ts error
@@ -95,7 +104,6 @@ export const signInWithEmailOrUsername = asyncHandler(async (req, res) => { | |||
|
|||
user.refreshToken = refreshToken; | |||
await user.save(); | |||
user.password = undefined; | |||
|
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.
Same, get it back
@@ -164,12 +178,6 @@ export const isLoggedIn = asyncHandler(async (req, res) => { | |||
console.log(error); | |||
} | |||
} | |||
const user = await User.findById(_id); | |||
if (!user) { |
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.
Why have we tottally removed this? please don't remove some functionality just because it is giving error XD
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.
not remove d but declared on above
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.
name of the file post-type
import { IUser } from './user-Type'; | ||
|
||
export interface IPost extends Document { | ||
_id?: string; |
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.
make it type and why does it extend Document? who is document and rename we don't need I prefix
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.
name
import { Request } from 'express'; | ||
|
||
interface RequestUserType { | ||
_id: string; |
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.
convert to type
} | ||
|
||
export interface RequestWithUserRole extends Request { | ||
user?: RequestUserType; |
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.
Do we actually need this? Can't we directly use the above ? make it both to type
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.
in middleware req.user , user is not found in Request type...extended the Request Type added user filed
@vamsidhar-914 please google these, it will be available.
And the request type as discussed would not be actually required, try to come up with alternative |
@vamsidhar-914 Hi, Are we good or blocked? |
@krishnaacharyaa sorry! been busy lately.....everything is done except for the type module issue...searched google but couldn't find it. |
No worries can we arrange a call we'll debug |
can we connect tomorrow morning? |
Nights or sat Sun |
sure....lets connect on sunday |
@vamsidhar-914 the earlier we get this, better it is :) |
Summary
refactored backend with typescript
Description
migrated all js files to ts files with all the types.
Issue(s) Addressed
Closes #3
Prerequisites