Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[typescript-angular] Added support for Angular v6 (by adding support for RxJS v6) #8155

Merged
merged 5 commits into from
May 11, 2018
Merged

[typescript-angular] Added support for Angular v6 (by adding support for RxJS v6) #8155

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
887afa8
Change RxJS imports, if using Angular >= 6
akehir May 8, 2018
d1328f5
Add testcases for Angular v5 and v6
akehir May 8, 2018
e6bc740
Remove unwanted whitespace in padding, and restore petstore with inte…
akehir May 8, 2018
03b955b
Update according to review of @macjohnny
akehir May 9, 2018
e0a5019
Correct padding for typescript v2 test, and revert typescript with in…
akehir May 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion bin/typescript-angular-petstore-all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,8 @@ java $JAVA_OPTS -jar $executable $ags

echo "Typescript Petstore API client (v5 { Uses latest version of ng-packagr })"
ags="$@ generate -i modules/swagger-codegen/src/test/resources/2_0/petstore.yaml -l typescript-angular -c bin/typescript-petstore-npm.json -o samples/client/petstore/typescript-angular-v5/npm --additional-properties ngVersion=5"
java $JAVA_OPTS -jar $executable $ags
java $JAVA_OPTS -jar $executable $ags

echo "Typescript Petstore API client (v6 { Uses RxJS version 6 })"
ags="$@ generate -i modules/swagger-codegen/src/test/resources/2_0/petstore.yaml -l typescript-angular -c bin/typescript-petstore-npm.json -o samples/client/petstore/typescript-angular-v6/npm --additional-properties ngVersion=6"
java $JAVA_OPTS -jar $executable $ags
31 changes: 31 additions & 0 deletions bin/typescript-angular-v6-petstore-with-npm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/sh

SCRIPT="$0"

while [ -h "$SCRIPT" ] ; do
ls=`ls -ld "$SCRIPT"`
link=`expr "$ls" : '.*-> \(.*\)$'`
if expr "$link" : '/.*' > /dev/null; then
SCRIPT="$link"
else
SCRIPT=`dirname "$SCRIPT"`/"$link"
fi
done

if [ ! -d "${APP_DIR}" ]; then
APP_DIR=`dirname "$SCRIPT"`/..
APP_DIR=`cd "${APP_DIR}"; pwd`
fi

executable="./modules/swagger-codegen-cli/target/swagger-codegen-cli.jar"

if [ ! -f "$executable" ]
then
mvn clean package
fi

# if you've executed sbt assembly previously it will use that instead.
export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties"
ags="$@ generate -i modules/swagger-codegen/src/test/resources/2_0/petstore.yaml -l typescript-angular -c bin/typescript-petstore-npm.json -o samples/client/petstore/typescript-angular-v6/npm --additional-properties ngVersion=6"

java $JAVA_OPTS -jar $executable $ags
2 changes: 1 addition & 1 deletion bin/windows/typescript-angular-petstore-all.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ call .\bin\windows\typescript-angular-v2.bat
call .\bin\windows\typescript-angular-v4-with-npm.bat
call .\bin\windows\typescript-angular-v4.3-with-npm.bat
call .\bin\windows\typescript-angular-v5-with-npm.bat

call .\bin\windows\typescript-angular-v6-with-npm.bat

10 changes: 10 additions & 0 deletions bin/windows/typescript-angular-v6-with-npm.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
set executable=.\modules\swagger-codegen-cli\target\swagger-codegen-cli.jar

If Not Exist %executable% (
mvn clean package
)

REM set JAVA_OPTS=%JAVA_OPTS% -Xmx1024M
set ags=generate -i modules\swagger-codegen\src\test\resources\2_0\petstore.yaml -c bin\typescript-petstore-npm.json -l typescript-angular -o samples\client\petstore\typescript-angular-v6\npm --additional-properties ngVersion=6

