Make compiled validation interface async aware

[villasv]

What issue does this pull request resolve?
Fixes #1279

What changes did you make?

1. Added specific interfaces for schemas with $async: true and schemas with $async: false
2. Added specific interfaces for validation functions compiled from said specific schemas
3. Overloaded the compile and compileAsync with a different signatures for each schema subtype

Is there anything that requires more attention while reviewing?

This will be a major compilation breaking change. The following code will not compile anymore, because validator now will return a narrower type. In truth, the problematic code was never running anyway, it should be just deleted.

const validator = ajv.compile({ $async: false });
const result = validator(data);

if (typeof result === "boolean") {
    // sync
    console.log(result);
} else {
    // async
    result.then(value => { // Property 'then' does not exist on type 'never'.ts(2339)
        data = value;
    });
}

However, using ajv.compile({}) or broad types like { $async: boolean } will maintain old behavior:

const schema = { $async: true };
const validator = ajv.compile(schema);
const result = validator(data);
//      ^^^^ still boolean | Promise<any>

This happens because the first overloaded signature had to be

compile(s: {$async?: never}, _?: boolean): ValidateFunction

Otherwise using ajv.compile({} as any) would yield an incorrectly narrow-typed validator.

So I had to choose between wrongly narrowed types for any and Record<string, unknown> typed schemas (which are common); or failing to guess that { $async: undefined } is the same as { $async: false }. I chose the latter.

[epoberezkin]

What does compile(s: {$async?: never}, _?: boolean): ValidateFunction do?

[villasv]

It's there to capture types that would match with anything. I tried to explain briefly in the PR description but I'll provide concrete examples.

The initial attempt would be:

  compile(s: SyncSchemaObject, _?: boolean): SyncValidateFunction
  compile(s: AsyncSchemaObject, _?: boolean): AsyncValidateFunction
  compile(s: Schema, _?: boolean): ValidateFunction

But then if I do:

function getValidator(schema: any) {
  return ajv.compile(schema);
}

The return type is SyncValidateFunction, because any is assignable to SyncSchemaObject (the first overload available). We don't want that, because that schema may have an $async: true option.

So the first overload has to be something that any will match to, and should return ValidateFunction. But it can't be something generic like any, or Schema because then it will always match and none of the other overloads will ever be used.

So it has to be something that will match with any and Record<string, uknown> (another very commonly used type for schemas), but won't match with a SyncSchemaObject or an AsyncSchemaObject.

One way to do it would be using { $async?: never }. But in reality, anything that is all optionals and is incompatible with Schema would suffice (like { $id?: never}). I left it with $async as to make it a bit easier to associate that overload presence with async typing issues.

[villasv]

The test suite I'm adding will fail to compile if that alien-looking overload is removed, because I explicitly added code paths that require schemas with type any to return either booleans or promises. I've just resolved conflicts, you should be able to try it out for yourself now.

[epoberezkin]

I think I understand - I will experiment with the tests.

I was also thinking that arg names in overload should be descriptive, rather than one-letter. These overloads come up in error messages and editor hints I think - is that correct?

I will change it if that's ok.

[epoberezkin]

I merged - somehow it worked without $async: never - please have a look.

Maybe because it is sync by default, false is not needed there - I corrected the types.

re "I am not particularly interested in using validation as a type guard"

I think it would be really cool to solve it in the spirit of "parse don't validate". If the schema represents a certain type, then having validation return type guard would be zero-cost for js users, but lots of value for ts users.

Another interesting question is whether it would be possible to make a type level transformation of data type into schema interface type - that is, if the schema represents a certain data type, then it should have this interface type, so that users would have additional type safety if they write schemas by hand (or even if they generate them). Some kind of:

interface MyData: {
  foo: string
  bar: number
}

type MyDataSchema = JSONSchemaType<MyData>

// equivalent to:
// interface MyDataSchema {
//   type: "object"
//   properties: {
//     foo: {type: "string"}
//     bar: {type: "number"}
//   }
//   required: ["foo", "bar"]
// }

const myDataSchema: MyDataSchema = {
  type: "object"
  properties: {
    foo: {type: "string"}
    bar: {type: "number"}
  }
  required: ["foo", "bar"]
}

So even though typescript would not convert type to data (although maybe it can?), at least if the data does not match the type it would not compile.

[epoberezkin]

With this "utility" type JSON schema literally writes itself (VSCode shows what it should look like in the hint and you can copy paste it from the hint with minimal changes):

