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

Add runtime type check #66

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

Add runtime type check #66

wants to merge 2 commits into from

Conversation

luizakp
Copy link
Contributor

@luizakp luizakp commented Nov 25, 2022

No description provided.

@vercel
Copy link

vercel bot commented Nov 25, 2022

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

Name Status Preview Updated
generalmagic ✅ Ready (Inspect) Visit Preview Nov 25, 2022 at 1:50PM (UTC)

Copy link
Contributor

@marthendalnunes marthendalnunes left a comment

Choose a reason for hiding this comment

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

Overall the changes are good, just check the proper error message handling and the optional helper function to keep your code cleaner :D

discord || telegram || github || projectLink || aditionalInformation
? 'Aditional information'
: ''
const discordHandle = discord ? `Discord handle: ${discord}` : ''
Copy link
Contributor

Choose a reason for hiding this comment

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

To keep it DRY, I recommend creating a helper function that takes the optional data variable and its message and returns an empty string, and replaces all of these ternary operators.

something like that

// Helper function that returns an empty string if the optional data is falsy
    const handleOptionalData = (optionalData: string, message: string) => optionalData ? message : ''

// instead of 
const discordHandle = discord ? `Discord handle: ${discord}` : ''

// replacing for
const discordHandle = handleOptionalData(discord, `Discord handle: ${discord}`)

Comment on lines +96 to +101
} catch (err) {
console.log(err)
res.status(400).json({
success: false,
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of an error in the execution of the endpoint is always good to also send the error message. Getting the message with the correct typing from an error response is a bit tricky, but here's a solution:

Suggested change
} catch (err) {
console.log(err)
res.status(400).json({
success: false,
})
}
} catch (err) {
console.log(err)
let message
if (err instanceof Error) message = err.message
else message = String(err)
res.status(400).json({
success: false,
message
})
}


export const SubmitSchema = z.object({
firstName: nonEmptyStringContraint,
email: nonEmptyStringContraint,
Copy link
Contributor

Choose a reason for hiding this comment

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

Zod string objects has a built-in method for checking the email syntax:

Suggested change
email: nonEmptyStringContraint,
email: nonEmptyStringContraint.email(),

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.

2 participants