Skip to content

Fix bug with zod ordering and default/optional methods - #324

Merged
david-crespo merged 10 commits into
mainfrom
zod-order-bugfix
Jan 9, 2026
Merged

Fix bug with zod ordering and default/optional methods#324
david-crespo merged 10 commits into
mainfrom
zod-order-bugfix

Conversation

@charliepark

Copy link
Copy Markdown
Contributor

There were two issues I discovered when running this generator against https://github.com/oxidecomputer/console. This PR addresses them and adds a few tests.

First, a commit I made yesterday introduced an ordering bug, where zod schemas had a default() dropped in before the constraints (min, max), as in here:
-"mvlan": z.number().default(null).min(0).max(65535).nullable().optional(),
This is now fixed so that the default gets added after the constraints:
+"mvlan": z.number().min(0).max(65535).nullable().default(null),

Second, the generator shouldn't generate both default and optional on the same record.

-"multicastIp": z.ipv4().nullable().default(null).optional(),
+"multicastIp": z.ipv4().nullable().default(null),

@david-crespo

Copy link
Copy Markdown
Collaborator

I think you can do without skipDefault — the snapshots still pass after this change:

diff --git a/oxide-openapi-gen-ts/src/schema/zod.ts b/oxide-openapi-gen-ts/src/schema/zod.ts
index f240fc8e50..7fe7a88555 100644
--- a/oxide-openapi-gen-ts/src/schema/zod.ts
+++ b/oxide-openapi-gen-ts/src/schema/zod.ts
@@ -85,11 +85,9 @@
   },
 
   integer(schema, io) {
-    schemaToZodInt(schema, io, { skipDefault: schema.nullable });
-    if (schema.nullable) {
-      io.w0(".nullable()");
-      io.w0(getDefaultString(schema));
-    }
+    schemaToZodInt(schema, io);
+    if (schema.nullable) io.w0(".nullable()");
+    io.w0(getDefaultString(schema));
   },
 
   array(schema, io) {
@@ -201,11 +199,7 @@
   },
 });
 
-function schemaToZodInt(
-  schema: OpenAPIV3.SchemaObject,
-  { w0 }: IO,
-  options?: { skipDefault?: boolean }
-) {
+function schemaToZodInt(schema: OpenAPIV3.SchemaObject, { w0 }: IO) {
   if ("enum" in schema) {
     /**  See comment in {@link setupZod} */
     w0(`IntEnum(${JSON.stringify(schema.enum)} as const)`);
@@ -230,8 +224,4 @@
     // It's signed so remove the most significant bit
     w0(`.max(${Math.pow(2, parseInt(size) - 1) - 1})`);
   }
-
-  if (!options?.skipDefault) {
-    w0(getDefaultString(schema));
-  }
 }

Comment thread oxide-openapi-gen-ts/src/schema/zod.ts
// Only emit .default(null) if the schema is nullable to avoid runtime errors
if (defaultValue === null && !schema.nullable) {
return "";
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This doesn't happen in our spec and the schema generator probably wouldn't let it happen, but I suppose it's fine to be defensive. Plus you can manually build a schema in the API if you really want to.

@david-crespo
david-crespo merged commit 4c951d9 into main Jan 9, 2026
3 checks passed
@david-crespo
david-crespo deleted the zod-order-bugfix branch January 9, 2026 20:41
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