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

moment.Duration & format #1496

Closed
jochenjonc opened this issue Jul 31, 2018 · 11 comments
Closed

moment.Duration & format #1496

jochenjonc opened this issue Jul 31, 2018 · 11 comments

Comments

@jochenjonc
Copy link
Contributor

Hi,

I've just tested the changes made to NSwag to handle TimeSpan as moment.Duration in TypeScript (#1460) and there is still a problem in the toJSON function that get's generated.
The following this.myTimeSpan.format('HH:mm:ss') doesn't work, it seems that moment.Duration has no format function. It has been a long and ongoing discussion on the moment GitHub see 463 & 1048.

The solution seems to be to create a moment.Moment and use format on that, it should be the following code: moment.utc(this.myTimeSpan).format('HH:mm:ss') or to use the Moment Duration Format plugin.

Anyway, an other problem is the loss of days & fractional seconds when doing the format back to Json. This can't be solved with the trick via moment.utc, and there the Moment Duration Format is needed.

this.myTimeSpan.format('d.hh:mm:ss.SS')

This should be conform the ASP.NET style time spans.

So I would suggest to change the format in code and to require the usage of the Moment Duration Format plugin. To install the plugin the following npm packages are needed:

npm install moment-duration-format
npm install --save-dev @types/moment-duration-format

And I needed to add the following import in the generated file, just after import * as moment from 'moment';:

import 'moment-duration-format';

I hoped there was an easier way, but after a lot of research it was the best I could come up with.

Regards,

Jochen

@RicoSuter
Copy link
Owner

So with moment-duration-format we only need to change format to .format('d.hh:mm:ss.SS')?

@jochenjonc
Copy link
Contributor Author

Yes that's right.

@RicoSuter
Copy link
Owner

Can you update the MomentJS wiki with your finding and moment-duration-format requirements? That would be great...

https://github.com/RSuter/NSwag/wiki/SwaggerToTypeScriptClientGenerator#datetime-handling

@jochenjonc
Copy link
Contributor Author

I'll have a look at the wiki, but the format you committed isn't correct. The d in front the HH isn't there and I think it is required for TimeSpans greater than 24h.

RicoSuter added a commit to RicoSuter/NJsonSchema that referenced this issue Jul 31, 2018
@RicoSuter
Copy link
Owner

Ok, i think it's fine now..

@RicoSuter
Copy link
Owner

Wondering if we should add this import or you have to manually import it in extension code...

@jochenjonc
Copy link
Contributor Author

In the NSwag code after the following line the extra import for moment-duration-format also needs to be added.
https://github.com/RSuter/NSwag/blob/2f5d7d35bdc07e76de09220d3f517d83eecbc9fc/src/NSwag.CodeGeneration.TypeScript/Templates/File.liquid#L55

But I'm not sure if there is a possibility to include the line only if TimeSpans are being generated? Because otherwise a lot of existing projects that don't use TimeSpan will have to install the moment-duration-format package.

@RicoSuter
Copy link
Owner

But I'm not sure if there is a possibility to include the line only if TimeSpans are being generated? Because otherwise a lot of existing projects that don't use TimeSpan will have to install the moment-duration-format package.

Yep, that was also my concern...

@jochenjonc
Copy link
Contributor Author

I think it isn't possible to import something globally in typescript and thus it has to be part of the file.

@RicoSuter
Copy link
Owner

RicoSuter commented Jul 31, 2018

Reopened until fixed in NSwag, NJS updated and released

@RicoSuter RicoSuter reopened this Jul 31, 2018
@RicoSuter
Copy link
Owner

v11.18.1

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

2 participants