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.

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

Support .then() in ion-overlay-method-*-should-use-await rule #47

Closed
JesperBalslev opened this issue Aug 8, 2018 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@JesperBalslev
Copy link

When changing overlay components (Loading, Toast, Modal, or Alert ) to v4, which are created asynchronously now, the tslint migration still gives an error when you write it like this:

showAlert() {
  this.alertCtrl.create({
    message: "Hello There",
    subHeader: "I'm a subheader"
  }).then(alert => alert.present());
}

giving this error:

 The create method of overlay controllers now returns a promise.
Please ensure that you are handling this promise correctly.
@JesperBalslev JesperBalslev changed the title When refactoring to overlay promises, an error is still given. When refactoring overlay components, an error is still given. Aug 8, 2018
@JesperBalslev JesperBalslev changed the title When refactoring overlay components, an error is still given. When refactoring overlay components correctly, an error is still given by the migration tool. Aug 8, 2018
@imhoffd imhoffd added the bug Something isn't working label Aug 8, 2018
@imhoffd
Copy link

imhoffd commented Aug 8, 2018

Thanks @JesperBalslev. We should probably just make that a warning, not an error.

I think if you do this, it may not complain:

async showAlert() {
  const alert = await this.alertCtrl.create({
    message: "Hello There",
    subHeader: "I'm a subheader"
  });

  await alert.present();
}

We just need to add logic to check for a .then() call as well

@JesperBalslev
Copy link
Author

Thanks for the reply, that is true. :)
I just prefer to write it the other way... unless there is a reason i shouldn't?

@imhoffd
Copy link

imhoffd commented Aug 9, 2018

No reason, it's a preference! We should handle both ways. 😄

@godenji
Copy link

godenji commented Sep 2, 2018

Linter output is kind of noisy with these false positives...

@imhoffd imhoffd added this to Backlog 🤖 in Tooling 🔧 via automation Sep 3, 2018
Ionitron added a commit that referenced this issue Sep 4, 2018
## [1.5.1](v1.5.0...v1.5.1) (2018-09-04)

### Bug Fixes

* **dependencies:** Update codelyzer to fix template mangling ([#49](#49)) ([630f8b2](630f8b2)), closes [#47](#47)
@imhoffd imhoffd changed the title When refactoring overlay components correctly, an error is still given by the migration tool. Support .then() in ion-overlay-method-*-should-use-await rule Oct 9, 2018
@imhoffd imhoffd moved this from Backlog 🤖 to In progress 🤺 in Tooling 🔧 Oct 9, 2018
Tooling 🔧 automation moved this from In progress 🤺 to Done 🎉 Oct 9, 2018
@Ionitron
Copy link
Collaborator

Ionitron commented Oct 9, 2018

🎉 This issue has been resolved in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Tooling 🔧
  
Done 🎉
Development

No branches or pull requests

4 participants