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

add some drafts #125

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add some drafts #125

wants to merge 1 commit into from

Conversation

dvv
Copy link

@dvv dvv commented Mar 30, 2023

Why

To not let #124 go in vain.

Problems addressed

  • custom file header text
  • process generics
  • decorate primitive builder to allow for schema appending via @schema .trim().email() or overriding via @schema z.coerce.int()
  • string-only unions issue z.enum([...])
  • unions issue z.discriminatedUnion(kind, ...) if @discriminator kind specified
  • import by the full path

I believe some of them can be pulled.

@dvv dvv force-pushed the main branch 2 times, most recently from 96f36d3 to 5f55cd6 Compare March 30, 2023 07:11
@fabien0102
Copy link
Owner

This looks cool, let’s remove the noises (commented code & todo) and ship some features from this PR :)

@dvv
Copy link
Author

dvv commented May 11, 2023

Cleaned up and rebased.

Copy link
Owner

@fabien0102 fabien0102 left a comment

Choose a reason for hiding this comment

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

Looks good, let's add few unit tests to ensure we are fixing real issues, and nobody breaks those later

(Sorry for the delay, I had some vacations 😇 )

Comment on lines +51 to +56
/**
* Generated file header
*
* @default "// Generated by ts-to-zod\nimport { z } from \"zod\";"
*/
headerText?: string;
Copy link
Owner

Choose a reason for hiding this comment

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

👌

Comment on lines +132 to +162
if (ts.isInterfaceDeclaration(node) || ts.isTypeAliasDeclaration(node)) {
if (schema !== undefined && node.typeParameters) {
const genericTypes = node
.typeParameters.map((p) => `${p.name.escapedText}`)
const genericDependencies = genericTypes.map((p) => getDependencyName(p))
dependencies = dependencies
.filter((dep) => !genericDependencies.includes(dep));
schema = f.createArrowFunction(
undefined,
genericTypes.map((dep) => f.createIdentifier(dep)),
genericTypes.map((dep) => f.createParameterDeclaration(
undefined,
undefined,
undefined,
f.createIdentifier(getDependencyName(dep)),
undefined,
f.createTypeReferenceNode(
f.createQualifiedName(
f.createIdentifier(zodImportValue),
f.createIdentifier(`ZodSchema<${dep}> = z.any()`)
),
undefined
),
undefined
)),
undefined,
f.createToken(ts.SyntaxKind.EqualsGreaterThanToken),
schema
);
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Let's add some unit tests on this

Comment on lines +256 to +258
if (schema.startsWith(".")) {
return f.createPropertyAccessExpression(generatedSchema, f.createIdentifier(schema.slice(1)));
}
Copy link
Owner

Choose a reason for hiding this comment

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

Let's add a unit test on this

Comment on lines +554 to +563
// string-only enum? issue z.enum
if (typeNode.types.every((i) =>
ts.isLiteralTypeNode(i) && i.literal.kind === ts.SyntaxKind.StringLiteral
)) {
return buildZodSchema(
z,
"enum",
[f.createArrayLiteralExpression(nodes.map((i) => (i as any)["literal"]))],
zodProperties
);
Copy link
Owner

Choose a reason for hiding this comment

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

Let's add a unit test on this

Comment on lines +606 to +612
jsDocTags.discriminator !== undefined
? "discriminatedUnion"
: "union",
jsDocTags.discriminator !== undefined
? [f.createStringLiteral(jsDocTags.discriminator),
f.createArrayLiteralExpression(values)]
: [f.createArrayLiteralExpression(values)],
Copy link
Owner

Choose a reason for hiding this comment

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

Same here, a little unit test would be perfect :)

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.

None yet

2 participants