ajv: JSONSchemaType incorrectly requires optional properties to be nullable
What version of Ajv are you using? Does the issue happen if you use the latest version?
Ajv 8.6.0, Typescript 4.3.4
Your typescript code
import { JSONSchemaType } from 'ajv/dist/2020';
interface Example {
foo: string;
bar?: string;
}
const schema: JSONSchemaType<Example> = {
additionalProperties: false,
type: 'object',
properties: {
foo: {
type: 'string'
},
bar: {
type: 'string'
}
},
required: ['foo']
};
Typescript compiler error messages
TS2322: Type '{ additionalProperties: false; type: "object"; properties: { foo: { type: "string"; }; bar: { type: "string"; }; }; required: "foo"[]; }' is not assignable to type 'UncheckedJSONSchemaType<Example, false>'.
Type '{ additionalProperties: false; type: "object"; properties: { foo: { type: "string"; }; bar: { type: "string"; }; }; required: "foo"[]; }' is not assignable to type '{ type: "object"; additionalProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; unevaluatedProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; ... 7 more ...; maxProperties?: number | undefined; } & { ...; } & { ...; } & { ...; }'.
Type '{ additionalProperties: false; type: "object"; properties: { foo: { type: "string"; }; bar: { type: "string"; }; }; required: "foo"[]; }' is not assignable to type '{ type: "object"; additionalProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; unevaluatedProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; ... 7 more ...; maxProperties?: number | undefined; }'.
The types of 'properties.bar' are incompatible between these types.
Type '{ type: "string"; }' is not assignable to type '{ $ref: string; } | (UncheckedJSONSchemaType<string | undefined, false> & { nullable: true; const?: undefined; enum?: readonly (string | null | undefined)[] | undefined; default?: string | ... 1 more ... | undefined; })'.
Type '{ type: "string"; }' is not assignable to type '{ type: "string"; } & StringKeywords & { allOf?: readonly UncheckedPartialSchema<string | undefined>[] | undefined; anyOf?: readonly UncheckedPartialSchema<string | undefined>[] | undefined; ... 4 more ...; not?: UncheckedPartialSchema<...> | undefined; } & { ...; } & { ...; }'.
Property 'nullable' is missing in type '{ type: "string"; }' but required in type '{ nullable: true; const?: undefined; enum?: readonly (string | null | undefined)[] | undefined; default?: string | null | undefined; }'.
Describe the change that should be made to address the issue?
This code will not compile unless bar is marked as nullable
in the schema. However, I don’t want to accept null
as a value for bar if it is present. It appears that JSONSchemaType
expects all properties not mentioned in the required
array to be marked as nullable
. null
and undefined
are different values and should not be conflated.
Are you going to resolve the issue? 🤷
About this issue
- Original URL
- State: open
- Created 3 years ago
- Reactions: 28
- Comments: 18 (5 by maintainers)
Commits related to this issue
- make JSONSchemaType treat null as nullable fixes #1664 JSONSchemaType now treats null as nullable instead of undefined, allowing specifying non-null optional parameters. They way that `Nullable<T>`... — committed to erikbrinkman/ajv by erikbrinkman 3 years ago
- make JSONSchemaType treat null as nullable fixes #1664 JSONSchemaType now treats null as nullable instead of undefined, allowing specifying non-null optional parameters. They way that `Nullable<T>`... — committed to erikbrinkman/ajv by erikbrinkman 3 years ago
- JSONSchemaType change nullable to be for nulls not undefined (#1701) * Upgrade to GitHub-native Dependabot * json-schema.md remove extra quotation mark (#1696) Co-authored-by: Erik Brinkman <er... — committed to ajv-validator/ajv by erikbrinkman 3 years ago
Hey future traveler đź‘‹
I just discovering this post and was about to give up, when I saw
nullable: true
and it worked!Here’s a clear example:
@erikbrinkman i think the correct way to improve it in the next major version is to only require nullable: true if the property has null as possible value in typescript, independently of whether it’s required or optional. That will be a breaking change.
This is a deliberate decision to require that optional properties are allowed to be null values, as it is how APIs are commonly designed…
Whether it was a correct choice is a good question but it’ll definitely stay like that until the next major version…
The problem with annotating a property with
nullable: true
is that allows for runtime values where the key exists and the value is null. This is different than the key being missing entirely or the value beingundefined
.An example object that would pass validation of the schema you provided that I would not want to be valid:
To say this another way, I want to be able to define a schema that says “if the
embed
key exists, the value must be a string”. This type of rule is important when the object in question goes through some mapping process whereObject.entries(([k, v]) => {})
is used.Using your schema, the signature of the callback is:
whereas I’d like to write a callback with a signature of
It’s not that optional properties are “allowed” to be null values, it’s that they must be marked as nullable in the schema. This means that it is impossible to use
JSONSchemaType
with any schema that has a non-nullable optional property. This seems like a pretty common use case…This is breaking because it change the existing typechecks in a way that causes type errors where there wouldn’t be before.
I think there’s an argument for gradual migration, but I don’t think it’s very compelling. Gradual migration for types like this is really complicated. It’s not about a flag in the validator, it’d be in constructing the type itself. The migration path would be more like creating two JSONSchemaTypes, and having validate work for either schema type. However this leaves a number of other problems:
However, there already exist a number of fixes for people who really care, and make the migration tradeoffs themselves:
If none of this is compelling, you’re always free to submit a PR with a proposed fix so we can further discuss the benefits versus costs of gradual migration.
Yes, this hasn’t been landed yet because it’s breaking, and hence why it’s in the v9 branch. I can’t speak for when v9 should be released.
I found https://github.com/ajv-validator/ajv/commit/b4b806fd03a9906e9126ad86cef233fa405c9a3e which is itself quite old at this point. Is this change not included because it is seen as a breaking change to the current typescript API?
Is there something we can follow to see when that happens? Would this be v9?
@kilahm See my post previously. ajv does validate appropriately if you leave out the
nullable
, it’s just the the type converter doesn’t handle it. You can either wait until the next major version of ajv where it’s fixed, or use my casting trick above to make it think the types work out, while still getting the validation you want.I should be clear that I think what needs to be redone from scratch is just
SomeJSONSchema
e.g. just a type for unchecked JSON Schemas. This should be independent of the actual implementation ofJSONSchemaType
which is why I think it’s an easier change. It still won’t be backwards compatible thought.As a side note, it may fix: #1652 as I think that also stems from misalignment of
SomeJSONSchma
. When / if implementing this, it probably makes sense to add a test for that too, but I’m ultimately not sure if this will fix that.