Skip to content

Fixes #6969: typescript-inversify client compile error, type casting …#6970

Merged
macjohnny merged 2 commits intoOpenAPITools:masterfrom
michalzubkowicz:master
Jul 23, 2020
Merged

Fixes #6969: typescript-inversify client compile error, type casting …#6970
macjohnny merged 2 commits intoOpenAPITools:masterfrom
michalzubkowicz:master

Conversation

@michalzubkowicz
Copy link
Contributor

@michalzubkowicz michalzubkowicz commented Jul 17, 2020

This client was unusable for long time because of typings, which fixes this PR, also I've changed apostrophes to be consistent in whole generator to more widely used (single).

fixes #6969

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if ({{paramName}} !== undefined) {
{{#isDateTime}}
queryParameters.push("{{paramName}}="+encodeURIComponent(<any>{{paramName}}.toISOString()));
queryParameters.push('{{paramName}}='+encodeURIComponent(({{paramName}} as Date).toISOString()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the original type of {{paramName}}? can it be casted to Date without issues?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of paramName normally implements Date interface so this change should be correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the general type for format: date-time in the typescript generators is string, see #5314

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@macjohnny

I think the general type for format: date-time in the typescript generators is string, see #5314

I'm pretty sure that is Date interface here, because I'm using it this generator.

@macjohnny
Copy link
Member

cc @gualtierim

@macjohnny
Copy link
Member

@michalzubkowicz thanks for your PR!
can you please merge the most recent master into your branch?

@michalzubkowicz
Copy link
Contributor Author

@macjohnny

Only Date data type have toISOString method, so casting itself will not be a problem, also I'm using this generator for a while after these fixes.

I'll merge last master

@macjohnny
Copy link
Member

Only Date data type have toISOString method, so casting itself will not be a problem, also I'm using this generator for a while after these fixes

if it is Date, why is casting necessary at all? before, the casting was to any, so I guess it wasn't any or Date, so maybe it was string, but this cannot be directly cast to Date: https://www.typescriptlang.org/play/index.html#code/MYewdgzgLgBARgGwIYC4bQE4EswHMYC8MA5EhACYBmxA3AFB0ICmsAtk7qjACJJROF4yGGR58m9OkA

@michalzubkowicz
Copy link
Contributor Author

Only Date data type have toISOString method, so casting itself will not be a problem, also I'm using this generator for a while after these fixes

if it is Date, why is casting necessary at all? before, the casting was to any, so I guess it wasn't any or Date, so maybe it was string, but this cannot be directly cast to Date: https://www.typescriptlang.org/play/index.html#code/MYewdgzgLgBARgGwIYC4bQE4EswHMYC8MA5EhACYBmxA3AFB0ICmsAtk7qjACJJROF4yGGR58m9OkA

You are right, this "cast" problably is not necessary, proper type should be introduced earlier, and for sure it's Date not string , because string doesn't have toISOString method. Just changed this type here to ease understanding why toISOString method is used.

@macjohnny macjohnny merged commit 1bfd86a into OpenAPITools:master Jul 23, 2020
@wing328 wing328 added this to the 5.0.0 milestone Aug 9, 2020
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

Successfully merging this pull request may close these issues.

Typescript-inversify generator is unusable

4 participants

Comments