java %JAVA_OPTS% -jar %executable% %ags%
1 change: 1 addition & 0 deletions ...er-codegen/src/main/java/io/swagger/codegen/languages/TypeScriptAngularClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public void processOpts() {
additionalProperties.put("injectionToken", ngVersion.atLeast("4.0.0") ? "InjectionToken" : "OpaqueToken");
additionalProperties.put("injectionTokenTyped", ngVersion.atLeast("4.0.0"));
additionalProperties.put("useHttpClient", ngVersion.atLeast("4.3.0"));
additionalProperties.put("useRxJS6", ngVersion.atLeast("6.0.0"));
if (!ngVersion.atLeast("4.3.0")) {
supportingFiles.add(new SupportingFile("rxjs-operators.mustache", getIndexDirectory(), "rxjs-operators.ts"));
}
Expand Down
5 changes: 5 additions & 0 deletions modules/swagger-codegen/src/main/resources/typescript-angular/api.service.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ import { Response, ResponseContentType } from '@angular/http
import { CustomQueryEncoderHelper } from '../encoder';
{{/useHttpClient}}

{{^useRxJS6}}
import { Observable } from 'rxjs/Observable';
{{/useRxJS6}}
{{#useRxJS6}}
import { Observable } from 'rxjs';
{{/useRxJS6}}
{{^useHttpClient}}
import '../rxjs-operators';
Copy link
Contributor

@macjohnny macjohnny May 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the import '../rxjs-operators'; should be moved inside the {{^useRxJS6}}{{/useRxJS6}} block, see my general review comment.

Copy link
Contributor Author

@akehir akehir May 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, if I misunderstood, the rxjs-operators is imported in a {{^useHttpClient}} {{/useHttpClient}} - I (wrongly?) thought, this meant that these imports are only used if angular is older than 4.3, and that after 4.3 they are not imported anymore.

For RxJS 6.1, the operators would need to be imported differently, if they are used - see: https://github.com/ReactiveX/rxjs/blob/master/MIGRATION.md#howto-convert-to-pipe-syntax .

From what I can see, actually, of the operators, throw & catch are not used by the api service, only map is. However I just found out, that at least at one point map is used, which will need to be converted to a .pipe( map( ()=> {}), ) for rxjs 6. Let me update the PR and get back to you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, you are right, I missed that it is in a {{^useHttpClient}}{{/useHttpClient}} block.

you don't need to convert the .map() to .pipe(map()), since it is only used is here: https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/typescript-angular/api.service.mustache#L89

And since this line is only used for Angular <4.3, it should be fine as it is

Copy link
Contributor Author

@akehir akehir May 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I misread. You want to move it to the {{^useRxJS6}}{{/useRxJS6}} block, which makes sense. I'll update the PR.

The .map is only used in a {{^useHttpClient}} {{/useHttpClient}} block, so not relevant to angular 6.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I think we shouldn't move it to the {{^useRxJS6}}{{/useRxJS6}} block, since this would also affect Angular >=4.3 and <6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After moving it, angular 5 test fails =(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's what I thought would happen. so you can simply ignore the initial review comment that stated

the import '../rxjs-operators'; should be moved inside the {{^useRxJS6}}{{/useRxJS6}} block, see my general review comment.

{{/useHttpClient}}
Expand Down
8 changes: 6 additions & 2 deletions modules/swagger-codegen/src/main/resources/typescript-angular/apiInterface.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ import { HttpHeaders } from '@angular/comm
import { Headers } from '@angular/http';
{{/useHttpClient}}

import { Observable } from 'rxjs/Observable';

{{^useRxJS6}}
import { Observable } from 'rxjs/Observable';
{{/useRxJS6}}
{{#useRxJS6}}
import { Observable } from 'rxjs';
{{/useRxJS6}}

{{#imports}}
import { {{classname}} } from '../{{filename}}';
Expand Down
6 changes: 3 additions & 3 deletions modules/swagger-codegen/src/main/resources/typescript-angular/package.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"@angular/compiler": "^{{ngVersion}}",
"core-js": "^2.4.0",
"reflect-metadata": "^0.1.3",
"rxjs": "^5.4.0",
"rxjs": "{{#useRxJS6}}^6.1.0{{/useRxJS6}}{{^useRxJS6}}^5.4.0{{/useRxJS6}}",
"zone.js": "^0.7.6"
},
"devDependencies": {
Expand All @@ -40,9 +40,9 @@
"@angular/platform-browser": "^{{ngVersion}}",{{#useNgPackagr}}
"ng-packagr": {{#useOldNgPackagr}}"^1.6.0"{{/useOldNgPackagr}}{{^useOldNgPackagr}}"^2.4.1"{{/useOldNgPackagr}},{{/useNgPackagr}}
"reflect-metadata": "^0.1.3",
"rxjs": "^5.4.0",
"rxjs": "{{#useRxJS6}}^6.1.0{{/useRxJS6}}{{^useRxJS6}}^5.4.0{{/useRxJS6}}",
"zone.js": "^0.7.6",
"typescript": "^2.1.5"
"typescript": "~2.7.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are requiring angular cli, constraining to "typescript": "^2.1.5" should be fine and more flexible, especially when choosing between angular 5 or 6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, with ^2.1.5, typescript 2.8.3 will be installed during the tests which is not compatible to angular core. I'll change it to >= 2.1.5 < 2.8, would that be more appropriate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this sounds better

}{{#npmRepository}},{{/npmRepository}}
{{#npmRepository}}
"publishConfig": {
Expand Down
2 changes: 2 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,8 @@
<!--<module>samples/client/petstore/typescript-angular-v2/npm</module>-->
<module>samples/client/petstore/typescript-angular-v4/npm</module>
<module>samples/client/petstore/typescript-angular-v4.3/npm</module>
<module>samples/client/petstore/typescript-angular-v5/npm</module>
<module>samples/client/petstore/typescript-angular-v6/npm</module>
<!--<module>samples/client/petstore/bash</module>-->
</modules>
</profile>
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v2/npm/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"reflect-metadata": "^0.1.3",
"rxjs": "^5.4.0",
"zone.js": "^0.7.6",
"typescript": "^2.1.5"
"typescript": "~2.7.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not good, we should not require typescript 2.7here

},
"publishConfig": {
"registry": "https://skimdb.npmjs.com/registry"
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v2/npm/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<artifactId>TSAngular2PestoreTest</artifactId>
<packaging>pom</packaging>
<version>1.0-SNAPSHOT</version>
<name>TS Angular2 Pettore Test</name>
<name>TS Angular2 Petstore Test</name>
<build>
<plugins>
<plugin>
Expand Down
3 changes: 1 addition & 2 deletions samples/client/petstore/typescript-angular-v2/with-interfaces/api/pet.serviceInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
*/
import { Headers } from '@angular/http';

import { Observable } from 'rxjs/Observable';

import { Observable } from 'rxjs/Observable';

import { ApiResponse } from '../model/apiResponse';
import { Pet } from '../model/pet';
Expand Down
3 changes: 1 addition & 2 deletions samples/client/petstore/typescript-angular-v2/with-interfaces/api/store.serviceInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
*/
import { Headers } from '@angular/http';

import { Observable } from 'rxjs/Observable';

import { Observable } from 'rxjs/Observable';

import { Order } from '../model/order';

Expand Down
3 changes: 1 addition & 2 deletions samples/client/petstore/typescript-angular-v2/with-interfaces/api/user.serviceInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
*/
import { Headers } from '@angular/http';

import { Observable } from 'rxjs/Observable';

import { Observable } from 'rxjs/Observable';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you re-generate the v2 sample files? this line should not change...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-generated by using ./bin/typescript-angular-petstore-all.sh . The padding was different in the apiInterface.mustache, I just committed a fix.


import { User } from '../model/user';

Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v2/with-interfaces/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"reflect-metadata": "^0.1.3",
"rxjs": "^5.4.0",
"zone.js": "^0.7.6",
"typescript": "^2.1.5"
"typescript": "~2.1.5"
},
"publishConfig": {
"registry": "https://skimdb.npmjs.com/registry"
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v4.3/npm/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"reflect-metadata": "^0.1.3",
"rxjs": "^5.4.0",
"zone.js": "^0.7.6",
"typescript": "^2.1.5"
"typescript": "~2.7.2"
},
"publishConfig": {
"registry": "https://skimdb.npmjs.com/registry"
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v4.3/npm/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<artifactId>TSAngular43PestoreTest</artifactId>
<packaging>pom</packaging>
<version>1.0-SNAPSHOT</version>
<name>TS Angular4.3 Pettore Test</name>
<name>TS Angular4.3 Petstore Test</name>
<build>
<plugins>
<plugin>
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v4/npm/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"reflect-metadata": "^0.1.3",
"rxjs": "^5.4.0",
"zone.js": "^0.7.6",
"typescript": "^2.1.5"
"typescript": "~2.7.2"
},
"publishConfig": {
"registry": "https://skimdb.npmjs.com/registry"
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/typescript-angular-v4/npm/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<artifactId>TSAngular4PestoreTest</artifactId>
<packaging>pom</packaging>
<version>1.0-SNAPSHOT</version>
<name>TS Angular4 Pettore Test</name>
<name>TS Angular4 Petstore Test</name>
<build>
<plugins>
<plugin>
Expand Down
Loading