timestamp fields get parsed as string in the ts client

[kklas]

Currently the generated typescript client code simply uses JSON.parse() when parsing responses. timestamp fields will get decoded as strings while in the generated type they're marked as a Date object.

I guess it's related to #3 since validation and decoding would be done at the same time?

[tj]

ahh I probably overlooked that, need an automated way to test the generated clients, doing it manually isn't great haha, they should be Dates. I didn't realize TypeScript still allows you to assign invalid types via JSON.parse(), thought it would do something magical there, it seems a bit odd that you can even assign any to a known type but I've never really used TypeScript

[tj]

I guess we need something like this in Deno / Node since JSON.dateParser isn't available:

const reDate = /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2}(?:\.\d*))(?:Z|(\+|-)([\d|:]*))?$/
   
function dateParser(key, value) {
  return typeof value == 'string' && reDate.test(value)
    ? new Date(value)
    : value
}

console.log(JSON.parse(`{ "name": "Whatever", "created": "2020-07-07T18:26:34.503Z" }`, dateParser))
{ name: 'Whatever', created: 2020-07-07T18:26:34.503Z }

[kklas]

Elegant. Yea TypeScript in a way is just optional typing on top of javascsript. TypeScript spec doc says this:

TypeScript is a syntactic sugar for JavaScript. TypeScript syntax is a superset of ECMAScript 2015 (ES2015)syntax. Every JavaScript program is also a TypeScript program.

So I guess nothing magical is possible because that would change the semantics of JSON.parse(). It's code is written in JS so it's just left as it is by the compiler. And of course, there're no types at runtime.

Do you mind if I patch it in?

[tj]

sure that would be great! 🎉

[kklas]

This regex in not quite right. It's not detecting 2020-07-08T16:25:52+02:00 which is what I got in a real response from the go server.

Found a different one here which works for all my test cases, but it's long:

/(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))/

It's either that or whitelisting timestamp fields in the generator.

[tj]

hmm I somehow thought date strings were part of the spec but I guess not haha, looks like Go uses RFC3339Nano = "2006-01-02T15:04:05.999999999Z07:00"

[tj]

let me know what you think of #27

new Date seems to handle Go's nano resolution fine but maybe it's not worth even trying to have an exact regexp

[kklas]

I think that's going to lead to more problems because the user might want to pass a timestamp in a string field for whatever reason.

This is a validation issue IMO. The biggest risk here is that we fail silently because the users are going to expect that since there is a schema file we have proper typing. The proper way to do it is to implement a validator on the client. We also have to decide on a timestamp standard because different languages will do it differently.

[tj]

Yeah true it could potentially be a problem if they really want a date string, maybe you're right about flagging which fields should be dates, those are already explicitly defined. Though it would be super safe to just use what the browser does, but apparently Go disagrees haha

he only tricky part there I suppose is they could be nested, so we'd need something more like the Go server Validate() methods.

[kklas]

How about we have a decoder method which receives the response and has schema info. Inside it we do a normal JSON.parse() and then convert the timestamp fields with new Date() and return the modified object. This way we can do proper decoding without having to do validation for now.

But yeah the tricky part might be nesting.

[kklas]

Actually when I think about it a bit more, we don't really need validation on the client because we can just assume that the server is sending the right thing and we can check that with testing. Unless maybe for security reasons if you don’t trust the server.

In any case I'm closing the issue because it has been fixed. But we have a new problem now and that is that string field containing timestamps are going to get parsed as Date objects. We might need a field based decoder for that so I'm going to create a separate issue.

[tj]

SGTM, what we have now seems like it should be fine for 99% of cases at least, it would be a bit weird to want a complete timestamp as a string but maybe someone will haha. As long as we don't hit any weird edge-cases with the regexp failing on certain timestamps it should be fine

[kklas]

Exactly