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 4 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.1.5 <2.8"
}{{#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.1.5 <2.8"
},
"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
18 changes: 9 additions & 9 deletions samples/client/petstore/typescript-angular-v2/with-interfaces/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## @swagger/[email protected]
## @

### Building

Expand All @@ -19,7 +19,7 @@ Navigate to the folder of your consuming project and run one of next commands.
_published:_

```
npm install @swagger/[email protected] --save
npm install @ --save
```

_without publishing (not recommended):_
Expand All @@ -37,7 +37,7 @@ npm link

In your project:
```
npm link @swagger/angular2-typescript-petstore
npm link
```

__Note for Windows users:__ The Angular CLI has troubles to use linked npm packages.
Expand All @@ -52,7 +52,7 @@ In your Angular project:

```
// without configuring providers
import { ApiModule } from '@swagger/angular2-typescript-petstore';
import { ApiModule } from '';
Copy link
Contributor

Choose a reason for hiding this comment

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

why does the package-name disappear here?

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.

I'm not sure about this one, I believe the package name should not disappear - but I was unfortunately not proficient enough to find the root cause of this. This behaviour is probably not caused by my pull request, though.

The same happens, if I run ./bin/typescript-angular-petstore-all.sh on swagger-codegen/master. Was it incorrect to generate all angular examples with my updated .mustache templates?

Notabene, the same happened here: https://github.com/swagger-api/swagger-codegen/pull/8102/files#diff-ad35a6b2374dd58b5781c610802426c4

Copy link
Contributor

Choose a reason for hiding this comment

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

@akehir I remember that the package name appears and disappears frequently, so I think it is a problem somewhere else, certainly not cause by your PR. If I generate the samples on Windows, the package name appears. Maybe this is the difference?


import { HttpModule } from '@angular/http';

Expand All @@ -70,7 +70,7 @@ export class AppModule {}

```
// configuring providers
import { ApiModule, Configuration, ConfigurationParameters } from '@swagger/angular2-typescript-petstore';
import { ApiModule, Configuration, ConfigurationParameters } from '';

export function apiConfigFactory (): Configuration => {
const params: ConfigurationParameters = {
Expand All @@ -89,7 +89,7 @@ export class AppModule {}
```

```
import { DefaultApi } from '@swagger/angular2-typescript-petstore';
import { DefaultApi } from '';

export class AppComponent {
constructor(private apiGateway: DefaultApi) { }
Expand Down Expand Up @@ -126,7 +126,7 @@ export class AppModule {
If different than the generated base path, during app bootstrap, you can provide the base path to your service.

```
import { BASE_PATH } from '@swagger/angular2-typescript-petstore';
import { BASE_PATH } from '';

bootstrap(AppComponent, [
{ provide: BASE_PATH, useValue: 'https://your-web-service.com' },
Expand All @@ -135,7 +135,7 @@ bootstrap(AppComponent, [
or

```
import { BASE_PATH } from '@swagger/angular2-typescript-petstore';
import { BASE_PATH } from '';

@NgModule({
imports: [],
Expand All @@ -159,7 +159,7 @@ export const environment = {

In the src/app/app.module.ts:
```
import { BASE_PATH } from '@swagger/angular2-typescript-petstore';
import { BASE_PATH } from '';
import { environment } from '../environments/environment';

@NgModule({
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
Loading