type JSONSchema<T> = T extends number
  ? {
      type: "number" | "integer"
      [keyword: string]: any
    }
  : T extends string
  ? {
      type: "string"
      [keyword: string]: any
    }
  : T extends boolean
  ? {
      type: "boolean"
    }
  : T extends undefined | null
  ? {
      type: "null"
    }
  : T extends [any, ...any[]]
  ? {
      type: "array"
      items: {
        [P in keyof T]: JSONSchema<T[P]>
      } & {length: T["length"]}
    }
  : T extends any[]
  ? {
      type: "array"
      items: JSONSchema<T[0]>
      [keyword: string]: any
    }
  : {
      type: "object"
      properties: {
        [P in keyof T]: JSONSchema<T[P]>
      }
      required: (keyof T)[]
    }

Now if you try to write schema for some interface it won't type check if it is not correct:

interface MyData {
  foo: string
  bar: number
  baz: {
    empty: null
  }
  boo: boolean
  arr: {id: number}[]
  tuple: [number, string]
}

const mySchema: JSONSchema<MyData> = {
  type: "object",
  properties: {
    foo: {type: "string"},
    bar: {type: "number"},
    baz: {
      type: "object",
      properties: {
        empty: {type: "null"},
      },
      required: ["empty"],
    },
    boo: {type: "boolean"},
    arr: {
      type: "array",
      items: {
        type: "object",
        properties: {
          id: {
            type: "integer",
          },
        },
        required: ["id"],
      },
    },
    tuple: {
      type: "array",
      items: [{type: "number"}, {type: "string"}],
    },
  },
  required: ["foo", "bar"],
}

[villasv]

I merged - somehow it worked without $async: never - please have a look.

The test was changed:

describe("of type any", () => {
    const schema: any = {}
    const validate = ajv.compile(schema)

became

describe("= any", () => {
    const schema: SchemaObject = {}
    const validate = ajv.compile(schema)

So it's not testing the any case anymore.

Also, the new tests are asserting like this:

      if (typeof result === "boolean") {
        should.exist(result)
      } else {
        throw new Error("should return boolean")
      }

Which is not the same thing as the original. The goal was to create a code path that assumed the type of result would be promise to cause a compilation failure if the typing is wrong. With the tests currently the way they are, the typing is wrong but the tests are still passing.

I'm sorry, I could have done better test cases. The examples I gave for the unknown and any case makes it look like those scenarios should indeed return boolean, when in fact they shouldn't and the reason isn't obvious.

The following are more explicit about why those scenarios need $async: never in there, and why they need to return boolean | Promise<any>

describe("$async: uknown", () => {
    const schema: Record<string, uknown> = { $async: 'bananas' }
    const validate = ajv.compile(schema)

describe("$async: any", () => {
    const schema: any = { $async: 'bananas' }
    const validate = ajv.compile(schema)

These two scenarios should return boolean | Promise<any>, but with current typing will return boolean.

[epoberezkin]

I see. The problem was that to get SyncValidateFunction you had to specify $async: false. I was trying to achieve that it would give SyncValidateFunction without $async property, and it did result into unknown/any $async returning SyncValidateFunction.

Is there a way to achieve that it would get SyncValidateFunction with $async: undefined, but ValidateFunction with any/unknown?

Re tests I guess I was confused about what they were testing - I thought from the descriptions they were supposed to test the actual result, not the type of the result. I've reverted now with two commented lines for $async: any/unknown

[epoberezkin]

Returning {$async: never} as the first overload fixes the last two tests, but breaks the first - ajv.compile({}) returns ValidateFunction.

So the trade-off seems to be:

1. to have "incorrectly" typed default case, when to get AsyncValidateFunction you would have to assert that the schema is AsyncSchemaObject. This would work better for the majority of users who don't use async validation (and don't even know what it is).
2. to have validate function typed broadly unless schema either have explicit $async: false (becoming non-standard) or explicitely typed as SyncSchemaObject.

[villasv]

It's not possible to do it with simple overloads like this, AFAIK. But may be possible if we use generics and type guards. Will take a look into it soon.

[epoberezkin]

Thank you. For now I've put the {$async: never} back, added type guard and the "utility type" - a bit more advanced version of the above:

type: https://github.com/ajv-validator/ajv/blob/v7-alpha-resolve/lib/types/json-schema.ts
test for it: https://github.com/ajv-validator/ajv/blob/v7-alpha-resolve/spec/types/json-schema.spec.ts