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

feat-#3: Made the entire backend code from javascript to typescript #466

Merged
merged 6 commits into from
Aug 27, 2024

Conversation

Ameerjafar
Copy link
Contributor

@Ameerjafar Ameerjafar commented Aug 19, 2024

Summary

Made the entire backend code from javascript to typescript.

Description

2024-08-19.21-31-54.mp4

currently in the project the backend code has not written by the typescript This pr converted the entire backend javascript to typescript.

Issue(s) Addressed

Enter the issue number of the bug(s) that this PR fixes

Prerequisites

Copy link

vercel bot commented Aug 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
wanderlust ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 1:47pm
wanderlust-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 1:47pm

Copy link

Hey @Ameerjafar! Thanks for sticking to the guidelines! High five! 🙌🏻

@Ameerjafar
Copy link
Contributor Author

@krishnaacharyaa can you review the pr sir

Copy link

Hey @Ameerjafar! Thanks for sticking to the guidelines! High five! 🙌🏻

@Ameerjafar
Copy link
Contributor Author

@krishnaacharyaa

@Ameerjafar Ameerjafar closed this Aug 20, 2024
@Ameerjafar Ameerjafar reopened this Aug 20, 2024
Copy link
Owner

@krishnaacharyaa krishnaacharyaa left a comment

Choose a reason for hiding this comment

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

Thank you for taking time and refactoring the code to ts, much appreciated,
Kindly address the review comments, looks good to me

p.s: Sorry didn't get time to review the PR because of office work

deleteDataFromCache(REDIS_KEYS.ALL_POSTS), // Invalidate cache for all posts
deleteDataFromCache(REDIS_KEYS.FEATURED_POSTS), // Invalidate cache for featured posts
deleteDataFromCache(REDIS_KEYS.LATEST_POSTS), // Invalidate cache for latest posts
// deleteDataFromCache(REDIS_KEYS.ALL_POSTS), // Invalidate cache for all posts
Copy link
Owner

Choose a reason for hiding this comment

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

What is the problem basically we are facing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[ErrorReply: NOAUTH Authentication required.] this is the error i got

Copy link
Owner

Choose a reason for hiding this comment

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

Can you please figure out why we require this,
Have you setup redis locally in the said port given in the env?

Once run redis locally, and see if you can figure out something from it, you would mostly like be able to do 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.

@krishnaacharyaa yes sir it is working well I just gave the correct port number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@krishnaacharyaa it shows the error in the backend/dist/server.js file (Object.defineProperty(exports, "__esModule", { value: true });) exports is not defined in the es module scope.

Copy link
Owner

Choose a reason for hiding this comment

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

Are we good to merge or still there is error?

Copy link
Contributor Author

@Ameerjafar Ameerjafar Aug 27, 2024

Choose a reason for hiding this comment

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

now the deletecache function is working perfectly @krishnaacharyaa

@@ -1,6 +1,19 @@
{
"type": "module",
"type": "commonjs",
"dependencies": {
Copy link
Owner

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

@Ameerjafar Ameerjafar Aug 24, 2024

Choose a reason for hiding this comment

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

In the type module we cannot use the exports or module.exports(we have used this is somewhere in our codebase i have checked but i don't know where we have used that. this is the reason i have useed type="commonjs" @krishnaacharyaa

Copy link
Owner

Choose a reason for hiding this comment

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

But module is better than commonjs,

we have used this is somewhere in our codebase i have checked but i don't know where we have used that
Can you figure out where and rectify by searching ctrl + shift +f

If not only then we'll change,
Let's not change because we are getting some error, instead lets see which is best commonjs or module? And then change our codebase

Copy link
Contributor Author

@Ameerjafar Ameerjafar Aug 24, 2024

Choose a reason for hiding this comment

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

In the ts config also they the mentioned module as common js @krishnaacharyaa

Copy link
Owner

Choose a reason for hiding this comment

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

@Sukomal07 what should we ideally use ?
Any take

Copy link
Contributor

Choose a reason for hiding this comment

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

@krishnaacharyaa Yeah, module is better than commonjs.

Copy link
Contributor

@Sukomal07 Sukomal07 Aug 26, 2024

Choose a reason for hiding this comment

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

In the type module we cannot use the exports or module.exports(we have used this is somewhere in our codebase i have checked but i don't know where we have used that. this is the reason i have useed type="commonjs" @krishnaacharyaa

Use export syntax instead of module.exports
example:
export const replaceStr = (str, char, replacer) => { const regex = new RegExp(char, "g") const replaced = str.replace(regex, replacer) return replaced }
export async function main{ console.log("Hi there") }
export default async function main{ console.log("Hi there") }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Sukomal07 This is the js code compiled by the ts compiler, In my ts code i did not use the module.exports.

@@ -0,0 +1,108 @@
{
Copy link
Owner

Choose a reason for hiding this comment

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

Can we clean this up please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I remove the full command code ?

Copy link
Owner

Choose a reason for hiding this comment

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

commented code, please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done sir @krishnaacharyaa

@Ameerjafar
Copy link
Contributor Author

@krishnaacharyaa

@krishnaacharyaa
Copy link
Owner

Thoda late karpaaungaa reviews, sorry yaar, kaam bohot hei, don't mind
I'll not be able to respond your messages in github/discord

@Ameerjafar
Copy link
Contributor Author

@Sukomal07

@Ameerjafar
Copy link
Contributor Author

@krishnaacharyaa i have done if you time please review the pr sir. I have keep the type as module in the package.json file.

Copy link
Owner

@krishnaacharyaa krishnaacharyaa left a comment

Choose a reason for hiding this comment

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

Thank you for the efforts @Ameerjafar, Much appreciated !

@krishnaacharyaa krishnaacharyaa merged commit 778bec3 into krishnaacharyaa:main Aug 27, 2024
3 checks passed
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.

Refactoring backend using typescript
3 participants