-
Notifications
You must be signed in to change notification settings - Fork 50
Zod V4 implementation #701
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?
Conversation
Ready for testing - but expect rough edges.
… in case it inspires use cases in others)
in #707 i put up some fixes but I couldn't remember the right way to try to upstream them to yours. let me know if I should fork your fork or if I could have permissions to push to that branch to collaborate
It's really promising! Thanks for all the work |
optional: z.object({ a: z.string(), b: z.number() }).optional(), | ||
// For Convex compatibility, we need to avoid patterns that produce undefined | ||
// Instead, use union with null for nullable fields and .optional() for optional fields | ||
nullableOptional: z.union([z.string(), z.null()]).optional(), |
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 seem to be a nullable optional..
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.
Yeah..this might be a hangover from before I revised the approach to optionals (overhauled it at one point during writing to recognise that objects can happily handle optionals without any special treatment)
The tests are failing but I ran it through AI and was able to fix all outstanding issues. I can commit if you like? |
@mikecann I'm happy if you're happy! Added you as collaborator too in case it makes things easier somehow. |
I'll close out #707 as I just was able to push up my changes here |
All tests pass thanks to Claude so long as the expected values in the original test have the correct assumptions
Okay I have added a commit that fixes the tests. Just to be clear the AI did all the hard work here based upon the expected values found in the failing tests. So hopefully those expected values were correct otherwise these fixes will be incorrect. I don't have enough context on Zod4 to say either way if this is correct or not but after scanning through the 3k lines of tests it all looks okay 🤷 I do like the look of the bidirectional schema that looks nice. |
Is there a package with zod v4 support on npm? The latest version of npm is 0.1.101-alpha.0 |
@hasanaktas the latest alpha now has these changes |
commit: |
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.
will the peer dependency be upgraded zod v4 for the alpha package? package.json still has "zod": "^3.22.4"
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.
I think the tuple handling can be improved :)
// If all items have the same kind, use that as the element type | ||
const firstKind = convexItems[0]?.kind; | ||
const allSameKind = convexItems.every((item) => item.kind === firstKind); | ||
|
||
if (allSameKind && convexItems.length > 0) { | ||
// All elements are the same type, use that as the array element type | ||
return v.array(convexItems[0]!); | ||
} else if (convexItems.length === 1) { | ||
// Single element tuple | ||
return v.array(convexItems[0]!); | ||
} else if (convexItems.length >= 2) { | ||
// Multiple different types, create a union for the array element type | ||
const unionValidator = v.union( | ||
convexItems[0]!, | ||
convexItems[1]!, | ||
...convexItems.slice(2), | ||
); | ||
return v.array(unionValidator); | ||
} else { | ||
return v.array(v.any()); | ||
} |
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 sure this is the correct implementation since this will incorrectly collapse multi-member tuples by shared “kind,” using only the first item’s Convex shape and dropping the rest; arrays derived from tuples then validate a single shape instead of a union of all tuple member shapes.
Because it compared only the Convex “kind” of each tuple item and, if they matched (e.g., all were objects), it picked the first item’s validator for the entire array element. That collapses distinct tuple shapes (like different literal-discriminated objects) into one, silently dropping the rest. Tuples require preserving each member’s shape, so the array element must be a union of all converted items, not just the first.
Here's a failing test case:
test("tuple with multiple distinct object members becomes array of union element type", () => {
const A = z.object({ key: z.literal("a"), v: z.string() });
const B = z.object({ key: z.literal("b"), x: z.number() });
const C = z.object({ key: z.literal("c"), y: z.boolean() });
const schema = z.object({ props: z.tuple([A, B, C]) });
const convexValidator = zodToConvex(schema);
expect(convexValidator.kind).toBe("object");
type LiteralKey = { kind: "literal"; value: string };
type ObjectMember = { kind: "object"; fields: { key: LiteralKey } };
type UnionElement = { kind: "union"; members: ObjectMember[] };
type ArrayValidator = { kind: "array"; element: UnionElement };
const props = convexValidator.fields.props as unknown as ArrayValidator;
expect(props.kind).toBe("array");
const elem = props.element;
expect(elem.kind).toBe("union");
expect(elem.members.length).toBe(3);
// Validate each union member shape retains its literal key
const keys = elem.members.map((m) => m.fields.key.value).sort();
expect(keys).toEqual(["a", "b", "c"]);
});
Here's a possible improvement:
diff --git a/packages/convex-helpers/server/zodV4.ts b/packages/convex-helpers/server/zodV4.ts
index 97c0c7b..ec216e2 100644
--- a/packages/convex-helpers/server/zodV4.ts
+++ b/packages/convex-helpers/server/zodV4.ts
@@ -2033,8 +2033,6 @@ function convertZodTupleToConvex(
const items = actualValidator.def.items as z.ZodTypeAny[];
// Tuples should become arrays in Convex, not objects
- // We'll use the first element's type as the array element type
- // If there are multiple different types, we'll create a union
if (items.length === 0) {
return v.array(v.any());
}
@@ -2044,27 +2042,19 @@ function convertZodTupleToConvex(
useRecursiveCall ? zodToConvexInternal(item) : zodToConvex(item),
);
- // If all items have the same kind, use that as the element type
- const firstKind = convexItems[0]?.kind;
- const allSameKind = convexItems.every((item) => item.kind === firstKind);
-
- if (allSameKind && convexItems.length > 0) {
- // All elements are the same type, use that as the array element type
- return v.array(convexItems[0]!);
- } else if (convexItems.length === 1) {
- // Single element tuple
+ // If there's only one item, it's a simple homogeneous array
+ if (convexItems.length === 1) {
return v.array(convexItems[0]!);
- } else if (convexItems.length >= 2) {
- // Multiple different types, create a union for the array element type
- const unionValidator = v.union(
- convexItems[0]!,
- convexItems[1]!,
- ...convexItems.slice(2),
- );
- return v.array(unionValidator);
- } else {
- return v.array(v.any());
}
+
+ // For multiple tuple members, always create a union of ALL item validators.
+ // Using only the "kind" (e.g., all objects) loses distinct shapes like different literals.
+ const unionValidator = v.union(
+ convexItems[0]!,
+ convexItems[1]!,
+ ...convexItems.slice(2),
+ );
+ return v.array(unionValidator);
}
} | ||
|
||
// For optional fields, create a union with null instead of using v.optional() | ||
// This matches the expected behavior where optional Zod types become unions |
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.
What "expected behavior' is this referring to? My two cents is that it's not intuitive that an .optional()
Zod field turns into a union. This prevents me from adding an optional field in Convex, backfilling some data, and then tightening up the validation to something stricter like string | null
.
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 is likely the result of me trying to fix the broken tests and not fully understanding what the expected behaviour was. If you feel like something doesnt type correctly then take a look at the tests first and change or create a new one
Ready for testing and refinement by community - but expect rough edges!
Current version includes several opinionated decisions with a goal of writing a schema once.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.