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

oneOf and additionalPropertes:false produces parser that can never validate #87

Closed
ianwremmel opened this issue Mar 18, 2024 · 3 comments

Comments

@ianwremmel
Copy link

I have a set of schemas spread across three files

client-message.schema.json:

{
  "$schema": "http://json-schema.org/schema",
  "additionalProperties": false,
  "oneOf": [
    {
      "$ref": "./ping-message.schema.json"
    }
  ],
  "properties": {},
  "title": "ClientMessage",
  "type": "object"
}

ping-message.schema.json:

{
  "$schema": "http://json-schema.org/schema",
  "additionalProperties": false,
  "allOf": [
    {
      "$ref": "./request-message.schema.json"
    },
    {
      "additionalProperties": false,
      "properties": {
        "method": {
          "enum": [
            "GET"
          ],
          "type": "string"
        },
        "path": {
          "enum": [
            "/ping"
          ],
          "type": "string"
        }
      },
      "required": [
        "method",
        "path"
      ]
    }
  ],
  "properties": {},
  "title": "PingMessage",
  "type": "object"
}

request-message.schema.json:

{
  "$schema": "http://json-schema.org/schema",
  "additionalProperties": false,
  "properties": {
    "method": {
      "type": "string"
    },
    "path": {
      "type": "string"
    }
  },
  "required": [
    "method",
    "path"
  ],
  "title": "RequestMessage",
  "type": "object"
}

it's a little weird right now since the oneOf only has one entry, but I expect to add a bunch over time.

The issue is that this combination of oneOf, "properties": {}, and "additionalProperties": false", produces the following parser:

export const ClientMessageSchema = z
  .object({})
  .strict()
  .and(
    z
      .object({})
      .strict()
      .and(
        z.intersection(
          z.object({method: z.string(), path: z.string()}).strict(),
          z
            .object({method: z.literal('GET'), path: z.literal('/ping')})
            .strict()
        )
      )
  );
export const PingMessageSchema = z
  .object({})
  .strict()
  .and(
    z.intersection(
      z.object({method: z.string(), path: z.string()}).strict(),
      z.object({method: z.literal('GET'), path: z.literal('/ping')}).strict()
    )
  );
export const RequestMessageSchema = z
  .object({method: z.string(), path: z.string()})
  .strict();

I'm not entirely sure what the correct transform is. At a minimum, I think the output could be simplified by recognizing an empty "properties" and removing e.g. z.object({}).strict().and(...) in favor of .... I think that still leaves a bug, though, when there are "properties" and "additionalProperties" is false. Maybe instead of z.object(A).strict().and(B) it should be

z.object(A).strict().merge(z.object(B).strict())

In any case, I'm pretty sure this is a blocker for upgrading to 2.x unless I'm missing something.

@ianwremmel ianwremmel changed the title Strict union produce parsers that can never validate Strict unions produce parsers that can never validate Mar 18, 2024
@ianwremmel ianwremmel changed the title Strict unions produce parsers that can never validate oneOf and additionalPropertes:false produces parser that can never validate Mar 18, 2024
@StefanTerdell
Copy link
Owner

StefanTerdell commented Mar 22, 2024

Thanks for opening an issue!

The output looks about right to me to be honest. You've defined a schema describing an empty object with no additional properties, and then effectively added required properties to that. I'd consider trying to simplify the input schemas.

I might have messed up the resolution, but FWIW I can't get the original schema to validate either:

https://www.jsonschemavalidator.net/s/JKvey7nC

Feel free to reopen the issue if you disagree.

@ianwremmel
Copy link
Author

Hmm, I'll have to look closer at the validator tomorrow. Your interpretation sounds reasonable, but that leads me to the question, "any idea how to do what I'm trying to do"? :)

Let's assume that, in addition to PingMessage, I've also got MessageType1 and MessageType2. My goal is to produce something that validates that a given input matches exactly one of PingMessage, MessageType1, or MessageType2, but not more than one.

@StefanTerdell
Copy link
Owner

Id probably flatten it to something like

{
  oneOf: [
    {
      type: object,
      properties: {
        method: {
          const: "GET"
        }
        path: {
          const: "/path"
        }
      }
    },
    {
      type: object,
      properties: {
        method: {
          const: "POST"
        }
        path: {
          const: "/path"
        }
      }
    },
    ... etc
  ]
}

Essentially I just skipped the schema factoring parts. Since you already have all the object schema properties like type: object etc in the leaf schemas this shouldn't cause an issue.

Factoring schemas is only partly supported with json-schema-to-zod (and lots of other tooling), so although it's part of the official json schema spec I would advice against it as far as possible.

Hope this helps! :)

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

No branches or pull requests

2 participants