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

Moving core Action types to Zod schema as source of truth #197

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

mattschoch
Copy link
Contributor

No description provided.

Comment on lines +80 to +86
export const ActionSchema = z.nativeEnum(Action)

export const BaseActionSchema = z.object({
action: ActionSchema,
nonce: z.string()
})
export type BaseActionSchema = z.infer<typeof BaseActionSchema>
Copy link
Collaborator

@wcalderipe wcalderipe Mar 27, 2024

Choose a reason for hiding this comment

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

Please move the schema to their own files like at src/lib/schema like the others.

Also, we're using camelCase plus the schema prefix as the naming convention. For example, from BaseActionSchema to baseActionSchema.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://narvalxyz.slack.com/archives/C067Q1TDA9G/p1711552650935279

I'm proposing that we do NOT put schema in their own files, and that we PascalCase lol

BaseActionSchema is a bit of a weird one, I haven't gotten it to map properly to our existing Action const type, so that's why it still has the word schema in the name. The goal is that there is just BaseAction.

@mattschoch mattschoch requested a review from wcalderipe March 27, 2024 19:54
@mattschoch mattschoch force-pushed the chore/type-to-zod-refactor branch from a729d3b to acf4096 Compare March 28, 2024 13:45
@mattschoch mattschoch merged commit 7a82d43 into main Mar 28, 2024
3 checks passed
@mattschoch mattschoch deleted the chore/type-to-zod-refactor branch March 28, 2024 14:42
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