joanllenas / ts.data.json

1.7kB JSON decoding library for Typescript
BSD 3-Clause "New" or "Revised" License
205 stars 16 forks source link

Improve the case where the input to a decoder is not an object #23

Open rhofour opened 4 years ago

rhofour commented 4 years ago

I just spent a fair bit of time trying to debug why an object decoder was failing, only to eventually consult the code and realize it's because the input I was passing was a JSON string rather than an actual object.

I think there are a couple of possible ways this could be better: 1) The error given could clearly state the input is not an object rather than just saying it's not a valid decoderName. 2) Strings could be automatically parsed to json. 3) The input to the decoder could be set to a more specific type than Any so the compiler could catch this.

Let me know what your thoughts are. If you'd like I can send a PR to do some or all of these, but I want to hear what you think first.

joanllenas commented 4 years ago

Hi @rhofour ,

  1. What wording do you suggest? Here is the function that the lib. uses to display that kind of error.
  2. This is a valid decoder: JsonDecode.string.decode('hello'), if you try to JSON.parse('hello') you'll get an exception. So, this is not an option.
  3. What do you suggest? This is where the type would have to be placed.
rhofour commented 4 years ago
  1. I think just saying "object" rather than decoderName would be more clear since the point at which that error comes up is when the input isn't an object. When I saw the name of my decoder I assumed the problem was in my decoder itself and it took a while to realize the problem was just in the input.
  2. I was thinking specifically for the object decoder, but I imagine this might cause problems with some other bits like failover. I think 1 and 3 are the more important bits.
  3. Do you see anything wrong with doing something like this?
    export class Decoder<a, b> {
    constructor(private decodeFn: (json: b) => Result<a>) {}
    decode(json: b): Result<a> {
joanllenas commented 4 years ago

Improving the API to avoid any adds verbosity, complicates things quite a bit in some scenarios, and it would be a breaking change. I think I'll stick to the "improve error messages" plan. I have some ideas.