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

async lifecycle hooks - should angular support 'await ngOnInit' or other hooks #17420

Closed
alex321 opened this issue Jun 12, 2017 · 9 comments
Closed

Comments

@alex321
Copy link

alex321 commented Jun 12, 2017

I'm submitting a ... (check one with "x")

[ X ] feature request

Hey, I would be interested whether it makes sense to support adding async in front of the Angular lifecycle hooks e.g.
async ngOnInit() { await ... }

I saw that being used recently and I immediately sounded an alarm as this would prevent the correct ordering of lifecycle hooks. IMHO I can see why the person used it but I am not sure that it makes sense for Angular to provide that.
Would it make sense to put any warnings in the docs? Thanks!

@IgorMinar IgorMinar added the area: core Issues related to the framework runtime label Jul 11, 2017
@chuckjaz chuckjaz added comp: docs and removed area: core Issues related to the framework runtime labels Jul 11, 2017
@chuckjaz
Copy link
Contributor

This seems like an awkward way of executing something in a microtask.

How often are you seeing this?

@alex321
Copy link
Author

alex321 commented Jul 12, 2017

The way people seem to think of it is as a neater way of getting your initial data before the component gets loaded. So instead of using Promise/Observable they revert to async.

It seems an easy mistake to make as you when you put async ngOnInit you completely forget that you don't control the execution of that method. Even this article suggests at the end to use async everywhere you'd use a promise: https://developers.google.com/web/fundamentals/getting-started/primers/async-functions

It's a bit difficult to say how often but it is there on the web
https://labs.encoded.io/2016/12/08/asyncawait-with-angular/

I think the main place people would think of using it is ngOnInit but anything is possible! :)

@chuckjaz
Copy link
Contributor

I am not sure that an explicit warning about this is needed as it doesn't interfere with Angular's lifecycle hooks. It is equivalent to saying something like.

  ngOnInit() {
    Promise.resolved().then(() => ...);
  }

which executes the lambda in its own microtask.

@robwormald
Copy link
Contributor

@alex321 do you see this as something that works more like resolve, where it waits for the promise to return before carrying on with instantiation?

@alex321
Copy link
Author

alex321 commented Jul 14, 2017

@chuckjaz Not quite as it pauses ngOnInit when await is used so it certainly changes the order in which the hooks will finish. Even if that doesn't cause a problem in angular, I think it is counter intuitive.

@robwormald That is how I interpreted my teammate's intention. It looked like another way of using resolve but within a component. I haven't used it myself.

I just wanted to raise it as a question to understand whether it will cause problems now or in the future. I personally don't love it as:

  • I cannot do await ngOnInit even if I wanted to
  • you better make sure that everything else is initialised before you do the await as you'd have uninitialised state
  • you are making all of your requests sequential (if you're making more than one)

I can see now that the hooks are just triggers and that angular doesn't care what happens afterwards but it does feel like a poor coding practice unless we wait ngOnInit to finish.

@chuckjaz
Copy link
Contributor

@alex321 Can you rewrite issue description above to make it clear what you are recommending?

@alex321 alex321 changed the title async lifecycle hooks async lifecycle hooks - should angular support 'await ngOnInit' or other hooks Jul 20, 2017
@alex321
Copy link
Author

alex321 commented Jul 20, 2017

@chuckjaz I am quite happy to close this one to be honest as judging from your answers, it's not a problem for someone to use async ngOnInit and it is just an awkward/not recommended coding practice.

I am not sure whether there is any value in making await ngOnInit as a shortcut for resolvers of sorts.

@chuckjaz
Copy link
Contributor

@alex321 Thanks!

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants