Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
thomasboyt opened this issue Feb 8, 2020 · 2 comments
Closed

Comments

@thomasboyt
Copy link

thomasboyt commented Feb 8, 2020

👋 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
Copy link
Owner

ostrowr commented Feb 10, 2020

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
Copy link
Owner

ostrowr commented May 21, 2020

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.

@ostrowr ostrowr closed this as completed May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants