Allow integer in number subschema with strictTypes #1938
Conversation
| @@ -286,7 +286,7 @@ function checkContextTypes(it: SchemaObjCxt, types: JSONType[]): void { | |||
| strictTypesError(it, `type "${t}" not allowed by context "${it.dataTypes.join(",")}"`) | |||
| } | |||
| }) | |||
| it.dataTypes = it.dataTypes.filter((t) => includesType(types, t)) | |||
| it.dataTypes = it.dataTypes.filter((t) => includesType([...types, "number"], t)) | |||
There was a problem hiding this comment.
I'm pretty sure this is the offending line, just not sure if this is the optimal solution.
In my example from the issue, it.dataTypes === ['number'] and types === ['integer']
The existing logic would filter dataTypes down to an empty array, then throw on error on keywords like maximum without a type.
It is filtered because includesType does not detect any overlap, and would (by intentional design) only allow number in the base type.
includesType(['number'], 'integer') === true
includesType(['integer'], 'number') === falseMy solution is to simply append number to the type array, to effectively allow the second case to be true as well.
Intuitively it seems like an overly broad solution, as number is not a valid type for every other type, and we still want to prevent number inside an integer.
However the lines just before this are performing that validation/filtering, so I'm pretty sure at this point both type arrays are are length <= 1.
If there is a better way to write this, or perhaps another way to attack this let me know!
There was a problem hiding this comment.
it's indeed more tricky than that... updated!
|
updated in #2192 |
What issue does this pull request resolve?
resolves #1935
What changes did you make?
Add
numbertoincludesTypecheck duringstrictTypesvalidation.Is there anything that requires more attention while reviewing?
Might be an overly broad solution. Should check be more conservative about appending
numbertype?