Missing support for 'integer'

[CatchABus]

Builder seems to handle 'integer' type as 'number' and does not even apply integer condition to yup schema. According to JSON Schema: There are two numeric types in JSON Schema: integer and number. They share the same validation keywords.

[kristianmandrup]

Looked into it a little bit. There should be integer support as indicated here and it should be enabled by default.

Currently it normalizes the type int to integer as in normalizeNumType.

The default functionality to test for integer type in a JSON schema is as follows config.isInteger which is used here

  integer() {
    this.isInteger && this.addConstraint("integer");
    return this;
  }

  get isInteger() {
    return this.config.isInteger(this.type);
  }

addConstraint can be found in the constraint builder class.

  addConstraint(propName, opts) {
    const constraint = this.build(propName, opts);
    if (constraint) {
      this.typeHandler.base = constraint;
      return constraint;
    }
    return false;
  }

Looks like it may need to be upgraded to pass in some additional options however, similar to this

      return this.addConstraint("integer", {
          value: this.getConstraints()
        });

So far the only integer test is here

Which uses the following test schema:

const createIntEntry = value => {
  const obj = { value, config, key: "value", type: "int" };
  return toYupNumberSchemaEntry(obj, config);
};

We could add an additional test with the other integer variant

const createIntegerEntry = value => {
  const obj = { value, config, key: "value", type: "integer" };
  return toYupNumberSchemaEntry(obj, config);
};

With the following test

  test("integer object - ok", () => {
    expect(createIntegerEntry({})).toBeTruthy();
  });

You can verify the integer support by running this specific test and expanding on it

$ jest number

or run the specific test(s) directly from VS Code

[kristianmandrup]

Should be sorted now. I've added the following tests to verify in number.test

  describe("min integer - validate", () => {
    describe("min: 2", () => {
      const entry = createIntEntry({ min: 2 });
      const schema = createSchema(entry);

      test("not int: invalid", () => {
        const valid = schema.isValidSync({
          value: 0.5,
        });
        expect(valid).toBeFalsy();
      });

      test("less: invalid", () => {
        const valid = schema.isValidSync({
          value: 0,
        });
        expect(valid).toBeFalsy();
      });

      test("more: invalid", () => {
        const valid = schema.isValidSync({
          value: 3,
        });
        expect(valid).toBeTruthy();
      });
    });
  });

[kristianmandrup]

Please verify via the latest commit on master. Then I will publish new release

[CatchABus]

Unfortunately, I tested latest repo changes and they don't seem to fix the problem. It seems tests are unable to catch this certain case. To be more precise, the problem is that integer is treated as a number by builder during validation. So if I set property to a decimal value, validation will still suceed.

I have created a sample in Codepen: https://codepen.io/catchabus/pen/gOqZVrv?editors=1112

[kristianmandrup]

Yes, I can see the codepen result is not what it should be

"the problem is that integer is treated as a number by builder during validation" - not sure I understand what you mean by this. Validation is done by Yup. The schema generated in Yup should be with an integer constraint as per the integer method in the YupNumber class in number/index.js

  integer() {
    this.isInteger && this.addConstraint("integer");
    return this;
  }

I've added two tests in number.test.js

  describe("min int - validate", () => {
    describe("min: 2", () => {
      const entry = createIntEntry({ min: 2 });
      const schema = createSchema(entry);

      test("not int: invalid", () => {
        const valid = schema.isValidSync({
          value: 0.5,
        });
        expect(valid).toBeFalsy();
      });

      test("less: invalid", () => {
        const valid = schema.isValidSync({
          value: 0,
        });
        expect(valid).toBeFalsy();
      });

      test("more than min: valid", () => {
        const valid = schema.isValidSync({
          value: 3,
        });
        expect(valid).toBeTruthy();
      });
    });
  });

  describe("min integer - validate", () => {
    describe("min: 2", () => {
      const entry = createIntegerEntry({ min: 2 });
      const schema = createSchema(entry);

      test("not int: invalid", () => {
        const valid = schema.isValidSync({
          value: 1.5,
        });
        expect(valid).toBeFalsy();
      });

      test("less: invalid", () => {
        const valid = schema.isValidSync({
          value: 0,
        });
        expect(valid).toBeFalsy();
      });

      test("more than min: valid", () => {
        const valid = schema.isValidSync({
          value: 3,
        });
        expect(valid).toBeTruthy();
      });
    });
  });

