search

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.91k stars 6.03k forks source link

[typescript-angular2] Suggestions for refactoring (baseUrl, url service, npm module) #4971

Open michaeljota opened 7 years ago

michaeljota commented 7 years ago

Description

I want to suggest a couple of change in the architecture distribution of the code generated by the typescript-angular2 version of the CLI. I know most of the changes I'm about to suggest are aesthetics, and you may find them useless in a code generator tool, but still.

Modulize generated code

In Angular way, and in npm way. This means the code generated should be able to be integrated as a regular Angular module, and be published in a repository to be manageable as a npm module, even if someone use npm private modules, can use this as well. This approach would generate more code, but their benefits should support the effort. I think it's important to notice that the code generated by this tool it's not part of the client, but a piece of the server, and though, it should be updated only when the Swagger spec in the server gets updated. The code that the Codegen should generate includes, but by any means I don't it limits to, package.json, tsconfig.json, .npmignore and Angular module file. The package.json should include a postinstall script preconfigured to compile all the ts code to regular, nodeable js, and a Typescript Definition file. The last, should also be added in the package.json. All the dependencies that are needed to build a regular Angular project should be added in the peerDependencies.

The definition file should export only the classes that will be consumed by public, and left inside all other Codegen generated code that are use inside those generated files.

The Angular module should only export the services that would be consumed. This is related to the previous point, but it's the way that Angular manages services.

Use index.ts in folders.

It's purely aesthetic, but I think it's better to request from index.ts than from a file name that's not even same casing than the others files.

Use a service for request the URL of the API.

I think this tool should generate a single file containing the URL of the API that the module would consume. This approach would allow to configure the URL of the API in the module importing. (The way that the module would be imported it's related to the last part of the first point).

basePath in api models should be readonly.

If there is a single service managing all the configuration of the URL, the api models should consume it and initialize a readonly basePath property, with the common URL of that API already prefixed.

Swagger-codegen version

v 2.2.1

Related issues

I think this has related issues, I'm trying to search them all. If you find any, pin me there, and I'll add it here. 👍

Suggest a Fix

I can and am willing to help this, if you find all those changes be reasonable. I know Java, Typescript and Angular (2 and up).

Thanks you.

wing328 commented 7 years ago

@michaeljota thanks for the suggestions.

This approach would generate more code, but their benefits should support the effort.

Sounds good to me. Other generators also generate code that can be published to different repository.

basePath in api models should be readonly.

There's a use case in which developers need to change the base path during runtime for different environments (e.g. QA, development) so I would suggest we keep it configurable.

We just released 2.2.2. Please take a look as it contains many enhancements and bug fixes to TS API clients.

You may want to checkout 2.3.0 branch as well, which contains enhancements with breaking changes to TS API clients.

There are several PRs (change requests) that you may also find relevant to this discussion:

https://github.com/swagger-api/swagger-codegen/pull/4795 https://github.com/swagger-api/swagger-codegen/pull/4766 https://github.com/swagger-api/swagger-codegen/pull/3403

michaeljota commented 7 years ago

There's a use case in which developers need to change the base path during runtime for different environments (e.g. QA, development) so I would suggest we keep it configurable.

Yeah. What I mean it's cover by #3403.

I think this should be the way the generated code should work. For this to be a package by itself, still need a lot a files and configurations. The main idea in this suggestion is to allow the code generated with this tool to be an independent package, both for Angular and for npm. The other suggestions are just adjustment that I think are needed for this to work.

wing328 commented 7 years ago

cc @Vrolijkx @taxpon @sebastianhaas @kenisteward @TiFu

kenisteward commented 7 years ago

This isn't needed.

  1. If you need to insert a baseURL per environemnt you can use APP_INIT in Angular to set that value.

  2. Also you can configure each service as you use it. When you inject it into a component you can in the constructor set the baseURL for the api to whatever you want it to be.

IN my current environment on APP_INIT which is during boostrap I go get a config file from the server which has the env I want to target. Then I set the base URL using a factory based InjectionToken.

sebastianhaas commented 7 years ago

Re Use index.ts in folders, this was removed when Webpack <2 had troubles resolving barrel imports recursively (afaik), but it should be fine to add it again so +1 from me for that