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

complete project migration to typescript with additional @types packages #26

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SandipGyawali
Copy link

@SandipGyawali SandipGyawali commented May 15, 2024

Issue: #18

  1. project successfully migrated to typescript without any issues.
  2. addition package addition @types for type saftey
  3. tailwindcss applied for .ts and .tsx files
  4. vite migrated to @plugin-react-swc
    Note: Also no additional issues in the browser console.

After migration result:
image

@GitGudCode440
Copy link
Contributor

GitGudCode440 commented May 17, 2024

I have cloned your repository, and looked at your code. So far, so good. However there are a few minor issues with the code that I would like to address:

  1. The commit pushes the old pnpm-lock.yaml file right back into the main, which has been corrected in commit a3d67d4. If you can revert pnpm-lock.yaml from the commit, that would solve issues.

  2. Why have a seperate network url, when localhost does the job in dev?

network

@GitGudCode440
Copy link
Contributor

@ArvindParekh What's your input on the PR?

@SandipGyawali
Copy link
Author

@GitGudCode440 i will fix the issues and commit it

@SandipGyawali
Copy link
Author

@ArvindParekh What's your input on the PR?

@ArvindParekh
Copy link
Owner

Hey @SandipGyawali @GitGudCode440 apologies for getting back late. Thanks for the PR! However, I think this'll have to wait a bit. I've applied to MLH fellowship and submitted this project in JavaScript, so until that is sorted out, maybe we should put a hold on the conversion to typescript. But will definitely get to it, once that is done.

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

3 participants