[typescript-node] fixed enum generator for top level enums #2266

christof-vollrath-spring-media · 2019-02-28T15:00:12Z

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, ./bin/openapi3/{LANG}-petstore.sh, ./bin/security/{LANG}-petstore.sh and ./bin/openapi3/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: master~~, 3.4.x, 4.0.x~~. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

…ls#665

…mustache OpenAPITools#665

auto-labeler · 2019-02-28T15:00:17Z

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

...va/org/openapitools/codegen/typescript/typescriptnode/TypescriptNodeEnumIntegrationTest.java

jmini · 2019-03-24T12:41:00Z

jmini · 2019-03-25T07:28:03Z

Dear TypeScript Technical Committee, can you have a look at this PR?

@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @nicokoenig @topce

macjohnny · 2019-04-11T05:25:40Z

modules/openapi-generator/src/main/resources/typescript-node/model.mustache

+export enum {{classname}} {
+    {{#allowableValues}}
+    {{#enumVars}}
+    {{name}} = <any> {{{value}}}{{^-last}},{{/-last}}


is there any particular reason for using <any> here?
it seems to work out without it e.g. here

openapi-generator/modules/openapi-generator/src/main/resources/typescript-fetch/modelEnum.mustache

Lines 6 to 12 in b797662

export enum {{classname}} {

{{#allowableValues}}

{{#enumVars}}

{{{name}}} = {{{value}}}{{^-last}},{{/-last}}

{{/enumVars}}

{{/allowableValues}}

}

I just copied this part from the already existing code for enums inside a request. "" was already used in the first typescript node template from Geoff Brown.
Being not a typescript expert myself I just didn't want to change too much.
And I just asked my typescript college, he thinks is not needed...

macjohnny · 2019-04-11T05:28:26Z

...va/org/openapitools/codegen/typescript/typescriptnode/TypescriptNodeEnumIntegrationTest.java

+import java.util.HashMap;
+import java.util.Map;
+
+public class TypescriptNodeEnumIntegrationTest extends AbstractIntegrationTest {


what does this test do?
to simply compare the generated files it would be better to add a new variation to https://github.com/OpenAPITools/openapi-generator/blob/master/bin/typescript-node-petstore-all.sh

AbstractIntegrationTest are generally disabled at the moment (disabled not by me, and I have not understood why).
To test my implementation, I temporarily activated integration test.
And I am hoping that integration tests will be active in the future.
The petstore example doesn't use enums.
And I agree an example with enums would be create.

alecf · 2019-07-11T16:41:21Z

@christof-vollrath-spring-media ping... I just came across this issue and was excited to see a fix... but it sounds like it still needs work?

christof-vollrath-spring-media · 2019-07-15T08:45:09Z

@christof-vollrath-spring-media ping... I just came across this issue and was excited to see a fix... but it sounds like it still needs work?

For us, everything looked fine with this fix.
At least it's better than what we have at the moment, which is just wrong.

In the end we stopped using the Typescript client because of other issues concerning inheritance for incoming data.
Maybe we will come back to it in the future.

macjohnny · 2019-07-15T09:27:54Z

@christof-vollrath-spring-media thanks for your feedback. which issues concerning inheritance to you refer to?
could you please update with the latest master, so we could merge this one. Also, is the <any> typecast really necessary?

wing328 · 2019-08-10T08:16:41Z

@christof-vollrath-spring-media thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553

This fixes #665 for the consolidated typescript generator. Original fix for typescript-node was in PR #2266, merged as 8417c5b in version 4.1.0.

christof-vollrath-spring-media and others added 3 commits February 26, 2019 18:14

[typescript-node] fixed enum generator for top level enums OpenAPIToo…

7990a73

…ls#665

[typescript-node] fixed enum generator for top level enums in models.…

112f840

…mustache OpenAPITools#665

Merge remote-tracking branch 'upstream/master'

8f24f28

auto-labeler bot added the Client: TypeScript label Feb 28, 2019

wing328 reviewed Mar 7, 2019

View reviewed changes

...va/org/openapitools/codegen/typescript/typescriptnode/TypescriptNodeEnumIntegrationTest.java Outdated Show resolved Hide resolved

wing328 added Feature: Enum Issue: Bug labels Mar 7, 2019

wing328 added this to the 4.0.0 milestone Mar 7, 2019

christof-vollrath-spring-media mentioned this pull request Mar 12, 2019

[typescript] support for enum #665

Closed

jmini added 2 commits March 24, 2019 13:25

Merge remote-tracking branch 'origin/master' into pr2266

204a16b

Remove wrong copyright line

58fa19b

mdaouas mentioned this pull request Apr 4, 2019

[Typescript-Node] - Referencing an enum definition from another definition does not output an enum swagger-api/swagger-codegen#9021

Open

macjohnny reviewed Apr 11, 2019

View reviewed changes

wing328 modified the milestones: 4.0.0, 4.0.1 May 13, 2019

wing328 modified the milestones: 4.0.1, 4.0.2 May 31, 2019

wing328 modified the milestones: 4.0.2, 4.0.3 Jun 20, 2019

wing328 modified the milestones: 4.0.3, 4.1.0 Jul 9, 2019

macjohnny merged commit 8417c5b into OpenAPITools:master Jul 15, 2019

wing328 changed the title ~~[typescript-node] fixed enum generator for top level enums #665~~ [typescript-node] fixed enum generator for top level enums Aug 9, 2019

bodograumann mentioned this pull request Sep 28, 2020

[typescript] Fix generation of enum models #7529

Merged

6 tasks

macjohnny pushed a commit that referenced this pull request Sep 28, 2020

[typescript] Fix generation of enum models (#7529)

54dd185

This fixes #665 for the consolidated typescript generator. Original fix for typescript-node was in PR #2266, merged as 8417c5b in version 4.1.0.

	export enum {{classname}} {
	{{#allowableValues}}
	{{#enumVars}}
	{{{name}}} = {{{value}}}{{^-last}},{{/-last}}
	{{/enumVars}}
	{{/allowableValues}}
	}

Uh oh!

[typescript-node] fixed enum generator for top level enums #2266

[typescript-node] fixed enum generator for top level enums #2266

Uh oh!

Conversation

christof-vollrath-spring-media commented Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Description of the PR

Uh oh!

auto-labeler bot commented Feb 28, 2019

Uh oh!

Uh oh!

jmini commented Mar 24, 2019

Uh oh!

jmini commented Mar 25, 2019

Uh oh!

macjohnny Apr 11, 2019

Choose a reason for hiding this comment

Uh oh!

christof-vollrath-spring-media Jul 15, 2019

Choose a reason for hiding this comment

Uh oh!

macjohnny Apr 11, 2019

Choose a reason for hiding this comment

Uh oh!

christof-vollrath-spring-media Jul 15, 2019

Choose a reason for hiding this comment

Uh oh!

alecf commented Jul 11, 2019

Uh oh!

christof-vollrath-spring-media commented Jul 15, 2019

Uh oh!

macjohnny commented Jul 15, 2019

Uh oh!

wing328 commented Aug 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

christof-vollrath-spring-media commented Feb 28, 2019 •

edited

Loading