fix(@angular/cli): Do not use floating promises, we can resolve these… #17652
Conversation
packages/schematics/angular/application/other-files/app.component.spec.ts.template
Outdated
Show resolved
Hide resolved
packages/schematics/angular/e2e/files/src/app.e2e-spec.ts.template
Outdated
Show resolved
Hide resolved
timfrijtersppw
force-pushed
the
fix-promise-handling
branch
from
4573010 to
e948768
Compare
|
Please note that I've never really done an open source pull request before. If there is anything missing please let me know. |
timfrijtersppw
force-pushed
the
fix-promise-handling
branch
from
e948768 to
b6f5bd4
Compare
| beforeEach(async(() => { | ||
| TestBed.configureTestingModule({<% if (routing) { %> | ||
| beforeEach(async () => { | ||
| return TestBed.configureTestingModule({<% if (routing) { %> |
There was a problem hiding this comment.
Is there any point in using async if you are not awaiting the promise?
I would expect either async () => { await TestBed... } or () => { return TestBed... }.
There was a problem hiding this comment.
i'd probably go with await to make it more explicit that it is an asynchronous operation and also slightly simplify future user additions (either before or after the call).
There was a problem hiding this comment.
I think async shouldn't only be used when awaiting, returning the promise also showcases asynchronous behavior.
| @@ -1,11 +1,11 @@ | |||
| import { browser, by, element } from 'protractor'; | |||
|
|
|||
| export class AppPage { | |||
| navigateTo(): Promise<unknown> { | |||
| async navigateTo(): Promise<unknown> { | |||
There was a problem hiding this comment.
FWIW, you can also use PromiseLike<...> here and not have to cast below.
Do you know if there are any downsides to using PromiseLike?
There was a problem hiding this comment.
PromiseLike is mainly useful when dealing with alternative implementations of promises (bluebird, etc.). The only real downside is that it won't be accepted for any consumer that expects a Promise (e.g., function myFunc(p: Promise<unknown>) {}).
There was a problem hiding this comment.
Also, no reason for async here since you are not using await.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In this PR I've fixed some floating promises in the generation of component spec files and app.component.spec.ts.
Without this fix tslint errors occur when generating components when the tslint option 'no-floating-promises' is enabled.
Also see:
https://palantir.github.io/tslint/rules/no-floating-promises/