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][Fetch] Add interfaces option #7831

Merged

Conversation

tsiq-jeremy
Copy link
Contributor

@tsiq-jeremy tsiq-jeremy commented Mar 13, 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.
    @TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny

Description of the PR

Implement generating interfaces option for api resources, closes #7789

This commit introduces another system property for typescript-fetch, called withInterfaces that is default to false. You can activate this flag by:

java -jar modules/swagger-codegen-cli/target/swagger-codegen-cli.jar generate \
-i http://petstore.swagger.io/v2/swagger.json \
-l typescript-fetch -o samples/server/petstore/springboot/typescript-fetch/with-interfaces \
-D withInterfaces=true

If set to true, generated api resource classes will be implemented with their interfaces. This change should not break and change the behavior even though withInterfaces flag set to true.

@@ -0,0 +1,31 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a .cmd file for windows users

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 is this resolved? Are there any more updates I should do for this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tsiq-jeremy the .cmd file was added, so this particular task is resolved.
so far, there are no other change requests, so maybe someone else will also approve this PR and @wing328 will eventually merge it.

* @summary {{&summary}}
{{/summary}}
{{#allParams}}
* @param {{=<% %>=}}{<%%dataType%%>}<%={{ }}=%> {{^required}}[{{/required}}{{paramName}}{{^required}}]{{/required}} {{description}}
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe use triple curly braces, i.e. {{{ and }}}, to prevent potential html encoding of e.g. Array<number> to Array&lt;number&gt; ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some research into this and it looks kind of ugly doing the triple brackets within Custom Delimiters. So you can also just do {<%&datatype%>} or {{& and }} to escape html, so will try implementing this using &

name: string;
/**
*
* @type {Array&lt;string&gt;}
Copy link
Contributor

Choose a reason for hiding this comment

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

would you mind fixing this in the model interface moustache template by using triple curly braces {{{ and ``}}}` (existed before)?

@macjohnny
Copy link
Contributor

@tsiq-jeremy awesome

@wing328 wing328 added this to the v2.4.0 milestone Mar 25, 2018
@wing328 wing328 changed the title Add interfaces option [TypeScript][Fetch] Add interfaces option Mar 25, 2018
@wing328 wing328 merged commit fc7e083 into swagger-api:master Mar 25, 2018
@wing328
Copy link
Contributor

wing328 commented Mar 25, 2018

@tsiq-jeremy thanks for adding the option.

Thanks @macjohnny for reviewing the change.

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.

[typescript/fetch] Implement interfaces for api resources
3 participants