swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.9k stars 6.03k forks source link

TypeScript generation forces lower case first character #1926

Closed dicko2 closed 8 years ago

dicko2 commented 8 years ago

To give some background, we use a C# WebAPI service, so we use Pascal casing in our objects. We have both C# cleints and TypeScript Clients that access the API.

When using the codegen to generate a TypeScript client for angularJS, the TypeScript code appears to be forcing the property names to camel case using the second variable in the camelize function on this line: https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractTypeScriptClientCodegen.java#L86

This causing issue for us because our we service returns an object like this:

{
"ObjectId": 203
"ObjectName" : "My Object Name"
}

And the object in TypeScript looks like this:

export interface MyObject {
 objectId: number;
 objectName: string;
}

So the object name is fine, its the properties that are wrong.

So when the javascript runs its failing as the names don't line up.

I think this second parameter in the camelize function should be added as a config option for those that need it compatible with C# services.

I understand that MS recommend camel casing for TypeScript properties (here https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines), but this makes it incompatible with services written in C#, so I think the config option for the language is the best option.

Regards, Joel

wing328 commented 8 years ago

@dicko2 thanks for the feedback. I believe the issue you mentioned is similar to this one: https://github.com/swagger-api/swagger-codegen/issues/1341

I left a comment with a potential way to fix it. Would you have time to test that?

dicko2 commented 8 years ago

Thanks for the prompt response!

After looking at it some further we found this article:

http://fizzylogic.nl/2014/07/30/changing-the-casing-of-json-properties-in-asp-dot-net-web-api/

Then looked at our swagger codegen and autorest output for c# using camel casing in the Json objects from WebAPI.

Both apps in C# output like this for camel casing:

    /// <summary>
    /// Gets or Sets TimeZoneName
    /// </summary>
    [DataMember(Name="timeZoneName", EmitDefaultValue=false)]
    public string TimeZoneName { get; set; }

    /// <summary>
    /// </summary>
    [JsonProperty(PropertyName = "timeZoneName")]
    public string TimeZoneName { get; set; }

So they adjust for the API being in camel casing back to pascal casing in the API, where as the other languages don't seem to do it the other way round out of the box, i think the best approach for language compatibility is to:

Though we could force TypeScript into pascal casing, using the work around you proposed, it feels like swimming upstream after I've seen this alternative :)

Thanks for you help @wing328

wing328 commented 8 years ago

That's one way to do it but as you've pointed out the C# API client needs some modification to translate the JSON property name (which means the solution will impact other existing API clients)

I think it's just a matter of time we encounter a case in which we must use the attribute mapping in TypeScript client to resolve the issue.

(btw, I was confused earlier as my comment aims to address another issue with JSON property name with special characters)

wing328 commented 8 years ago

@dicko2 what about making it a CLI option whether to keep the original name or convert to camelCase or PascalCase for property names?

dicko2 commented 8 years ago

@wing328 i think that's a good idea. I think you will find people that use C# a lot that are used to pascal casing for properties and want to stick to it in their TypeScript and/or JavaScript Clients.

Also you may find some hardcore people out their that are using snake case or something lol. we still have some legacy apps that use(d) snake case.

fehguy commented 8 years ago

Seems to me like whatever the server is describing via swagger.json is what we should generate. There are provisions in the other clients to preserve the property names.

wing328 commented 8 years ago

Submitted https://github.com/swagger-api/swagger-codegen/pull/1965 to add an option to determine the model property naming convention.

wing328 commented 8 years ago

PR merged.

@dicko2 please pull the latest and give it another try.