Migrate to Zod #808
Migrate to Zod #808
Conversation
|
@ryscheng i have added a samlpe zod url schema and a validateObjectZod function . Is this how you are expecting the migration to zod ? Should i implement similarly for other types too ? Plz suggest any improvements or flaws in my approach |
src/resources/schema/url.ts
Outdated
| @@ -0,0 +1,7 @@ | |||
| import { z } from "zod"; | |||
There was a problem hiding this comment.
Instead of hard-coding the schemas using Zod, I'd suggest we leave it as JSON Schema and convert it at build/test time to the respective library.
For example, you should be able to use this for Zod.
https://www.npmjs.com/package/json-schema-to-zod
In the future, we may also want to do something similar to Python Pydantic
https://github.com/richard-gyiko/json-schema-to-pydantic
src/validator/index.ts
Outdated
| } | ||
|
|
||
| const errors: Record<string, string> = {}; | ||
| const formatted = result.error.format(); |
There was a problem hiding this comment.
It looks like formatted errors are deprecated?
https://zod.dev/error-formatting?id=zformaterror
I also don't think this is necessary
You can just get a list of issues
https://zod.dev/basics?id=handling-errors
|
@ryscheng i have made the changes you asked for , plz review |
| "types:json": "pnpm json2ts -i './src/resources/schema/' -o 'src/types' --cwd './src/resources/schema'", | ||
| "types:mv": "find src/types/ -name '*.d.ts' -type f | sed s/\\.d.ts$// | xargs -I _ bash -c 'mv _.d.ts _.ts'", | ||
| "types": "pnpm types:zod", | ||
| "types:zod": "pnpm json-schema-to-zod -i src/resources/schema/blockchain-address.json -o src/types/blockchain-address.ts -n BlockchainAddressSchema -t BlockchainAddress --withJsdocs && pnpm json-schema-to-zod -i src/resources/schema/collection.json -o src/types/collection.ts -n CollectionSchema -t Collection --withJsdocs && pnpm json-schema-to-zod -i src/resources/schema/named.json -o src/types/named.ts -n NamedSchema -t Named --withJsdocs && pnpm json-schema-to-zod -i src/resources/schema/project.json -o src/types/project.ts -n ProjectSchema -t Project --withJsdocs && pnpm json-schema-to-zod -i src/resources/schema/social-profile.json -o src/types/social-profile.ts -n SocialProfileSchema -t SocialProfile --withJsdocs && pnpm json-schema-to-zod -i src/resources/schema/url.json -o src/types/url.ts -n UrlSchema -t URL --withJsdocs", |
There was a problem hiding this comment.
Is there no way to glob for the files? (e.g. **/*.json)
| [k: string]: unknown; | ||
| } | ||
| /**A project is a collection of artifacts*/ | ||
| export const ProjectSchema = z.object({ "version": z.number(), "name": z.string(), "display_name": z.string(), "description": z.string().optional(), "websites": z.array(z.any()).optional(), "social": z.any().optional(), "github": z.array(z.any()).optional(), "npm": z.array(z.any()).optional(), "crates": z.array(z.any()).optional(), "pypi": z.array(z.any()).optional(), "go": z.array(z.any()).optional(), "open_collective": z.array(z.any()).optional(), "blockchain": z.array(z.any()).optional(), "defillama": z.array(z.any()).optional(), "comments": z.array(z.string()).optional() }).describe("A project is a collection of artifacts") |
There was a problem hiding this comment.
This doesn't look right. We shouldn't be use any() anywhere
| [k: string]: unknown; | ||
| } | ||
| /**All social profile*/ | ||
| export const SocialProfileSchema = z.object({ "farcaster": z.array(z.any()).optional(), "medium": z.array(z.any()).optional(), "mirror": z.array(z.any()).optional(), "telegram": z.array(z.any()).optional(), "twitter": z.array(z.any()).optional() }).describe("All social profile") |
| for (const [key, value] of Object.entries(formatted)) { | ||
| if (key === "_errors") continue; | ||
|
|
||
| if (value && typeof value === "object" && "_errors" in value) { |
There was a problem hiding this comment.
This still looks wrong, I don't think we should be unsafely casting as a ZodFormattedError and directly accessing _errors. Can you check the Zod documentation and please use the typed accessors?
|
@Haxry this PR has been open for a few months and you still haven't responded to the latest comment from our reviewer. If I don't hear back from you in a week, I will close this PR. |
3 participants
No description provided.