Required Fields are not returning Error Messages about schema-to-yup

[vsevolodbvd]

@kristianmandrup @ritchieanesco Hello guys! I have the same issue.
I made some debugging and it seems that the problem is related to src/types/mixed/mixed.js.
Here we have addMappedConstraints function:

addMappedConstraints() {
    const $map = this.constraintsMap;
    const keys = Object.keys($map);
    keys.map(key => {
      const list = $map[key];
      const fnName = key === "value" ? "addValueConstraint" : "addConstraint";
      list.map(this[fnName]);  <---- problem here
    });
    return this;
  }

So for the required constraint the addConstraint function is executed.

addConstraint(propName, opts) {
    const contraint = this.buildConstraint(propName, opts);
    this.base = contraint || this.base;
    return this;
  }

As we can see, this function receives two parameters propName ('required' in our case) and opts (0 in our case, because this function is executed as a callback of the list.map).

Next both these parameters are passed to the buildConstraint function:

buildConstraint(propName, opts = {}) { <--- here propName = 'required', opts = 0
    let {
      constraintName,
      constraintValue,
      propValue,
      method,
      yup,
      value,
      values,
      errName
    } = opts; <---- all the values in this object will be undefined
    yup = yup || this.base; <---- yup = this.base
    constraintValue =
      constraintValue || propValue || this.constraints[propName]; <---- constraintValue = true
    if (this.isNothing(constraintValue)) {
      this.warn("no prop value");
      return yup;
    }
    constraintName = constraintName || propName; <---- constraintName = 'required'
    method = method || constraintName; <---- method = 'required'
    const yupConstraintMethodName = this.aliasMap[method] || method; <---- yupConstraintMethodName = required
    if (!yup[yupConstraintMethodName]) {
      this.warn(`Yup has no such API method: ${yupConstraintMethodName}`);
      return this;
    }
    const constraintFn = yup[yupConstraintMethodName].bind(yup);
    const errFn =
      this.valErrMessage(constraintName) ||
      (errName && this.valErrMessage(errName)); <---- errFn = 'User is required'
    !!! THIS BLOCK WONT BE EXECUTED IN OUR CASE
    if (this.isPresent(values)) {
      if (!Array.isArray(values)) {
        this.warn(
          "buildConstraint: values option must be an array of arguments"
        );
        return yup;
      }
      this.onConstraintAdded({ name: constraintName, value: values });
      const newBase = constraintFn(values, errFn);
      return newBase;
    }
   !!! THIS BLOCK WILL BE EXECUTED IN OUR CASE
    if (this.isPresent(constraintValue)) {
      this.onConstraintAdded({ name: constraintName, value: constraintValue });
      const newBase = this.isPresent(constraintValue)
        ? constraintFn(constraintValue, errFn) <----- PROBLEM HERE
        : constraintFn(errFn);
      console.log('before add -> ', constraintValue, errFn);
      console.log('on add -> ', constraintFn(errFn, constraintValue).tests)
      console.log('newBase -> ', newBase.tests)
      return newBase;
    }

    if (!this.isPresent(constraintValue)) {
      this.onConstraintAdded({ name: constraintName });
      const newBase = constraintFn(errFn);
      return newBase;
    }
    this.warn("buildConstraint: missing value or values options");
    return yup;
  }

According to the fact that constraintValue in our case is true this.isPresent(constraintValue) will return true and constraintFn(constraintValue, errFn) will be executed to determine newBase.

Here constraintFn function is equal to yup.string().required() function that receives only one optional parameter message?: string | function (https://github.com/jquense/yup).

The problem is that we pass constraintValue (true in our case) as a first parameter to yup.string().require() that will set true as a custom message.

So, newBase.tests array will look like this:

newBase.tests ->  [ { [Function: validate]
          OPTIONS:
           { message: true, name: 'required', test: [Function: notEmpty] } },
        { [Function: validate]
          OPTIONS:
           { message: true, name: 'required', test: [Function: hasLength] } } ]

@kristianmandrup I hope this info will be helpful and allow you to quickly resolve this issue.

[kristianmandrup]

Yeah, this whole part needs some love... in my latest branch I was extracting this part as a builder, cleaning up, refactoring and reworking it to be easier to manage and work with. I hope to get it sorted the coming weekend. Been too busy at work lately.

If you want to help out, have a look at constraint-builder branch

[kristianmandrup]

There are also a bunch of other branches with various refactoring, more tests etc

[vsevolodbvd]

@kristianmandrup Cool! Thank you for your response. Will check other branches too. Can't wait for the next release

[kristianmandrup]

Cool :) I plan to work on it Friday and Saturday and make a 2.0 release on Sunday if all goes well. Regarding the required error, to fix it see how rules for email and url does it, I think

[kristianmandrup]

Hi @vsevolodbvd , I've spent all morning trying to fix the required issue as per this branch

Was behaving very weirdly, creating required tests (yup notEmpty and hasLength) no matter what.
When I test the behaviour in isolation (in required.test.js) for the string type (string schema tests) it works just fine and doesn't have this behaviour. I will try your suggestion now, avoiding message: true
Cheers.

[kristianmandrup]

I created the following test:

const userSchema = {
  title: "users",
  type: "object",
  properties: {
    username: { type: "string", required: true, email: true },
    // last_name: { type: "string", required: false }
    last_name: { type: "string" },
    // nothing: {}, FAILS - type required
    level: { type: "number" },
    age: { type: "number", notRequired: true }
  }
};

test.only("yup validates valid json to return true - error messages", done => {
  try {
    const yupSchema = buildYup(userSchema, {
      err: msg => console.error(`ERROR: ${msg}`)
    });
    const { fields } = yupSchema;

    console.log({ fields });
    const { username, last_name, age, level, nothing } = fields;
    console.log("username", username.tests);
    console.log("last_name", last_name && last_name.tests);
    console.log("age", age.tests);
    console.log("level", level.tests);
    console.log("nothing", nothing && nothing.tests);

    yupSchema
      .validate({
        username: "mike"
      })
      .then(valid => {
        console.log({ valid });
        expect(valid).toBeTruthy();
        done();
      })
      .catch(err => {
        const { errors, name } = err;
        console.log("ERRORS", { err, errors, name });
        expect(errors[0]).toBeFalsy();
        done();
      });
  } catch (e) {
    console.error(e);
    done();
  }
});

As soon as you set type: "string" it will create a yup.string() property schema, which will have default rules to test that the property value is a valid string, ie. that it is not empty and has a length.
You explicitly have to add the constraint notRequired for it to be ignored.

this.addConstraint(name, { method: 'notRequired, value: true })

  convert() {
    this.cleanConstraints();
    // base constraints
  }

  cleanConstraints() {
    // yeah, I know - but ugly or should be set in constructor as instance var
    const propName = this.value.key;

    this.addConstraint(propName, {
      constraintName: "notRequired",
      value: true
    });
  }

[kristianmandrup]

Check this: #33 (comment)

[kristianmandrup]

Finally solved it in latest release 1.9.10 - also added runtime mode configuration