search

stepzen-dev / custom-site-template

template repo for end-to-end demo
0 stars 0 forks source link

Possible improvements #1

Open royderks opened 2 years ago

royderks commented 2 years ago

I'd add this logic to a separate function in possibly a separate file. It's configuration logic that clutters the code on the rest of this page: https://github.com/stepzen-samples/custom-site-template/blob/297a541c4ac6d5339e8b249fc5e01c7ee54170b3/pages/api/stepzen_fetch.js#L4

You could also add this to a Next.js getServerProps function instead. This way you don't have to create an API Route and don't have to fetch "twice" in your UI component. As a nice addition, have a look at https://github.com/prisma-labs/graphql-request instead of fetch. As this way you can use GQL. https://github.com/stepzen-samples/custom-site-template/blob/297a541c4ac6d5339e8b249fc5e01c7ee54170b3/pages/api/stepzen_fetch.js#L25

A lot of code is duplicated now. I would check for the presence of the data of each individual API inline, this will save you lots of lines of code - which makes it better readable and understandable: https://github.com/stepzen-samples/custom-site-template/blob/297a541c4ac6d5339e8b249fc5e01c7ee54170b3/pages/index.js#L24

With graphql-request you don't have to dive this deep into the data object: https://github.com/stepzen-samples/custom-site-template/blob/297a541c4ac6d5339e8b249fc5e01c7ee54170b3/pages/index.js#L26

Why not use a .map() to iterate over these arrays? https://github.com/stepzen-samples/custom-site-template/blob/297a541c4ac6d5339e8b249fc5e01c7ee54170b3/pages/index.js#L49 https://github.com/stepzen-samples/custom-site-template/blob/297a541c4ac6d5339e8b249fc5e01c7ee54170b3/pages/index.js#L61

Cerchie commented 2 years ago

Thanks Roy! Item 1, separating the variable massage logic into a different file, is complete. Ran into an issue while trying to implement getServerSideProps

Screen Shot 2021-11-17 at 11 30 13 AM

: Looking at graphql-request now.

royderks commented 2 years ago

Thanks Roy! Item 1, separating the variable massage logic into a different file, is complete. Ran into an issue while trying to implement getServerSideProps Screen Shot 2021-11-17 at 11 30 13 AM : Looking at graphql-request now.

You can actually delete the whole api route and do the logic you have there inside the getServerSideProps. The request will also be done server side, so no worries about leaking sensitive information

Cerchie commented 2 years ago

"Safer bet would be to get the github and twitter username from the Github and twitter fields instead of devto" -- need to change the query too