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

Feature: dict with number keys #39

Open
laurence-bird opened this issue Aug 5, 2019 · 2 comments
Open

Feature: dict with number keys #39

laurence-bird opened this issue Aug 5, 2019 · 2 comments

Comments

@laurence-bird
Copy link

laurence-bird commented Aug 5, 2019

Currently the dict decoder only supports keys as string types :

static dict = <A>(decoder: Decoder<A>): Decoder<Record<string, A>> =>

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

@geraldalewis
Copy link

@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

@mulias
Copy link
Contributor

mulias commented Aug 6, 2019

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.

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

3 participants