required ignored for properties referencing definitions with allOf

[ackl]

Hi,

I've noticed what seems like a bug with the generated types when the required keyword is used in a property which references a definition using allOf

Here's an example schema:

{
  "title": "Example Schema",
  "type": "object",
  "properties": {
    "reference": {
      "allOf": [{ "$ref": "#/definitions/defined" }],
      "required": ["subproperty"]
    }
  },
  "definitions": {
    "defined": {
      "type": "object",
      "properties": {
        "subproperty": {
          "type": "integer"
        }
      }
    }
  },

  "additionalProperties": false
}

and the generated typings

export interface ExampleSchema {
  reference?: Defined;
}
export interface Defined {
  subproperty?: number;
  [k: string]: unknown;
}

I understand that the interface Defined is probably just pulled straight from the definitions section which is why the subproperty key is optional, but I would expect that the generated types would know that the Defined interface being used by ExampleSchema has subproperty as mandatory.

If you explore this in a json schema validator you will see that given the example schema above, this object:

{
   "reference": {
      "foo": "bar"
    }
}

is invalid since Required properties are missing from object: subproperty.

but with the current json-schema-to-typescript implementation,

const thing:ExampleSchema = {
   "reference": {
      "foo": "bar"
    }
}

is valid

[ackl]

Could fix with something like

rules.set('Set required on allOf', (schema) => {
  if (schema.allOf && schema.required) {
    schema.allOf.forEach(subSchema => {
      Object.assign(subSchema, { required: schema.required })
    });
  }
})

in src/normaliser.ts

[rkrauskopf]

@ackl Were you able to find a work around for this? Running into the same issue.

[amh4r]

This should have the behavior you're looking for:

{
  "title": "Example Schema",
  "type": "object",
  "properties": {
    "reference": {
      "allOf": [
        { "$ref": "#/definitions/defined" },
        {
          "type": "object",
            "properties": {
              "subproperty": {
                "type": "integer"
              }
            },
            "required": [
              "subproperty"
            ]
          }
      ],
    }
  },
  "definitions": {
    "defined": {
      "type": "object",
      "properties": {
        "subproperty": {
          "type": "integer"
        }
      }
    }
  },

  "additionalProperties": false
}

Or if you don't want to use allOf, since there's only 1 item in it:

{
  "title": "Example Schema",
  "type": "object",
  "properties": {
    "reference": {
      "$ref": "#/definitions/Defined",
      "required": [
        "subproperty"
      ]
    }
  },
  "definitions": {
    "defined": {
      "type": "object",
      "properties": {
        "subproperty": {
          "type": "integer"
        }
      }
    }
  },

  "additionalProperties": false
}

[chbdetta]

Actually, according to https://github.com/json-schema-org/json-schema-spec/issues/672, $ref with siblings in the schema behaves exactly like allOf:

"reference": {
      "$ref": "#/definitions/Defined",
      "required": [
        "subproperty"
      ]
}

is equivalent to:

"reference": {
      "allOf": [
           {"$ref": "#/definitions/Defined"},
           {"required": [
                "subproperty"
             ]
           }
      ]
}

And most of the schema keywords can independently validate values, including "required". So a schema like:

{
    "required": ["something"]
}

is perfectly valid.

So I think, @ackl is correct that this is a bug.

[amh4r]

@chbdetta

It is important to note that the schemas listed in an allOf, anyOf or oneOf array know nothing of one another.

(Source)

So when you put {"required": ["subproperty"]} in an allOf item, JSON schema hasn't been told that subproperty exists. In the same allOf item, you need to specify subproperty as a property.

[chbdetta]

@goodoldneon The thing is required doesn't need to specify properties that only exist in properties. That's why it's independent of properties.

See spec: https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.6.5.3 It mentions NOTHING about the strings inside required must be defined in properties.

Therefore,

{
    "required": ["something"]
}

This is a valid schema by itself, it just validates that an object must have a property named something, somewhat similar to something like

interface A {
  something: unknown
}

except that typescript allows undefined to be assigned to unknown.

[amh4r]

Oh interesting, so each of the "validation keywords" has an implicit type? In other words, this:

{
  "required": ["foo"]
}

Is the same as this:

{
  "type": "object",
  "required": ["foo"]
}

In an allOf?

[chbdetta]

@goodoldneon nope. type also validates independently. see https://json-schema.org/draft/2020-12/json-schema-core.html#rfc.section.7.6.1

[amh4r]

Thank you for being patient with me, @chbdetta. We're getting into some advanced JSON Schema stuff I haven't dealt with.

Given the schema that @ackl originally posted, I think that the following types would work:

export interface ExampleSchema {
  reference?: Defined & {
    subproperty: any;
  };
}
export interface Defined {
  subproperty?: number;
  [k: string]: unknown;
}

I was able to get that working by modifying this switch case:

View git diff

diff --git a/src/parser.ts b/src/parser.ts
index 6cb6cef..0abb5b3 100644
--- a/src/parser.ts
+++ b/src/parser.ts
@@ -124,11 +124,44 @@ function parseNonLiteral(
 
   switch (type) {
     case 'ALL_OF':
+      const astParams = schema.allOf!.map(_ => parse(_, options, undefined, processed, usedNames))
+
+      // Required properties have been specified alongside `allOf`.
+      if (Array.isArray(schema.required)) {
+        // This block will add an anonymous interface whose keys are the
+        // required properties. Every type is `any`, since this interface doesn't
+        // care what the type is, but rather is only concerned that the
+        // properties exist.
+
+        const requiredParams = schema.required.map(
+          (propertyName: string): TInterfaceParam => {
+            return {
+              ast: {
+                keyName: propertyName,
+                type: 'ANY'
+              },
+              isPatternProperty: false,
+              isRequired: true,
+              isUnreachableDefinition: false,
+              keyName: propertyName
+            }
+          }
+        )
+
+        astParams.push({
+          comment: undefined,
+          keyName: undefined,
+          params: requiredParams,
+          superTypes: [],
+          type: 'INTERFACE'
+        })
+      }
+
       return {
         comment: schema.description,
         keyName,
         standaloneName: standaloneName(schema, keyNameFromDefinition, usedNames),
-        params: schema.allOf!.map(_ => parse(_, options, undefined, processed, usedNames)),
+        params: astParams,
         type: 'INTERSECTION'
       }
     case 'ANY':

If @bcherny is OK with my approach, I can work on a PR.

[bcherny]

Sorry for the delay!

I don't think a normalizer rule would be sufficient here. We also need to consider:

• anyOf and oneOf
• Nested sub-schemas (N layers deep)
• The same definition being referenced both by a property that requires some of its properties, and a property that does not

My hunch is the right solution is emitting some sort of intersection type, similar to what @goodoldneon suggested. I'd love to see this fleshed out in a PR, preferably without the any :).