Skip to content

Switch to ZOD#1565

Open
greysilly7 wants to merge 5 commits intospacebarchat:masterfrom
greysilly7:zod-conversion
Open

Switch to ZOD#1565
greysilly7 wants to merge 5 commits intospacebarchat:masterfrom
greysilly7:zod-conversion

Conversation

@greysilly7
Copy link
Copy Markdown
Contributor

It ran compiles, fermi connects, openapi generates, probally didnt fully utilize zods validators though

@TheArcaneBrony
Copy link
Copy Markdown
Member

this... looks like a nightmare to maintain

@MaddyUnderStars
Copy link
Copy Markdown
Member

@TheArcaneBrony what makes you say that? zod is great and looking through the diff, it looks much simpler. removes ajv entirely. I always hated things like these comments

/**
* @minLength 6
* @maxLength 6
*/
code?: string; // 2fa code

Copy link
Copy Markdown
Member

@MaddyUnderStars MaddyUnderStars left a comment

Choose a reason for hiding this comment

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

  • need to run prettier
  • need to fix the test ci
  • should do a pass and simplify some of the schemas. the ChannelModifySchema for example, you've set all of the properties to optional rather than just using a .partial() on the parent object

@TheArcaneBrony
Copy link
Copy Markdown
Member

I feel like those comments are perfectly fine? On the other hand, that specific case should be handled in code rather than schema validation... (ie. for use with 8 or 10 digit TOTP codes)

@greysilly7
Copy link
Copy Markdown
Contributor Author

  • need to run prettier
  • need to fix the test ci
  • should do a pass and simplify some of the schemas. the ChannelModifySchema for example, you've set all of the properties to optional rather than just using a .partial() on the parent object

The CI fails on main too so I feel it's kind of out of scope of this PR especially since it still fails after running the linter plus nix CI succeeds. I went through somewhat and did psrtials I'm at work though now so I can't do anything more.

@MathMan05
Copy link
Copy Markdown
Contributor

I though ajv was unable to work with GW, if we could get something to work there, that'd be great

@greysilly7
Copy link
Copy Markdown
Contributor Author

greysilly7 commented Feb 26, 2026

Zod should be able to work with gateway since it just parses and validates data, I was working on converting more stuff to Zod since this just did the API, I've got some partial progress but I feel that's another PR?

@MathMan05
Copy link
Copy Markdown
Contributor

Hey, just gonna point this out now. I don't think this works at all. Zod does not have a way to do type cohesion everywhere which is required here.

@greysilly7
Copy link
Copy Markdown
Contributor Author

Hey, just gonna point this out now. I don't think this works at all. Zod does not have a way to do type cohesion everywhere which is required here.

What do you mean we literally have z.infer giving us a ts type??

@MathMan05
Copy link
Copy Markdown
Contributor

cohesion, not checking

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.

4 participants