oneOf schemas do not validate as expected

[brenden-ef]

Not 100% sure if this is related to #109 or not.

We have a use case where we need to use oneOf to conditionally validate one field based on the value of another.

Given the following schema

{
  "title": "Schema title",
  "description": "Schema description",
  "type": "object",
  "properties": {
    "firstName": {
      "type": "string",
    },
    "lastName": {
      "type": "string",
    },
    "requireAddress": {
      "type": "string",
      "enum": [
        "yes",
        "no"
      ]
    },
    "addressLine1": {
      "type": "string",
    },
    "addressLine2": {
      "type": "string",
    }
  },
  "required": [
    "firstName",
    "lastName",
    "requireAddress"
  ],
  "oneOf": [
    {
      "properties": {
        "requireAddress": {
          "const": "yes"
        },
        "addressLine1": {
          "minLength": 2
        },
        "addressLine2": {
          "minLength": 2
        }
      },
      "required": [
        "addressLine1",
        "addressLine2"
      ]
    },
    {
      "properties": {
        "requireAddress": {
          "const": "no"
        }
      }
    }
  ]
}

I would expect this object to fail validation because requireAddress is yes, but addressLine1 is not present:

{
  "firstName": "han",
  "lastName": "yolo",
  "requireAddress": "yes",
  "addressLine2": "houseNumber"
}

However, the object is marked as valid by the schema returned from buildYup.
You can see the validation behaving the way we would expect on https://www.jsonschemavalidator.net if you enter the above objects.

Reproduction: https://github.com/brenden-ef/schema-to-yup-one-of-repro

[kristianmandrup]

Thanks. I will add it as a test case and try to resolve it shortly.

[kristianmandrup]

Ah yes, the const issue again. This is indeed related to #109

It is currently implemented this way:

• If const is set to nothing (null or undefined) it ignores it and returns.
• If const is a data reference, ie. there is a $data property, then the value is set to yup.ref(dataRef), which is the normalized data ref path.
• Finally a yup constraint is added for const with the value (dataref or "as-is").

  const() {
    let value = this.constraints.const;
    if (this.isNothing(value)) return this;
    // TODO: resolve const data ref if valid format
    if (this.isDataRef(value)) {
      const dataRefPath = this.normalizeDataRefPath(value);
      value = yup.ref(dataRefPath);
    }
    return this.addConstraint("const", { value });
  }

Looking closer at the yup schema documentation, I can see there is no yup constraint const.
The closest alternative is oneOf that is passed a list containing a single value to test on.
So I think the following fix should solve it.

  const() {
    let value = this.constraints.const;
    if (this.isNothing(value)) return this;
    // TODO: resolve const data ref if valid format
    if (this.isDataRef(value)) {
      const dataRefPath = this.normalizeDataRefPath(value);
      value = yup.ref(dataRefPath);
    }
    return this.addConstraint("oneOf", { values: [value] });
  }

Try it in the new const-oneOf branch.

[kristianmandrup]

It passed the test

const schema = {
 // schema from the repo https://github.com/brenden-ef/schema-to-yup-one-of-repro
}

const object = {
  firstName: "han",
  lastName: "yolo",
  requireAddress: "yes",
  addressLine2: "houseNumber",
};


test("oneOf const", async () => {
  const yupSchema = buildYup(schema);
  // console.dir(schema)
  try {
    const valid = await yupSchema.isValid(object);
    expect(valid).toBe(false);
  } catch (e) {
    console.log(e);
  }
});

Test results

 PASS  test/one-of-const.test.js
  ✓ oneOf const (23 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total

[kristianmandrup]

Please confirm, then I will merge to master and close the issue ;)

[brenden-ef]

I think the try/catch in your test might be swallowing the failure? When I run it locally I see this.

Ran all test suites matching /one-of-const.test.js/i.
❯ npx jest one-of-const.test.js
  console.log
    JestAssertionError: expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true
        at Object.toBe (/Users/brenden/code/schema-to-yup/test/one-of-const.test.js:63:19)
        at processTicksAndRejections (node:internal/process/task_queues:96:5) {
      matcherResult: {
        actual: true,
        expected: false,
        message: '\x1B[2mexpect(\x1B[22m\x1B[31mreceived\x1B[39m\x1B[2m).\x1B[22mtoBe\x1B[2m(\x1B[22m\x1B[32mexpected\x1B[39m\x1B[2m) // Object.is equality\x1B[22m\n' +
          '\n' +
          'Expected: \x1B[32mfalse\x1B[39m\n' +
          'Received: \x1B[31mtrue\x1B[39m',
        name: 'toBe',
        pass: false
      }
    }

      at Object.log (test/one-of-const.test.js:65:13)

And when I comment out the try/catch I see the test failure.

❯ npx jest one-of-const.test.js
 FAIL  ./one-of-const.test.js
  ✕ oneOf const (5 ms)

  ● oneOf const

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      61 |   // try {
      62 |     const valid = await yupSchema.isValid(object);
    > 63 |     expect(valid).toBe(false);
         |                   ^
      64 |   // } catch (e) {
      65 |     // console.log(e);
      66 |   // }

      at Object.toBe (test/one-of-const.test.js:63:19)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total

[kristianmandrup]

Could you write the equivalent Yup schema that would work for your minimal case? then we can work to target that. Cheers.

[brenden-ef]

Sure thing. My schedule is a bit wild today, but will work to get it to you ASAP.

[brenden-ef]

The only way I've been able to get it to work without using js to conditionally compose multiple schemas is by using when:

