Skip to content

Improve async handling in generated code #18783

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

Closed
apeeters wants to merge 2 commits into from
Closed

Improve async handling in generated code #18783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c35b541
fix(@schematics/angular): Make sure promise returning methods are async
apeeters Sep 14, 2020
a960cc0
fix(@schematics/angular): Fix handling of returned promises
apeeters Sep 14, 2020
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: 3 additions & 3 deletions integration/angular_cli/e2e/src/app.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ describe('workspace-project App', () => {
page = new AppPage();
});

it('should display welcome message', () => {
page.navigateTo();
expect(page.getTitleText()).toEqual('integration-project app is running!');
it('should display welcome message', async () => {
await page.navigateTo();
await expect(await page.getTitleText()).toEqual('integration-project app is running!');
Copy link
Copy Markdown
Member

@clydin clydin Sep 15, 2020

Choose a reason for hiding this comment

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

The outer await should not be needed here and in the other tests.

Copy link
Copy Markdown
Author

@apeeters apeeters Sep 15, 2020

Choose a reason for hiding this comment

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

The tsconfig.json.template file in packages/schematics/angular/e2e/files specifies jasminewd2 in types. The method signatures in these type definitions return Promises. So I believe this change is needed.

Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 Sep 18, 2020

Choose a reason for hiding this comment

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

It returns a promise because it's a generic, therefore if you await the return type is no longer a promise.

declare function expect<T>(actual: ArrayLike<T>): jasmine.ArrayLikeMatchers<T>;

});

afterEach(async () => {
Expand Down
4 changes: 2 additions & 2 deletions integration/angular_cli/e2e/src/app.po.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { browser, by, element } from 'protractor';

export class AppPage {
navigateTo(): Promise<unknown> {
async navigateTo(): Promise<unknown> {
return browser.get(browser.baseUrl) as Promise<unknown>;
}

getTitleText(): Promise<string> {
async getTitleText(): Promise<string> {
return element(by.css('app-root .content span')).getText() as Promise<string>;
}
}
6 changes: 3 additions & 3 deletions packages/angular_devkit/build_angular/test/hello-world-app/e2e/app.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ describe('hello-world-app App', () => {
page = new AppPage();
});

it('should display welcome message', () => {
page.navigateTo();
expect(page.getTitleText()).toEqual('Welcome to app!');
it('should display welcome message', async () => {
await page.navigateTo();
await expect(await page.getTitleText()).toEqual('Welcome to app!');
});
});
4 changes: 2 additions & 2 deletions packages/angular_devkit/build_angular/test/hello-world-app/e2e/app.po.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
import { browser, by, element } from 'protractor';

export class AppPage {
navigateTo(): Promise<unknown> {
async navigateTo(): Promise<unknown> {
return browser.get(browser.baseUrl) as Promise<unknown>;
}

getTitleText(): Promise<string> {
async getTitleText(): Promise<string> {
return element(by.css('app-root h1')).getText() as Promise<string>;
}
}
6 changes: 3 additions & 3 deletions packages/schematics/angular/e2e/files/src/app.e2e-spec.ts.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ describe('workspace-project App', () => {
page = new AppPage();
});

it('should display welcome message', () => {
page.navigateTo();
expect(page.getTitleText()).toEqual('<%= relatedAppName %> app is running!');
it('should display welcome message', async () => {
await page.navigateTo();
await expect(await page.getTitleText()).toEqual('<%= relatedAppName %> app is running!');
});

afterEach(async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/schematics/angular/e2e/files/src/app.po.ts.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { browser, by, element } from 'protractor';

export class AppPage {
navigateTo(): Promise<unknown> {
async navigateTo(): Promise<unknown> {
return browser.get(browser.baseUrl) as Promise<unknown>;
}

getTitleText(): Promise<string> {
async getTitleText(): Promise<string> {
return element(by.css('<%= rootSelector %> .content span')).getText() as Promise<string>;
}
}