-
Notifications
You must be signed in to change notification settings - Fork 0
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: convert last zod usages to typebox #1232
Conversation
Reviewer's Guide by SourceryThis pull request replaces Zod with TypeBox for schema validation in the dapp. The changes include updating the environment configuration, pincode form, and create API key form to use TypeBox schemas and resolvers. Zod has also been removed as a dependency. Updated class diagram for ServerEnvironmentclassDiagram
class ServerEnvironment {
<<Type>>
+BETTER_AUTH_URL: string | undefined
+NEXT_PUBLIC_APP_URL: string | undefined
+NEXTAUTH_URL: string | undefined
+SETTLEMINT_HASURA_ADMIN_SECRET: string
+RESEND_API_KEY: string | undefined
+GOOGLE_CLIENT_ID: string | undefined
+GOOGLE_CLIENT_SECRET: string | undefined
+GITHUB_CLIENT_ID: string | undefined
+GITHUB_CLIENT_SECRET: string | undefined
+SETTLEMINT_HD_PRIVATE_KEY: string
+APP_URL: string | undefined
}
Updated class diagram for ClientEnvironmentclassDiagram
class ClientEnvironment {
<<Type>>
+NEXT_PUBLIC_EXPLORER_URL: string | undefined
}
Updated class diagram for CreateApiKeyFormValuesclassDiagram
class CreateApiKeyFormValues {
<<Type>>
+name: string
+expiresIn: string
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @roderik - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider creating a shared utility function for constructing TypeBox schemas with consistent metadata like
$id
. - It might be worth adding a changeset to document the removal of
zod
.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Review instructions: 2 issues found
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
// TypeBox schema for the pincode form | ||
const pincodeSchema = t.Object( | ||
{ | ||
pincodeName: t.String({ minLength: 1, error: "Name is required" }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (testing): Double-check that typebox custom error messages render as intended.
Since you are migrating from zod, ensure that the provided 'error' messages (e.g. for 'pincodeName' and 'pincode') propagate correctly through the typeboxResolver and are displayed appropriately in the UI.
GOOGLE_CLIENT_SECRET: z.string().optional(), | ||
GITHUB_CLIENT_ID: z.string().optional(), | ||
GITHUB_CLIENT_SECRET: z.string().optional(), | ||
import { safeParse, t, type StaticDecode } from "@/lib/utils/typebox"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (review_instructions): The code is switching from Zod to TypeBox for schema validation, which is a good modernization, but there's a potential issue with error handling in the safeParse function.
The new implementation uses safeParse but doesn't appear to handle validation errors explicitly like Zod's .parse() method would throw. Make sure the safeParse function properly handles validation failures and throws appropriate errors to maintain the same level of validation safety that was present in the original code.
Review instructions:
Path patterns: **/*.ts
Instructions:
Always write correct, up to date, bug free, fully functional and working, secure, performant and efficient code.
} | ||
|
||
export function getClientEnvironment(): ClientEnvironment { | ||
return clientEnvironmentSchema.parse(process.env); | ||
return safeParse(clientEnvironmentSchema, process.env); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (review_instructions): The getClientEnvironment function doesn't handle potential validation errors from safeParse.
Unlike Zod's parse method which would throw on validation errors, it's unclear if safeParse throws or returns a result object with validation status. If it returns an object with success/error properties (similar to Zod's safeParse), you should check for validation success before returning the result to maintain the same error behavior as the original code.
Review instructions:
Path patterns: **/*.ts
Instructions:
Always write correct, up to date, bug free, fully functional and working, secure, performant and efficient code.
…rice * main: chore(deps): update dependency @types/node to v22.13.13 (#1244) chore(deps): update github/codeql-action digest to 1b549b9 (#1241) feat: add input suggestions based on history (#1240) feat: add issue button to all table tabes (#1239) fix: type the validated fields array (#1238) fix: user balances in popup (#1237) fix: page titles and graphs (#1235) fix: rtl issues (#1233) feat: convert last zod usages to typebox (#1232) fix: collateral proof validity (#1231) fix: translated errors (#1229) fix: missing event details (#1230) fix: formfield postfix ui (#1228) fix: asset details and actions (#1227)
Summary by Sourcery
Migrates from Zod to TypeBox for schema validation in environment configuration and form components.
Enhancements: