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

typescript-angular: type difference between date and date-time #4170

Closed
skorunka opened this issue Oct 16, 2019 · 11 comments
Closed

typescript-angular: type difference between date and date-time #4170

skorunka opened this issue Oct 16, 2019 · 11 comments

Comments

@skorunka
Copy link

Yaml:

        dateAndTime:
          format: date-time
          type: string

is generated for typescript-angular as:

dateAndTime?: Date;

Yaml:

        date:
          format: date
          type: string

is generated for typescript-angular as:

date?: sring;

Why is there difference?
Why is not date-time generated as string?

Thnx.

@skorunka skorunka added the Announcement Project/release related announcements label Oct 16, 2019
@macjohnny
Copy link
Member

a date would be something like 2019-10-16, but since Date can also contain time information I think string is a good choice for date properties.
for date-time, it should be string, too, since Date is not serializable in json. We usually override the type to be string, too.
But this should be changed in the code.
@akehir what do you think?

@macjohnny macjohnny changed the title Why YAML oas3 format: date-time is generated as Date intypescript-angular and not as string(as ormat: date is) typescript-angular: type difference between date and date-time Oct 16, 2019
@macjohnny macjohnny added Client: TypeScript Issue: Bug and removed Announcement Project/release related announcements labels Oct 16, 2019
@akehir
Copy link
Contributor

akehir commented Oct 16, 2019

I agree, it should be string.

However, that could lead to "breaking" changes in typescript being used today (which is admittedly better than the current case, where you could run into runtime bugs).

The main drawback I see, however, is that you loose the 'documentation' given by the "Date" type. Right now, if I get a Date in the generated typescript. I know that I can parse that string into a JS date, which can be helpful.

@skorunka
Copy link
Author

skorunka commented Oct 17, 2019

Hi, well I'm fine with both being string or date(I prefere strings though) but it should be consistent, which is not now :(

Right now, if I get a Date in the generated typescript. I know that I can parse that string into a JS date, which can be helpful.

I'm sorry, but I don't understand what you mean?

@macjohnny
Copy link
Member

since there is still the possibility to supply the type-mapping option, I would change it to string for both, date and date-time.
@skorunka would you like to file a PR?

@skorunka
Copy link
Author

@macjohnny Might try to. But if you guys know its 5 mins job for ya, go for it. It might take hours for me. But yeah, will try..

@macjohnny
Copy link
Member

@skorunka I can point you to the right files to change

@macjohnny
Copy link
Member

I think the source is here:

typeMapping.put("date", "string");
typeMapping.put("DateTime", "Date");

a custom type mapping should be added here:

then simply run

mvn clean package
bin/typescript-angular-petstore-all.sh

and commit the changed samples in the /samples folder

@Xambey
Copy link
Contributor

Xambey commented Dec 26, 2019

@macjohnny I suggest putting this in the settings. I do not agree that the date should be just a string. In the project that I am developing, the date is transmitted in ISO format (ex 2019-12-26T14:46:56+00:00 - date or 2019-12-26T14:46:56Z - date time) and this standard is used a lot. The option that you offer will result in the parameter having the date format specified by the locale (this is never ISO in js / ts). I came here just because date is generated as a string ...

@Xambey
Copy link
Contributor

Xambey commented Dec 26, 2019

PR for fix #4869

@kisdaniel
Copy link

a date would be something like 2019-10-16, but since Date can also contain time information I think string is a good choice for date properties.
for date-time, it should be string, too, since Date is not serializable in json. We usually override the type to be string, too.
But this should be changed in the code.
@akehir what do you think?

Date is converted to json by JSON.stringify(...) function, when you convert a date from new Date("2020-04-13T00:00:00.000+08:00") to json you will get a 2020-04-12T16:00:00.000Z string, It is converted to UTC, in server side if you receive this string for instance in java spring boot app, LocalDateTIme will get a value of 2020-04-12T16:00:00. In server side it means 2020-04-12T16:00:00.000+08:00, not same as original 2020-04-13T00:00:00.000+08:00.

I think date-type need to be a string in client side to make is possible to skip the timezone conversion of the javascript date class.

@macjohnny
Copy link
Member

this issue is fixed with #5314 and #5266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants