-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Response Date type is received as string #104
Comments
Minimally reproducible example? |
@mrctrifork Here you go const app = new Elysia().get('/', () => ({ date: new Date() })).listen(3000)
const api = treaty<typeof app>('http://localhost:3000')
const res = await api.index.get()
console.log(res.data.date.getTime()) // Expected to work
// Actual result: Property 'getTime' does not exist, because date is a string The intellisense shows the type of |
You're right. I fixed a very similar bug some time ago in the websocktets part of eden. See this PR: #87 The fix should be simple but again; check this discussion to see why eden cannot differentiate when to parse a date from a string or to leave it as a string. |
Thank you for looking into this. So the fix would involve adding the same check for ISO8601 strings, which isn't done currently in treaty correct? If you ask me, I think it's worth adding to improve developer experience, even though we are limited by JSON structure. Perhaps one day Elysia could go the protobuf route and use a custom binary format for messaging to ensure 100% e2e type safety, which comes with its own downsides of course. |
Totally agree, I am not the author or the owner of this project; I just really like it :P; so it's not really my call You could try submitting a similar PR to mine that checks for Date-shaped values as the values of a JSON response and attempt to parse them as dates. |
I'll take a crack at it later today, looks pretty straight forward (never contributed to this project, so we'll see how it goes :P ) P.S. I Like this project a lot too :D |
@mrctrifork Looking at the code I realized that in the case of P.S. while making the changes I realized there is a lot of duplicated code surrounding parsing (both in treaty2 and across other modules) I'm trying to combine everything into one parseUtils.ts file, what do you feel about it? Probably as part of this PR only treaty2 should be changed, in order to not make it as big. |
I agree that recursively traversing objects of unknown depth seeking for date-likes could take a hit on the performance. I gave it some thought; and the approach with the least friction from the user" I believe is that this behavior could be toggled by a boolean flag; either when instantiating eden level or per-request level. Another approach; perhaps Eden could ship an "onResponse" callback that people would import directly from the library and pass it along whenever they require dates to be parsed; making "enabling" such behavior explicit. Regarding the dupped code; at least to me, it makes sense to extract common factor and reuse code whenever possible. Still, @SaltyAom might have reasons (of which, I am unaware of) on why this was done in such a way. Since this is not my project I don't think I get to say how it should be done, hopefully @SaltyAom will share his opinion on the matter. |
I thought about it some more, and I think that adding this step after calling There might be more creative solutions, but I don't have any easy ones off the top of my head. At least the docs should be updated to mention this known issue, since it may occur in many places where objects/arrays are sent with dates. Because the parsing code isn't centralized to one place, edge cases like that might reappear elsewhere. (Because of this we also sometimes check different standards to parse strings into Date for example) I hope @SaltyAom can weigh in on this. |
True. The obvious approach sounds like it'd be to have the typebox schema also on the frontend do the parsing for each request but of course sometimes we don't even use it; like in your example. |
Another solution to think about would be to use superjson? |
|
I have an endpoint which returns an object containing a
Date
field, eden displays the expected response correctly withDate
so there are no TS errors. But during runtime, the field is actually a string.Of course it makes sense because the JSON body can't contain
Date
fields, but if the intellisense is telling me that, I should receive a Date. It's still broken.Ideally eden would have some process to fix this issue and convert the string to a Date, another solution would be for eden to interpret
Date
s as strings...The text was updated successfully, but these errors were encountered: