-
Notifications
You must be signed in to change notification settings - Fork 203
Migrate to Zod #808
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
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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") |
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.
Not any()
, should be URLs
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
No description provided.