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

Convert to TypeScript #15

Merged
merged 102 commits into from Mar 18, 2022
Merged

Convert to TypeScript #15

merged 102 commits into from Mar 18, 2022

Conversation

NuroDev
Copy link
Contributor

@NuroDev NuroDev commented Jan 21, 2022

What

This PR migrates the code base to using TypeScript, as recommended by @leerob in #3.
It also includes a number of other changes, including the following:

  • Migrate entire codebase to TypeScript
  • Add global / reusable fetcher function (lib/fetcher)
  • Adds request methods bouncers to "most" API endpoints
  • Added @/types import alias
  • Added types/ directory for all custom types
  • Removed some unnecessary console.log's
  • Removed jsconfig.json
  • Minor api endpoint restructuring for domain endpoints
  • Renamed GitHub client ID/secret env vars to better fitting names
  • Lots of cleanup / code standard improvements
  • Move major API functionality to lib/ directory

@steven-tey
Copy link
Contributor

steven-tey commented Feb 12, 2022

@NuroDev I resolved some merged conflicts but I'm currently running into the Error: PrismaClient is unable to be run in the browser. error on all the post slug pages (e.g. http://demo.localhost:3000/platforms-starter-kit):
CleanShot 2022-02-11 at 16 24 14

Will look into this more next week but if you wanna take a stab at it in the meantime feel free to do so!

# Conflicts:
#	pages/_middleware.js
#	pages/app/post/[id]/index.tsx
#	pages/app/post/[id]/settings.tsx
@NuroDev
Copy link
Contributor Author

NuroDev commented Feb 16, 2022

@steven-tey I've had a look into this Prisma issue a bit more and, as the error implies, the Prisma client instance is somewhere being leaked into the browser, which is not to be expected.

I've not been able to track down the exact cause but tracked down the issue to the following commit: NuroDev@ccf348a. Seems at some point during that main branch merge the conflict occured.

@Rocinante89
Copy link
Contributor

Rocinante89 commented Mar 3, 2022

@NuroDev I opened a PR that should hopefully fix the above

@leerob
Copy link
Member

leerob commented Mar 3, 2022

Amazing! Thank you so much.

@NuroDev
Copy link
Contributor Author

NuroDev commented Mar 4, 2022

Thanks for the help @Rocinante89, merged that change that actually was needed.
However it sadly did not fix the issue. After finally looking into this more I found the actual cause. Seems I mistakenly removed the prisma prop being drilled to the replaceExamples remark plugin in favour of importing prisma directly in the @/lib/prisma file, thereby exposing it to the browser.

This should now be fixed if anyone wants to give it a test 🙂

@cezarneaga
Copy link
Contributor

hi there,
is there any blocker still left here?
willing to pitch in if there is something that need be done

cheers,
C

@NuroDev
Copy link
Contributor Author

NuroDev commented Mar 18, 2022

is there any blocker still left here?

Not as far as I am aware. Just waiting on final review & approval from someone at the @vercel team so we can get this merged

Copy link
Contributor

@steven-tey steven-tey left a comment

Choose a reason for hiding this comment

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

This has been a work in progress for a while, thanks a lot for your hard work @NuroDev! I just got around to reviewing this fully and I'm gonna merge this to main now! 🚀

@steven-tey steven-tey merged commit 327dfa1 into vercel:main Mar 18, 2022
@NuroDev NuroDev deleted the feature/typescript branch March 18, 2022 17:13
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

10 participants