object({
  firstName: string().required(),
  lastName: string().required(),
  requireAddress: string().required().oneOf(["yes", "no"]),
  addressLine1: string().when("requireAddress", {
    is: "yes",
    then: (schema) => schema.required()
  })
});

Normally I'd try to go the route using with, but we don't have control over the schemas we're converting, and other systems that don't use yup have to validate against same schemas. According to json schema spec though oneOf can be used to handle the same kind of conditional logic.

[kristianmandrup]

I see. Currently it doesn't understand specific const values such as yes or no to indicate on/off switching of logic.
Should not be too hard to customize it to implement such logic using the customization options included (see Readme docs).

[kristianmandrup]

It turns out this is due to a limitation of oneOf in yup

• https://stackoverflow.com/questions/72925269/how-to-validate-using-oneof-with-yup
• .oneOf() uses referential equality checks for objects jquense/yup#1393
• .oneOf() uses referential equality checks for objects jquense/yup#1393 (comment)

Yup doesn't treat oneOf in any way like a conditional. The library currently attempts a 1-1 mapping to the extent possible.

I'm not sure what the best approach for your case would be. Perhaps override the built-on oneOf so that if passed one or more objects, see for which property they all overlap and if so use that for the when-is condition and in the then return whichever object passes the is test.

https://stackoverflow.com/questions/34392741/best-way-to-get-intersection-of-keys-of-two-objects

function intersectKeys(first, ...rest) {
    const restKeys = rest.map(o => Object.keys(o));
    return Object.fromEntries(Object.entries(first).filter(entry => restKeys.every(rk => rk.includes(entry[0]))));
}

or

function intersectingKeys(...objects) {
  return objects
    .map((object) => Object.keys(object))
    .sort((a, b) => a.length - b.length)
    .reduce((a, b) => a.filter((key) => b.includes(key)));
}

[kristianmandrup]

Likely the only option is to leverage https://github.com/jquense/yup#schematestname-string-message-string--function--any-test-function-schema

[kristianmandrup]

I think something like this should do the trick

  oneOfConditional() {
    let { config, parentNode, isObject, value, key } = this;
    // optionally set custom errMsg
    const oneOfConstraints = value;
    this.base = this.base.test(
      key,
      // errMsg,
      (value, context) => {
        for (let constraint in oneOfConstraints) {
          if (!isObject(constraint)) {
            return value === constraint;
          }
          const yupSchema = config.buildYup(constraint, config, parentNode);
          const result = yupSchema.validate();
          if (result) return true;
        }
        return false;
      }
    );
  }

  notOneOfConditional() {
    let { config, parentNode, isObject, value, key } = this;
    // optionally set custom errMsg
    const oneOfConstraints = value;
    this.base = this.base.test(
      key,
      // errMsg,
      (value, context) => {
        for (let constraint in oneOfConstraints) {
          if (!isObject(constraint)) {
            return value !== constraint;
          }
          const yupSchema = config.buildYup(constraint, config, parentNode);
          const result = yupSchema.validate();
          if (result) return false;
        }
        return true;
      }
    );
  }

[kristianmandrup]

There should now be better support for overriding mixed constraint handler methods such as oneOf. Also added advanced customization example that handle your case.

https://github.com/kristianmandrup/schema-to-yup#Advancedcustomconstraintexample

    const typeConfig = this.config[this.type] || {};
    const mixedConfig = this.config.mixed || {};

    this.mixedConfig = mixedConfig;
    this.typeConfig = {
      ...mixedConfig,
      ...typeConfig,
    };


  configureTypeConfig() {
    if (this.typeConfig.enabled || this.typeConfig.extends) return;
    if (!this.typeConfig.convert) return;
    this.typeConfig.extends = Object.keys(this.typeConfig.convert);
  }

  get configuredTypeEnabled() {
    return Array.isArray(this.typeConfig.enabled)
      ? this.typeConfig.enabled
      : this.typeEnabled;
  }

  get $typeEnabled() {
    return this.$typeExtends || this.configuredTypeEnabled;
  }

  get enabled() {
    return [...this.mixedEnabled, ...this.$typeEnabled];
  }

  convertEnabled() {
    this.enabled.map((name) => {
      const convertFn = this.convertFnFor(name);
      if (convertFn) {
        convertFn(this);
      }
    });
  }

  convertFnFor(name) {
    return (
      this.customConvertFnFor(name, this) || this.builtInConvertFnFor(name)
    );
  }

  customConvertFnFor(name) {
    const typeConvertMap = this.typeConfig.convert || {};
    return typeConvertMap[name];
  }

Might be a little hard to follow, but essentially if the custom constraint function is set in typeConfig and all part of enabled array, then it will be triggered here and passed this as the single argument (the type handler) and override the built in type handler method if one exists for the same name.

  convertEnabled() {
    this.enabled.map((name) => {
      const convertFn = this.convertFnFor(name);
      if (convertFn) {
        convertFn(this);
      }
    });
  }

[brenden-ef]

Ah great! I will try this out later on today and let you know how it goes.

[caioedut]

@kristianmandrup any news? the "oneOf" below dont work:

{
    type: 'object',
    properties: { ... },
    oneOf: [
      {
        required: ['questionId'],
      },
      {
        required: ['categoryId', 'questionsCount'],
      },
    ]
}

[kristianmandrup]

Sorry, this is currently a known limitation. Not sure how to translate this to a Yup schema. Feel free to have a go at it and if you find a good approach I can integrate it shortly. Sorry for the late response. Not really maintaining this repo much.
It was meant as a starting point for the community to continue. In dire need of re-engineering from the ground up!