json-type-validation icon indicating copy to clipboard operation
json-type-validation copied to clipboard

Feature: dict with number keys

Open laurence-bird opened this issue 6 years ago • 2 comments

Currently the dict decoder only supports keys as string types : https://github.com/mojotech/json-type-validation/blob/c28d34dd7ecc3d89210573abba56f7cf91ae53cf/src/decoder.ts#L403

Would you accept a pull request to provide the ability to decode dicts with numbers as keys? Happy to do the work and raise if so

laurence-bird avatar Aug 05 '19 11:08 laurence-bird

@mulias there's an implied expectation that input is valid JSON ("json-type-validation"), but it doesn't technically have to be, right? I can't remember if you explicitly made a determination on that.

Otherwise, JSON object fields must be strings:

member
    ws string ws ':' element

geraldalewis avatar Aug 06 '19 16:08 geraldalewis

I'm still chewing this over, but here are some initial thoughts

I think the best way to think about the scope of this project is that decoders take in values of the type returned by JSON.parse and validate to more specific types. Unfortunately the return type of JSON.parse is any. It would be nice if we instead got a more accurate JsonValue type to reflect that functions etc are not included. Maybe it would have been better to support any JS value, but I don't think I'm going to go back on that at this point. Different goals, different project.

So while it's true that the json spec requires object fields to be strings, when we parse a json encoded object into javascript the resulting JS object can have both string and number keys. So objects with number keys are part of the JsonValue type.

Maybe relevant, here's how typescript handles string vs number records:

const c: Record<string, string> = {1: "a"}
c["1"] // just fine
c[1] // also fine

vs

const c: Record<number, string> = {1: "a"}
c["1"] // error
c[1] // ok

@laurence-bird have you experimented with finding a way to get the behavior you want with the existing decoders? I ask because I'm not 100% sure if it is or is not possible to create a Decoder<Record<number, A>> already. That would be my first step before evaluating the best way to add a new feature.

Thanks for the question. I'd look over a PR, but at this point I'm not sure exactly what the API would look like, so I'm not yet confident if this is a feature we can or want to support.

mulias avatar Aug 06 '19 18:08 mulias