OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
22.01k stars 6.6k forks source link

[REQ][Typescript] Make `TypeScriptClientCodegen` extend from `AbstractTypeScriptClientCodegen ` #10711

Open tiagoblackcode opened 3 years ago

tiagoblackcode commented 3 years ago

Is your feature request related to a problem? Please describe.

TypeScriptClientCodegen is very similar to AbstractTypeScriptClientCodegen aside from some options specific to the typescript generator such as the http framework to use. Most of the codegen handling code is pretty much the same, however there are some fixes that have been brought to AbstractTypeScriptClientCodegen that are not present in TypeScriptClientCodegen such as #8000 (fixed by #9275).

Describe the solution you'd like

Instead of porting the changes into TypeScriptClientCodegen I suggest we make TypeScriptClientCodegen extend from AbstractTypeScriptClientCodegen, adapting the former where needed. The language is the same so most of the codegen handling can be re-used and further improvements can be done in a single place.

Describe alternatives you've considered

Alternatively, we can port the fixes/enhancements made to AbstractTypeScriptClientCodegen into TypeScriptClientCodegen but I think this will make feature/enhancement parity between the typescript generator and the typescript-* generators diverge over time.

Additional context

I've already prepped a branch with this change. All tests are passing. I've tested with some specs of my own and it seems to be generating properly as well. (https://github.com/tiagoblackcode/openapi-generator/commit/88b63f1bf9f633bc71ac9125f3e8d7d766c8461d).

macjohnny commented 3 years ago

cc @TiFu

TiFu commented 3 years ago

Originally, TypeScriptClientCodegen was supposed to eventually replace all of the other generators (as there is even more code duplication in those) and in order to get rid of some of the baggage from these other clients, it was a conscious choice not to inherit from AbstractTypescriptClientCodegen.

However, given the reality of very slow progress on that part and the resulting need to port features between the 2, I am not really opposed to having TSClientCodegen extend AbstractTSClientCodegen. Happy to also have some other people from the committee chime in :)

tiagoblackcode commented 3 years ago

@TiFu, thank you for your feedback. It certainly clarifies the fact there's so much duplicated code between the two.

Since porting features between the two might be cumbersome to maintain mid-term, what do you think of doing this progressively, but keeping the TSClientCodegen part of the process:

  1. Make TSClientCodegen extend AbstractTSClientCodegen and remove duplicated code. This is what I've done in https://github.com/tiagoblackcode/openapi-generator/commit/88b63f1bf9f633bc71ac9125f3e8d7d766c8461d.
  2. Port common features from TS clients into AbstractTSClientCodegen. This will reduce duplicated code and make it easier to have features available for all clients. The main drawback of doing this is that there may have to be introduced some logic to deal with conceptual differences between the clients - nodeJS HTTP library vs browser Fetch API, etc.
  3. The step above will make it more evident what the big differences are between TS clients, at which point it's more sensible to determine if there should only be a single TS Client or not.

At the moment I'm using an annotations-aware backend to generate a single Typescript client that's being used in both browser (React) and node environments. I might be able to shine some light on the biggest differences between the two.

Kind Regards.

TiFu commented 2 years ago

Sounds good to me :)

kesheshyan commented 2 years ago

Hey guys, I wonder if there is any update on this issue? We are facing similar problems where typescript generator produces uncompilable code whereas generators based on AbstractTypeScriptClientCodegen class produce correct output for semantics like AllOf etc.