Skip to content

[Feature][TypeScript] request param enum as literal unions#7433

Merged
wing328 merged 13 commits intoswagger-api:masterfrom
macjohnny:feature/7365-typescript-request-param-enum
Jan 25, 2018
Merged

[Feature][TypeScript] request param enum as literal unions#7433
wing328 merged 13 commits intoswagger-api:masterfrom
macjohnny:feature/7365-typescript-request-param-enum

Conversation

@macjohnny
Copy link
Copy Markdown
Contributor

@macjohnny macjohnny commented Jan 18, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Allows request params of type enum to be represented as enum in the generated angular client code.

Before:

public findPetsByStatus(status: Array<string>, observe?: 'body', reportProgress?: boolean): Observable<Array<Pet>>;

after:

public findPetsByStatus(status: Array<'available' | 'pending' | 'sold'>, observe?: 'body', reportProgress?: boolean): Observable<Array<Pet>>;

fixes #7365

Replaces #7366

@macjohnny
Copy link
Copy Markdown
Contributor Author

@macjohnny
Copy link
Copy Markdown
Contributor Author

See also the discussion in #7366

any,
Pet,
ApiResponse,
Array&lt;&#39;available&#39; | &#39;pending&#39; | &#39;sold&#39;&gt;,
Copy link
Copy Markdown
Contributor Author

@macjohnny macjohnny Jan 18, 2018

Choose a reason for hiding this comment

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

it seems that this has already been broken before, see the previous lines:

 import {
   Array&lt;string&gt;,		  

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@macjohnny what about fixing it by replacing {{ ... }} with {{{ ... }}} ?

Copy link
Copy Markdown
Contributor Author

@macjohnny macjohnny Jan 20, 2018

Choose a reason for hiding this comment

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

@wing328 it is broken even if the html issue is fixed, since it tries to import Array<string> from the file, but this is a generic type...
I will open a separate issue to have this fixed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see #7465

@@ -1,4 +1,4 @@
## @swagger/jquery-typescript-petstore@0.0.1
## @swagger/angular2-typescript-petstore@0.0.1
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it seems this was introduced at some point before...

@macjohnny
Copy link
Copy Markdown
Contributor Author

Could anyone of the technical comittee please revie this PR?
It is similar to #7366 and just requires your approval again.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Jan 22, 2018

I had a look at this PR and the change (which covers default codegen) looks good to me.

I'll merge this into master if no one has any question by coming Wednesday.

Copy link
Copy Markdown

@svenhuber svenhuber left a comment

Choose a reason for hiding this comment

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

successfully tested for Angular

@wing328 wing328 merged commit 2030513 into swagger-api:master Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature][Angular]: Represent enum request parameters as enum literal unions

3 participants