All the above tests pass locally. Strange.

[kristianmandrup]

I've now created a full integer test suite including your example and they all pass locally.

const { createIntEntry, createIntegerEntry, isNumber } = require("./_helpers");

describe("isNumber", () => {
  test("int", () => {
    expect(isNumber({ type: "int" })).toBeTruthy();
  });

  test("integer", () => {
    expect(isNumber({ type: "integer" })).toBeTruthy();
  });
});

describe("toYupNumber", () => {
  test("int object - ok", () => {
    expect(createIntEntry({})).toBeTruthy();
  });

  test("integer object - ok", () => {
    expect(createIntegerEntry({})).toBeTruthy();
  });

  test("integer min object - ok", () => {
    expect(createIntegerEntry({})).toBeTruthy();
  });

  describe("min int - validate", () => {
    describe("min: 2", () => {
      const entry = createIntEntry({ min: 2 });
      const schema = createSchema(entry);

      test("not int: invalid", () => {
        const valid = schema.isValidSync({
          value: 0.5,
        });
        expect(valid).toBeFalsy();
      });

      test("less: invalid", () => {
        const valid = schema.isValidSync({
          value: 0,
        });
        expect(valid).toBeFalsy();
      });

      test("more than min: valid", () => {
        const valid = schema.isValidSync({
          value: 3,
        });
        expect(valid).toBeTruthy();
      });
    });
  });

  describe("min integer - validate", () => {
    describe("min: 2", () => {
      const entry = createIntegerEntry({ min: 2 });
      const schema = createSchema(entry);

      test("not int: invalid", () => {
        const valid = schema.isValidSync({
          value: 1.5,
        });
        expect(valid).toBeFalsy();
      });

      test("less: invalid", () => {
        const valid = schema.isValidSync({
          value: 0,
        });
        expect(valid).toBeFalsy();
      });

      test("more than min: valid", () => {
        const valid = schema.isValidSync({
          value: 3,
        });
        expect(valid).toBeTruthy();
      });
    });
  });
});

describe("Integer schema test", () => {
  let yupSchema;
  beforeEach(() => {
    const buildSchema = (type) => ({
      title: "Person",
      description: "A person",
      type: "object",
      properties: {
        age: {
          type: "int",
        },
      },
    });

    yupSchema = {
      int: buildYup(buildSchema("int")),
      integer: buildYup(buildSchema("int")),
    };
  });

  test("int schema - not valid", () => {
    const isValid = yupSchema.int.isValidSync({
      age: 39.5,
    });
    expect(isValid).toBeFalsy();
  });

  test("integer schema - not valid", () => {
    const isValid = yupSchema.integer.isValidSync({
      age: 39.5,
    });
    expect(isValid).toBeFalsy();
  });
});

[CatchABus]

I meant that validator was fine with value of integer property being float which is wrong, so something is probably skipped while generating yup schema. I just tried to meddle with new tests myself but it seems there are failures on my end.

Test Suites: 9 failed, 4 skipped, 34 passed, 43 of 47 total
Tests:       5 failed, 32 skipped, 179 passed, 216 total
Snapshots:   0 total
Time:        2.616 s, estimated 4 s
Ran all test suites.

[kristianmandrup]

I looked again and you are correct. The issue is that addConstraint doesn't work correctly. It tries to determine if the constraint method is available on the yup object, but that is apparently not always a safe bet.

    // needs to be reworked for Yup 1.0
    const yupConstraintMethodName = this.aliasMap[method] || method;
    if (!yup[yupConstraintMethodName]) {
      const msg = `Yup has no such API method: ${yupConstraintMethodName}`;
      console.log(msg);
      this.warn(msg);
      return false;
    }

So there needs to be some major refactoring to fix this.

I've so far tried to refactor YupNumber as follows, but running into some weird issues

import { YupMixed } from "../mixed";
import { createRangeConstraint, RangeConstraint } from "./range-constraint";
import { createNumberGuard, NumberGuard } from "./guard";

const proceed = (obj, config = {}) => {
  return createNumberGuard(obj, config).verify();
};

function toYupNumber(obj, config = {}) {
  return proceed(obj, config) && buildYupNumber(obj);
}

