swagger-typescript-codegen icon indicating copy to clipboard operation
swagger-typescript-codegen copied to clipboard

date-time format field is generated as string type

Open itamarco opened this issue 4 years ago • 9 comments

Suppose we have a Date query param

@Get()
getNextDate(@Query('date') date: Date) { .. }

A generated swagger parameter definition for query param would be

"parameters": [
	{
		"in": "query",
		"name": "",
		"required": true,
		"format": "date-time",
		"type": "string"
	}
]

But swagger-typescript-codegen would type it as string

GetNextDate(parameters: {
        'date': string,
    } & CommonRequestOptions)

And when try to use it with Date, TS error would be raised Type 'Date' is not assignable to type 'string' e.g.

const date:Date =  new Date();
const res = await cl.GetDate({date: date});

itamarco avatar Mar 11 '20 09:03 itamarco

date-time is a string in the Swagger specs https://swagger.io/docs/specification/data-models/data-types/#string

We should update the Typescript Type based on the format too, then convert that to string before the request.

I wonder if this could introduce a few breaking changes changing the type.

erictuvesson avatar Mar 11 '20 09:03 erictuvesson

P.S. official swagger-codegen is indeed generating Date type from date-format (for typescript-fetch language)

itamarco avatar Mar 11 '20 10:03 itamarco

This code-generator currently only supports Swagger 2.0 right now, so we need to look at the spec for 2.0 here instead. Interestingly enough dates are not mentioned explicitly for 2.0, and looking more at the documentation it only briefly mentions some date examples where they are also passed as strings. So seems like the 2.0 spec doesn't consider dates at all. Not sure if that means we should add support for it ourselves, or fall back to strings.

@itamarco - Are you generating a 2.0 or 3.0 swagger spec in your example?

mtennoe avatar Mar 11 '20 11:03 mtennoe

I'm generating 2.0 swagger spec

itamarco avatar Mar 11 '20 11:03 itamarco

Thanks! Backwards compatibility is important so we could consider a solution that exposes a union type for dates, where the type would be Date | string, and then we could check for which type is being passed in from the caller side. The actual call would contain a string version of the date either way. This would maintain backwards compatibility, while making it easier for callers/consumers as well, where you wouldn't have to manually do that stringification of the date

Thoughts?

mtennoe avatar Mar 11 '20 11:03 mtennoe

Sound good.

By union it with string we lose the date type enforcement but this is a price to pay for backward compatibility :(

Appreciate your fast response!

itamarco avatar Mar 11 '20 12:03 itamarco

Cool! Not sure when a solution can be looked into though (my bandwidth is sadly filled at the moment), but anyone should be able to give it a shot :)

mtennoe avatar Mar 11 '20 12:03 mtennoe

By union it with string we lose the date type enforcement but this is a price to pay for backward compatibility :(

I suppose there could be an option to not have the backward compatibility, I would be interested in something like that.

erictuvesson avatar Mar 11 '20 12:03 erictuvesson

We could consider doing the string union as a backwards compatible stop-gap solution, and then remove the | string part when going to the next major version

mtennoe avatar Mar 11 '20 12:03 mtennoe