[Typescript] Reduce generated file size

[michaelaird]

The generated file can be quite large with a lot of repeated code for each operation. I think this could be greatly reduced with a couple of private (generic?) methods that get called with each operation.

[RicoSuter]

Can you post some specific suggestions?

[michaelaird]

This kind of block

jQuery.ajax({
            url: url,
            beforeSend: this.beforeSend,
            type: "delete",
            data: content,
            dataType: "text",
            headers: {
                "Content-Type": "application/json; charset=UTF-8"
            }
        }).done((data, textStatus, xhr) => {
            this.processDeleteAccommodationReservationWithCallbacks(url, xhr, onSuccess, onFail);
        }).fail((xhr) => {
            this.processDeleteAccommodationReservationWithCallbacks(url, xhr, onSuccess, onFail);
        });

gets repeated over and over again. In our existing code we've abstracted that to a method like this

public static MakeRequestGeneric<T>(data: Object, url: string, method: 'GET' | 'PUT' | 'POST' | 'DELETE'): JQueryPromise<T>
        {
            var options: JQueryAjaxSettings = <JQueryAjaxSettings>{};
            options.data = ko.toJSON(data);

            options.dataType = "json";
            options.contentType = "application/json";

            options.type = method;

            var request: JQueryPromise<T> = jQuery.ajax(url, options);
            return request;
        }

[michaelaird]

Parameter validation might also be a good candidate

        var url = this.baseUrl + "/api/OCC/{businessUnitId}/Booking/{bookingId}/AccommodationReservation/{reservationId}/{version}?"; 

        if (businessUnitId === undefined || businessUnitId === null)
            throw new Error("The parameter 'businessUnitId' must be defined.");
        url = url.replace("{businessUnitId}", encodeURIComponent("" + businessUnitId)); 
        if (bookingId === undefined || bookingId === null)
            throw new Error("The parameter 'bookingId' must be defined.");
        url = url.replace("{bookingId}", encodeURIComponent("" + bookingId)); 
        if (reservationId === undefined || reservationId === null)
            throw new Error("The parameter 'reservationId' must be defined.");
        url = url.replace("{reservationId}", encodeURIComponent("" + reservationId)); 
        if (version === undefined || version === null)
            throw new Error("The parameter 'version' must be defined.");
        url = url.replace("{version}", encodeURIComponent("" + version)); 

[RicoSuter]

This is definitely a good idea. However it is very dangerous to change that much on the templates... We should first have some sort of unit test project where we can test the generated output so that we can be sure that nothing breaks...

[michaelaird]

How detailed are you picturing for unit tests? do you want to have an API that actually gets called to verify against? or would validating that the generated typescript compiles properly be enough to start?

[RicoSuter]

There should be an "Integration.WebApi" and an "Integration.Web" project where the clients for the controllers in the Web API are generated into the web project. Then this clients are called against the Web API in some JavaScript unit tests (e.g. Jasmine).

[Guymestef]

Hi, Any news about this topic?

[michaelaird]

@Guymestef I've switched to using TypeWriter to generate TypeScript API clients. I like the flexibility of building my own template.

[RicoSuter]

@michaelaird Just FYI, you can replace all templates with the TemplateDirectory setting... @Guymestef No bigger improvements have been made mostly because we want to have a good balance between maintanability and file size... discussions and PRs are welcome..

[michaelaird]

@RSuter fair enough. We also use some types that don't come through Swagger very well (NodaTime dates and times) so TypeWriter worked out better for us. NSwag is still great!

[RicoSuter]

@michaelaird Yes, Swagger is sometimes limited and thus there may be some problems and more complexity when converting C# => Swagger => TypeScript vs. C# => TypeScript (Typewriter)...

[Guymestef]