function toYupNumberSchemaEntry(obj, config = {}) {
  return proceed(obj, config) && buildSchemaEntry(obj);
}

function buildSchemaEntry(obj) {
  return YupNumber.schemaEntryFor(obj);
}

function buildYupNumber(obj) {
  return YupNumber.create(obj);
}

class YupNumber extends YupMixed {
  constructor(obj) {
    super(obj);
    this.type = this.baseType;
    this.base = this.validatorInstance;
    this.rangeConstraint = createRangeConstraint(this);
  }

  get baseType() {
    return this.normalizeNumType(this.opts.type);
  }

  get validatorInstance() {
    return this.validator.number();
  }

  normalizeNumType(type) {
    return type === "int" ? "integer" : type;
  }

  static create(obj) {
    return new YupNumber(obj);
  }

  static schemaEntryFor(obj) {
    return YupNumber.create(obj).createSchemaEntry();
  }

  // missing round and truncate
  get typeEnabled() {
    return ["range", "posNeg", "integer"];
  }

  convert() {
    super.convert();
    return this;
  }

  range() {
    this.rangeConstraint.add();
    return this;
  }

  // use base.truncate() ?
  truncate() {
    return this.addConstraint("truncate");
  }

  round() {
    const { round } = this.constraints;
    if (this.isNothing(round)) {
      return this;
    }
    const $round = this.isStringType(round) ? round : "round";
    this.base = this.base.round($round);
    return this;
  }

  posNeg() {
    this.positive();
    this.negative();
    return this;
  }

  integer() {
    console.log("try add integer constraint");
    if (!this.isInteger) {
      console.log("not an integer type", this.type);
      return;
    }
    if (!this.base.integer) {
      console.error("invalid base object", this.base);
    }
    this.base = this.base.integer();
    return this;
  }

  get isInteger() {
    return this.config.isInteger(this.value);
  }

  positive() {
    if (!this.isPositive) return this;
    //return this.addConstraint("positive");
    this.base = this.base.positive();
    return this;
  }

  negative() {
    if (!this.isNegative) return this;
    // return this.addConstraint("negative");
    this.base = this.base.negative();
    return this;
  }

  get isNegative() {
    const { exclusiveMaximum, negative } = this.constraints;
    if (negative) return true;
    if (exclusiveMaximum === undefined) return false;
    return exclusiveMaximum === 0;
  }

  get isPositive() {
    const { exclusiveMinimum, positive } = this.constraints;
    if (positive) return true;
    if (exclusiveMinimum === undefined) return false;
    return exclusiveMinimum === 0;
  }

  normalize() {
    this.constraints.maximum = this.constraints.maximum || this.constraints.max;
    this.constraints.minimum = this.constraints.minimum || this.constraints.min;
  }
}

export {
  toYupNumber,
  toYupNumberSchemaEntry,
  YupNumber,
  createNumberGuard,
  NumberGuard,
  RangeConstraint,
  createRangeConstraint,
};

    try add integer constraint

      at YupNumber.log [as integer] (src/types/number/index.js:90:13)
          at Array.map (<anonymous>)

  console.error
    invalid base object true

      94 |     }
      95 |     if (!this.base.integer) {
    > 96 |       console.error("invalid base object", this.base);
         |               ^
      97 |     }
      98 |     this.base = this.base.integer();

When running

node 'node_modules/.bin/jest' '/Users/kristian/repos/personal/schema-to-yup/test/types/number/integer.test.js' -t 'Integer schema test int schema - not valid'

base should be the yup schema object that is continually built using a chaining builder pattern. Now it is simply true

[kristianmandrup]

Ideally the whole library should be re-engineered from the ground up to be more modular and much simpler.

[CatchABus]

I see, thanks a lot for the detailed explanation and good job figuring out! As a workaround, I'm currently setting my own type handler for number. Also, glad to hear library can become simpler.

I took a small look at how yup operates now. This is an example on how one can have access to supported number constraints dynamically:

import { NumberSchema } from "yup";

console.log(Object.getOwnPropertyNames(NumberSchema.prototype));

The result:

['constructor', 'min', 'max', 'lessThan', 'moreThan', 'positive', 'negative', 'integer', 'truncate', 'round']

[kristianmandrup]

Working on fix here: https://github.com/kristianmandrup/schema-to-yup/tree/fix-valid-method