[TypeScript] Add Angular v7 support by topce · Pull Request #1297 · OpenAPITools/openapi-generator

topce · 2018-10-23T09:15:20Z

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 and ./bin/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

There is problem when compile in strict mode typescript-angular generator
because type of formParams was not good
let formParams: { append(param: string, value: any): void; }
it should be
let formParams: { append(param: string, value: any): void; } | { append(param: string, value: any): HttpParams; };

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

JohannesHoppe · 2018-10-23T19:56:10Z

Had the same idea. You were faster! 👍

I went for let formParams: { append(param: string, value: any): any; } to suppress error TS1345: An expression of type 'void' cannot be tested for truthiness. I don't understand the union type here, could you explain this?!
TSLINT: I would also recommend to suppress the following new tslint rules:

import-spacing
max-line-length

TSLINT: Please also fix: let httpHeaderAccepts, let useForm, let convertFormParamsToString to use const instead.

topce · 2018-10-24T08:16:26Z

@JohannesHoppe
indeed your solution is good as well

union type I put because formParams
could be of type FormData,HttpParams or URLSearchParams
both FormData and URLSearchParams get method
append(name: string, value: string): void;
and HttpParams get method
append(param: string, value: string): HttpParams;

As in template they use formParams.append('{{baseName}}', <any>element)

that why I put union type
let formParams: { append(param: string, value: any): void; } | { append(param: string, value: any): HttpParams; };

main problem still is in generated code
formParams = formParams.append('fileData', <any>'trt') || formParams;

so if TS conclude that formParams is type of URLSearchParams or FormData
it will complained (compile type error) you can see in playground
because in this case it make more sense to write just
formParams.append('fileData', <any>'trt');

on contrary if TS conclude that formParams is of type HttpParams
it will not complain about
formParams = formParams.append('fileData', <any>'trt') || formParams;

you can see example in TS playground
example

Maybe to see with @macjohnny to refactor code to be more typesafe

@JohannesHoppe
Regarding linting (point 2 and 3)
please create new PR in order not to mix apples and oranges ;-)

JohannesHoppe · 2018-10-24T08:57:37Z

Regarding point 1: I see. Thanks for the detailed explanation! 😄 I appreciate your effort! 👍

Regarding point 2 + 3:
A separate PR could be done, too. No question. But the templates already have tslint-suppressions in place. So I think we should follow that path and keep them green. This is relevant, because not everybody generates an npm-package and directly outputs files into the main project instead --> tslint error count grows. Just a little bit housekeeping. 😄

JohannesHoppe

This will fix my build. Highly appreciated!

topce · 2018-10-24T09:17:41Z

Hi @JohannesHoppe
probably you are right regarding linting ,
I do not lint generated code , but we could stick to some linting rules already defined

I noticed also that append in FormData has this signature
append(name: string, value: string | Blob, fileName?: string): void;

I was playing in playground ;-)
Probably better solution would be :
`
let formParams : HttpParams | FormData | URLSearchParams;

if ( formParams instanceof FormData ) {
formParams.append('fileData', 'trt');
} else if (formParams instanceof URLSearchParams) {
formParams.append('fileData', 'trt');
} else {
formParams= formParams.append('fileData', 'trt') || formParams;
}
`
Maybe I implement this one and fix linting errors.
To see with @macjohnny

Thank you for your comments!

topce · 2018-10-24T12:59:35Z

Hi @JohannesHoppe
I implemented other way ( hopefully better way )
Can you please test if it is still working for you.

Hi @macjohnny
Can you check on your side if it is OK for you ?
If yes maybe @wing328 to merge it ;-)

macjohnny · 2018-10-24T23:01:30Z

@topce thanks for the improvement. I am currently on holiday so I cant check it, but maybe @JohannesHoppe can review the changes?

topce · 2018-10-25T08:27:07Z

@macjohnny no problem enjoy holiday ;-)

wing328 · 2018-10-25T08:38:38Z

+                }
+                {{/useHttpClient}}
+                {{^useHttpClient}}
+                    formParams.append('{{baseName}}', {{paramName}}.join(COLLECTION_FORMATS['{{collectionFormat}}']));


What about removing 4-space to have the code better align?

{{^useHttpClient}} formParams.append('{{baseName}}', {{paramName}}.join(COLLECTION_FORMATS['{{collectionFormat}}'])); {{/useHttpClient}}

wing328 · 2018-10-25T08:38:50Z

+            }
+            {{/useHttpClient}}
+            {{^useHttpClient}}
+                formParams.append('{{baseName}}', <any>{{paramName}});


Same as above: https://github.com/OpenAPITools/openapi-generator/pull/1297/files#r228081801

topce · 2018-10-25T08:53:32Z

thanks @wing328
done

topce · 2018-10-25T13:38:53Z

@JohannesHoppe @wing328
any news ?

wing328 · 2018-10-26T04:04:57Z

@topce the Travis CI tests failed with the following error messages:

BUILD ERROR
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:557:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:560:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:633:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:636:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error: Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:557:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:560:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:633:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
Error at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/.ng_pkg_build/@swagger-angular2-typescript-petstore/ts/api/pet.service.ts:636:13: Cannot invoke an expression whose type lacks a call signature. Type '((param: string, val: string) => void) | ((name: string, value: string | Blob, fileName?: string)...' has no compatible call signatures.
    at new UserError (/home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/node_modules/@angular/tsc-wrapped/src/tsc.js:27:28)
    at check (/home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/node_modules/@angular/tsc-wrapped/src/tsc.js:93:15)
    at Tsc.typeCheck (/home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/node_modules/@angular/tsc-wrapped/src/tsc.js:173:9)
    at /home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/typescript-angular-v4/npm/node_modules/@angular/tsc-wrapped/src/main.js:122:23
    at <anonymous>
