search

ostrowr / ts-json-validator

Let JSON play nicely with Typescript
MIT License
342 stars 7 forks source link

Question: Why `{[k: string]: unknown}` for `{type: "object"}` schemas? #28

Closed thomasboyt closed 4 years ago

thomasboyt commented 4 years ago

👋 hi! I've been investigating various forms of runtime validation in TypeScript over the past couple weeks (in the service of making a small HTTP router with typed and validated parameters/query/body), and I stumbled upon this while looking for a better way to use JSON Schema than duplicating my interfaces and schemas. This is really awesome work!

I've just started looking into using this, but I had a quick question: I was surprised to see that the base type for an object schema is {[k: string]: unknown}, rather than simply {}. I discovered this when accessing a nonexistent field on a validated object showed a property with unknown, rather than the usual "Property foo does not exist" error.

In practice, this isn't a huge deal - any attempts to actually use a nonexistent field will result in an error (since you can't use unknown without a type assertion) - but sometimes results in less intuitive errors than I'd like, as figuring out why something is "unknown" when you try to use it can be less intuitive than being warned the field doesn't exist when first accessed.

I assumed there was some important reason for using an unknown map, but out of curiosity, I forked the repo and changed this line to return {} instead of the map, and all of the tests still passed (except for the one that expects the map type, but that's a quick change). Now, I get the "property does not exist" errors that I was hoping for when I try to use a nonexistent field.

So, now I'm wondering: there a subtle reason to use the unknown map that I'm missing? If not, I'm hopeful it can be changed to {} going forward :)

ostrowr commented 4 years ago

Good question!

By default, additional properties are allowed in a JSON schema – which means that I can parse an object with the keys "hello" and "goodbye" with a schema that only specifies "hello" – and the validation will pass.

In that case, I want "goodbye" to be available on the derived type, but with an unknown value.

That said, though, the error messages that this library emits are already nearly impossible to parse, and I'm not sure how much utility specifying something as unknown adds, since hopefully a consumer of this library will specify all of the types they expect in their schema.

I have to think about this a little more tonight – and whether there's any valid use case for leaving properties out of your schema – but if you open a PR with your fork I'll take a closer look!

ostrowr commented 4 years ago

I think I'll leave the unknown behavior in for now, but happy to reconsider with a concrete argument that the "unknown" type makes this library markedly harder to use.