StefanTerdell / zod-to-json-schema

Converts Zod schemas to Json schemas
ISC License
854 stars 67 forks source link

Schema refs are assigned regardless of `.describe()` calls #123

Open kachkaev opened 2 months ago

kachkaev commented 2 months ago

I believe I have found a small bug similar to https://github.com/StefanTerdell/zod-to-json-schema/issues/66. Using zod-to-json-schema@3.23.0. Here is a reproduction:

const sharedFieldSchema = /* ... see below ... */;
const schema = z.object({
  foo: sharedFieldSchema.describe('This is foo'),
  bar: sharedFieldSchema.describe('This is bar'),
  baz: sharedFieldSchema,
});
console.log(zodToJsonSchema(schema));

z.string()

No description in baz, as expected.

{
  "type": "object",
  "properties": {
    "foo": {
      "type": "string",
      "description": "This is foo"
    },
    "bar": {
      "type": "string",
      "description": "This is bar"
    },
    "baz": {
      "type": "string"
    }
  },
  "required": [
    "foo",
    "bar",
    "baz"
  ],
  "additionalProperties": false,
  "$schema": "http://json-schema.org/draft-07/schema#"
}

z.string().min(1).max(255)

No description in baz, as expected.

{
  "type": "object",
  "properties": {
    "foo": {
      "type": "string",
      "minLength": 1,
      "maxLength": 255,
      "description": "This is foo"
    },
    "bar": {
      "type": "string",
      "minLength": 1,
      "maxLength": 255,
      "description": "This is bar"
    },
    "baz": {
      "type": "string",
      "minLength": 1,
      "maxLength": 255
    }
  },
  "required": [
    "foo",
    "bar",
    "baz"
  ],
  "additionalProperties": false,
  "$schema": "http://json-schema.org/draft-07/schema#"
}

z.string().default('default value')

Both bar and baz refer to foo, so baz unexpectedly inherits foo’s description.

{
  "type": "object",
  "properties": {
    "foo": {
      "type": "string",
      "default": "default value",
      "description": "This is foo"
    },
    "bar": {
      "$ref": "#/properties/foo",
      "default": "default value",
      "description": "This is bar"
    },
    "baz": {
      "$ref": "#/properties/foo",
      "default": "default value"
    }
  },
  "additionalProperties": false,
  "$schema": "http://json-schema.org/draft-07/schema#"
}

z.string().catch('caught value')

Same as above: Both bar and baz refer to foo, so baz unexpectedly inherits foo’s description.

{
  "type": "object",
  "properties": {
    "foo": {
      "type": "string",
      "description": "This is foo"
    },
    "bar": {
      "$ref": "#/properties/foo",
      "description": "This is bar"
    },
    "baz": {
      "$ref": "#/properties/foo"
    }
  },
  "additionalProperties": false,
  "$schema": "http://json-schema.org/draft-07/schema#"
}

If I call zodToJsonSchema(schema, { $refStrategy: 'none' }), description no longer ‘leaks’, which is great. However, it’d be great to see it not leaking by default because misplaced tooltips with description are confusing.

PS: Thanks for the lib! It’s really neat an super-useful! 🙌

StefanTerdell commented 2 months ago

unfortunately this has the same root issue: calling describe on your zod schemas does not create new instances, so there's nothing to check against. I could create an internal map of all seen descriptions, but with zod 4 around the corner it might not be worth the effort. Let's see how it turns out :)

kachkaev commented 2 months ago

I briefly checked the implementation of describe, it seems to be creating a new instance: https://github.com/colinhacks/zod/blame/03f2f080c4bb0ccd4434748a6e930ebdd1bd75a6/src/types.ts#L499-L505

The implementation of describe has not been changed since Oct 2021, according to git blame.

StefanTerdell commented 2 months ago

Sorry, yes, you're right. I mixed this up with another bug and didn't read the issue properly.