mokkabonna / json-schema-merge-allof

Simplify your schema by combining allOf
97 stars 23 forks source link

Merge incompatible types #39

Closed danthegoodman1 closed 1 year ago

danthegoodman1 commented 1 year ago

If there are schema with with types string and number being merged, is there an option to have the merged schema come out with type: ['string', 'number']?

mokkabonna commented 1 year ago

No and probably will not be. As that would change the schemas fundamentally and change the original meaning of the schema.

However you can do this yourself if you want that custom behaviour.

  const result = merge(
    {
      type: "string",
      allOf: [
        {
          type: "number",
        },
      ],
    },
    {
      resolvers: {
        type(values, path, mergeSchemas, options) {
          return values;
        },
      },
    }
  );

  console.log(JSON.stringify(result, null, 2));
{
  "type": [
    "string",
    "number"
  ]
}

You probably also want to remove any duplicates on the values.

danthegoodman1 commented 1 year ago

Yep, that’s what I just finished doing. Thanks! However that is still valid json schema, so I think it’s something to make more visible for folks who want a property to be one of two or more types

On Mon, Feb 27, 2023 at 4:50 AM Martin Hansen @.***> wrote:

No and probably will not be. As that would change the schemas fundamentally and change the original meaning of the schema.

However you can do this yourself if you wnat that custom behaviour.

const result = merge( { type: "string", allOf: [ { type: "number", }, ], }, { resolvers: { type(values, path, mergeSchemas, options) { return values; }, }, } );

console.log(JSON.stringify(result, null, 2));```json{ "type": [ "string", "number" ]} You probably also want to remove any duplicates on the values.

— Reply to this email directly, view it on GitHub https://github.com/mokkabonna/json-schema-merge-allof/issues/39#issuecomment-1446021934, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEH3JLDUDCOO5F6KJZBLHTLWZR2HNANCNFSM6AAAAAAVIW2LGM . You are receiving this because you authored the thread.Message ID: @.***>

mokkabonna commented 1 year ago

Yes multiple types are allowed and supported.

It is just that this schema will always fail validation. One schema says it must be string, another that it must be number.

{
      type: "string",
      allOf: [
        {
          type: "number",
        },
      ],
}

This library does not combine schemas and allow all variants. It preserves the validation of how it was before.

Either both of the schemas should have "type": ["string", "number"] or just one of them. That way it would work as expected.

danthegoodman1 commented 1 year ago

Copy, thanks for clarifying

On Mon, Feb 27, 2023 at 8:55 AM Martin Hansen @.***> wrote:

Yes multiple types are allowed and supported.

It is just that this schema will always fail validation. One schema says it must be string, another that it must be number.

{ type: "string", allOf: [ { type: "number", }, ],}

This library does not combine schemas and allow all variants. It preserves the validation of how it was before.

Either both of the schemas should have "type": ["string", "number"] or just one of them. That way it would work as expected.

— Reply to this email directly, view it on GitHub https://github.com/mokkabonna/json-schema-merge-allof/issues/39#issuecomment-1446363844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEH3JLCAKVS2QUQCANSFHB3WZSW5TANCNFSM6AAAAAAVIW2LGM . You are receiving this because you authored the thread.Message ID: @.***>

mokkabonna commented 1 year ago

But of course if you have control and know the "risks" then this is a perfect use case for the custom resolver.

The only other variant of this that is officially supported is to ignore the additionalProperties: false during the merge phase. As two schemas with one of them having that set will basically never allow anything outside the schema it is in. And that is not very useful when composing multiple schemas.