Another good candidate would be the process response function that is repeated a lot: In my case already 17 times for my small new project...

    protected processGet(response: Response): Promise<AppContext> {
        const status = response.status;
        let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v, k) => _headers[k] = v); };
        if (status === 200) {
            return response.text().then((_responseText) => {
            let result200: any = null;
            result200 = _responseText === "" ? null : <AppContext>JSON.parse(_responseText, this.jsonParseReviver);
            return result200;
            });
        } else if (status === 401) {
            return response.text().then((_responseText) => {
            return throwException("A server error occurred.", status, _responseText, _headers);
            });
        } else if (status === 403) {
            return response.text().then((_responseText) => {
            return throwException("A server error occurred.", status, _responseText, _headers);
            });
        } else if (status !== 200 && status !== 204) {
            return response.text().then((_responseText) => {
            return throwException("An unexpected server error occurred.", status, _responseText, _headers);
            });
        }
        return Promise.resolve<AppContext>(<any>null);
    }

All these process* functions could be replaced by one put inside a BaseApiClient class like for the getBaseUrl:

    protected processResponse<T>(response: Response): Promise<T> {
        const status = response.status;
        let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v:any, k:any) => _headers[k] = v); };
        if (status === 200) {
            return response.text().then((_responseText) => {
            let result200: any = null;
            result200 = _responseText === "" ? null : <T>JSON.parse(_responseText, this.jsonParseReviver);
            return result200;
            });
        } else if (status === 401) {
            return response.text().then((_responseText) => {
            return throwException("A server error occurred.", status, _responseText, _headers);
            });
        } else if (status === 403) {
            return response.text().then((_responseText) => {
            return throwException("A server error occurred.", status, _responseText, _headers);
            });
        } else if (status !== 200 && status !== 204) {
            return response.text().then((_responseText) => {
            return throwException("An unexpected server error occurred.", status, _responseText, _headers);
            });
        }
        return Promise.resolve<T>(<any>null);
    }

I guess, the only difficulty would be to list all the error codes from swagger.

@RSuter What do you think about this change?

[RicoSuter]

@Guymestef In general a good idea, but keep in mind that we have to always correctly generate this - manually changing it is much simpler :-). Also it's important to keep the templates as simple as possible to make them more maintainable - keep in mind that process* template is also shared for all template types (fetch, angular, etc.).

[RicoSuter]

So how would we do that?

[Guymestef]

For now, I don't know :D I need to look at the code/template first...

But instead of listing error codes from swagger, another possibility would be to just have code 20* => result200 else throwException... If we really want to keep the case "An unexpected server error occurred." we can do a check on code 500? By doing this, the processResponse function wouldn't be a templated one but just a utility function, and so we would only have to update all the template to use it.

Do you know if there are particularities implemented in the process* function, depending on the type of template used?

[Guymestef]

@RSuter I've made a PR, I checked the result with NSwag.Integration.WebAPI but I haven't pushed the generated files. I tested to generate a client for my project with the template Fetch, it seems okay at first view. But I haven't tested everything so a good review, tests and comments are welcomed :)

[Guymestef]

For info, on my project the previously generated file was ~200KB. The new version is ~124KB

[snebjorn]

The large size of the generated file is causing significant IntelliSense slowdown in VSCode.

While not duplicating code as suggested is helpful I think it's just as important to split the code into multiple files. A good start would be to move the models/interfaces into their own files.

I see that DotLiquid supports custom blocks https://github.com/dotliquid/dotliquid/wiki/DotLiquid-for-Developers#create-your-own-tag-blocks which I assume can be used for this.

Jekyll Liquid discussed a solution over at https://github.com/jekyll/jekyll/issues/16

{% for category in categories %}
  {% output categories/{{category}}/index.html %} // <--- output writes the content to the file
    <html>
    {% for post in category.posts %}
      <a href="{{ post.url }}">{{ post.title }}</a>
    {% endfor %}
    </html>
  {% endoutput %}
{% endfor %}

With this approach the template can still be just one file but generate multiple files.