npm ERR! code ELIFECYCLE
npm ERR! errno 111
npm ERR! @swagger/[email protected] build: `ng-packagr -p ng-package.json`
npm ERR! Exit status 111
npm ERR! 
npm ERR! Failed at the @swagger/[email protected] build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2018-10-25T09_15_46_101Z-debug.log
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (npm-run-build) on project TSAngular4PestoreClientTests: Command execution failed.: Process exited with an error: 111 (Exit value: 111) -> [Help 1]

Please have a look and let us know if you can't reproduce it locally.

Ref: https://travis-ci.org/OpenAPITools/openapi-generator/builds/446066683?utm_source=github_status&utm_medium=notification

topce · 2018-10-26T08:05:15Z

@wing328
I will check it look like compile time error with previous versions of angular

topce · 2018-10-26T11:15:46Z

try @JohannesHoppe idea
to fix it properly it is complicated and it look like that
TypeScript Clients need to be refactored anyway

JohannesHoppe · 2018-10-26T13:05:33Z

any ftw! 😎

topce · 2018-10-26T13:16:32Z

@wing328 on end of day I took @JohannesHoppe solution

generated code is little bit strange
but it is compatible with previous versions
and most important it works!
As all TypeScript clients would be refactored
maybe in future code would be more type safe
less use of any
can you please merge this PR

wing328 · 2018-10-27T10:52:52Z

@topce thanks for the enhancemet, which has been merged into master.

Tweet to promote Angular v7 support: https://twitter.com/oas_generator/status/1056132688749355014

Reddit: https://www.reddit.com/r/angular/comments/9rtcyb/typescript_angular_client_generator_in/

Hacker News: https://news.ycombinator.com/item?id=18315211

wing328 · 2018-10-31T15:55:14Z

@topce thanks again for the PR, which is included in the v3.3.2 release: https://twitter.com/oas_generator/status/1057649626101112832

topce · 2018-10-31T18:21:56Z

You are welcome! Thank you for great code generator!!

…

On Wed, 31 Oct 2018 at 17:04, William Cheng ***@***.***> wrote: @topce <https://github.com/topce> thanks again for the PR, which is included in the v3.3.2 release: https://twitter.com/oas_generator/status/1057649626101112832 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1297 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABRtsvCGq5l1SzulF0ZEPW9c1v9lgAuks5uqcn8gaJpZM4X1Fwb> .

* fix wrong formParams type * run typescript-angular-petstore-all.bat * more typesafe fix * fix formatting * run typescript-angular-petstore-all.bat * fix compile problem with previous versions of angular * generate code * fix compile problem add <any> * try Johannes Hoppe solution * generate files

topce added 2 commits October 23, 2018 10:58

fix wrong formParams type

71d2c25

run typescript-angular-petstore-all.bat

1b8ec79

JohannesHoppe approved these changes Oct 24, 2018

View reviewed changes

more typesafe fix

056e734

wing328 reviewed Oct 25, 2018

View reviewed changes

topce added 2 commits October 25, 2018 10:48

fix formatting

de0f9ec

run typescript-angular-petstore-all.bat

b786635

wing328 added Client: TypeScript Enhancement: Compatibility labels Oct 26, 2018

topce and others added 5 commits October 26, 2018 10:44

fix compile problem with previous versions of angular

035b522

generate code

2830e1b

fix compile problem add <any>

be57462

try Johannes Hoppe solution

58c7ebb

generate files

6c99633

wing328 merged commit 0a45890 into OpenAPITools:master Oct 27, 2018

wing328 changed the title ~~Fix angular 7 compile problem~~ [TypeScript] Add Angular v7 support Oct 27, 2018

wing328 added this to the 3.3.2 milestone Oct 27, 2018

topce deleted the fix-ng7-compile branch April 23, 2019 12:43

Uh oh!

Conversation

topce commented Oct 23, 2018

PR checklist

Description of the PR

Uh oh!

JohannesHoppe commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

topce commented Oct 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohannesHoppe commented Oct 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohannesHoppe left a comment

Choose a reason for hiding this comment

Uh oh!

topce commented Oct 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

topce commented Oct 24, 2018

Uh oh!

macjohnny commented Oct 24, 2018

Uh oh!

topce commented Oct 25, 2018

Uh oh!

wing328 Oct 25, 2018

Choose a reason for hiding this comment

Uh oh!

wing328 Oct 25, 2018

Choose a reason for hiding this comment

Uh oh!

topce commented Oct 25, 2018

Uh oh!

topce commented Oct 25, 2018

Uh oh!

wing328 commented Oct 26, 2018

Uh oh!

topce commented Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

topce commented Oct 26, 2018

Uh oh!

JohannesHoppe commented Oct 26, 2018

Uh oh!

topce commented Oct 26, 2018

Uh oh!

wing328 commented Oct 27, 2018

Uh oh!

wing328 commented Oct 31, 2018

Uh oh!

topce commented Oct 31, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JohannesHoppe commented Oct 23, 2018 •

edited

Loading

topce commented Oct 24, 2018 •

edited

Loading

JohannesHoppe commented Oct 24, 2018 •

edited

Loading

topce commented Oct 24, 2018 •

edited

Loading

topce commented Oct 26, 2018 •

edited

Loading