Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

$onChanges hook is not working as expected in angular 1.5.4 #14433

Closed
bogdanalexe90 opened this issue Apr 14, 2016 · 12 comments
Closed

$onChanges hook is not working as expected in angular 1.5.4 #14433

bogdanalexe90 opened this issue Apr 14, 2016 · 12 comments

Comments

@bogdanalexe90
Copy link

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?
Report a bug

What is the current behavior?

  1. $onChanges is not called every time.
  2. $onChanges is called before $onInit.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).
I'm having a component with a binding defined like this:

{
   obj: '<' @Object
}

1.The $onChanges is called before $onInit hook.

  1. The $onChanges is called when the components inits, it's called for the first change and than is not called for the rest of the changes.
    The property that changes is an object but the changes itself is done on the object reference not on the one of the properties. This works with angular 1.5.3.

What is the expected behavior?

  1. The $onChanges hook should be called every time when there's a change on a one-way binding property.
  2. $onChanges should be called after $onInit

What is the motivation / use case for changing the behavior?
Not compatible with applications that worked on 1.5.3

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.
1.5.4 / Mac OS X 10.11.4

Other information (e.g. stacktraces, related issues, suggestions how to fix)

@gkalpak
Copy link
Member

gkalpak commented Apr 14, 2016

The fact that $onChanges is called before $onInit is intended (it was actually a bug that it wasn't called). The lifecycle hooks, are modeled as close as (reasonably) possible after their ng2 counterparts. And in ng2, $onChanges is ideed called before $onInit.

The other issue that you mention, $onChanges not being called all the times, is weird. IIRC the only other change doen to $onChanges would case it to be called more times, not less (see #14406).

It's impossible to investigate it, though, if we can't reproduce. So, could you please create a runnable demo (e.g. using CodePen, Plnkr etc) ?

@bogdanalexe90
Copy link
Author

So sorry for this one.

  1. Apparently the $onChanges hook is not called if another component throws an error in $onChanges hook.
  2. If the expected behaviour for $onChanges is to be called before $onInit i will update my code accordantly. Since it seems a bit odd because you may want to react with an update and you may depend on a require controller.

@gkalpak
Copy link
Member

gkalpak commented Apr 15, 2016

@bogdanalexe90, nothing to be sorry about 😃

There's nothing inherently better with $onChanges being called before $onInit (AFAIK), it's just that this how it is done in ng2 (and tbh this is what happens under the hood too), so we shouldn't have different behavior in ng1 (imagine having to port a component controller to ng2 and having your hooks applied in different order).

FWIW, the required controllers should be already bound on the controller instance before calling $onChanges for the first time. Did you observe something different ?

Apparently the $onChanges hook is not called if another component throws an error in $onChanges hook.

This seems like a real issue. It shouldn't happen imo. Could you please open a new issue about specifically ?

@bogdanalexe90
Copy link
Author

bogdanalexe90 commented Apr 15, 2016

#14444

Apparently the $onChanges hook is not called if another component throws an error in $onChanges hook.
This seems like a real issue. It shouldn't happen imo. Could you please open a new issue about specifically ?

@adamreisnz
Copy link

adamreisnz commented Jul 7, 2016

@gkalpak I find it rather problematic as well that $onChanges is called before $onInit.

I rely on flags and settings in $onChanges that are initialized in the initialization hook, e.g. $onInit.
If $onChanges is called before $onInit, then the point of an $onInit is kind of moot, as I will have to do initialization in $onChanges instead.

If this is happening in Angular 2 as well, then I think it's a problem there too. But at least in Angular 2 we have constructors for our components, so that can alleviate the problem a bit. However, in Angular 1 we don't have those for our directives, so we have to rely on $onInit being the first method that's called.

How can you have a true initialization method, if it is not the first method that is called?

@gkalpak
Copy link
Member

gkalpak commented Jul 7, 2016

If this is happening in Angular 2 as well, then I think it's a problem there too.

@adambuczynski , it does happen in ng2 as well. I don't see why it is a problem, but if you convince them to change their implementation, we will probably change it in ng1 too 😛

But at least in Angular 2 we have constructors for our components, so that can alleviate the problem a bit.

Ehm...we do have constructors in ng1 too 😟

How can you have a true initialization method, if it is not the first method that is called?

It is not an initialization method (it would be called $init or $initialize then). It is a lifecycle hook that is supposed to be called right after the component/controller has been initialized ($onInit means that initialization just happened).
Been initialized obviously means having required controllers initialized and assigned, having bindings initialized and assigned and having called the $onChanges hook with the first changes (because that's what has happened).
It makes perfect sense to me fwiw 😄

BTW, if you need to, you can detect and skip the first run of $onChanges, using the isFirstChange() function of any of the SimpleChange objects (the values of the changes object passed to $onChanges()).

@adamreisnz
Copy link

adamreisnz commented Jul 7, 2016

Ehm...we do have constructors in ng1 too

@gkalpak While the controller function is technically a constructor, yes, various things are still inaccessible at the time of running (for example form controls). I've run into various timing issues with putting my initialisation code in these "constructors", so I resorted to using $onInit instead which seemed a lot more stable.

It is not an initialization method (it would be called $init or $initialize then). It is a lifecycle hook

We can argue semantics about the name of the function, but that's not the point here. The function is, and will be used by people to do initialization logic. Angular 2's guides recommend and encourage that in numerous locations. Two or three that I could quickly find:

Purpose: Initialize the directive/component after Angular initializes the data-bound input properties (source).

We write an ngOnInit method with our initialization logic inside and leave it to Angular to call it at the right time (source).

The constructor is for simple initializations like wiring constructor parameters to properties. It's not for heavy lifting. We should be able to create a component in a test and not worry that it might do real work — like calling a server! — before we tell it to do so. If not the constructor, something has to call getHeroes. Angular will call it if we implement the Angular ngOnInit Lifecycle Hook. (same page)

From reading that, it most certainly seems to me that this hook is meant to be used for any kind of initialization which is heavier than setting basic scope/component properties.

Anyway, I do agree that it makes sense that the first set of changes to your bound properties be available to use in the $onInit hook, so I guess I'll leave it at that and head off to rewrite some of my code ;)

@gkalpak
Copy link
Member

gkalpak commented Jul 7, 2016

You can't always agree on everything, right 😃 The important thing is to agree on the stuff that matters!

Just to be clear, I didn't imply that you should put your initialization logic in the construtors - I just pointed out that there are constructors in ng1 too. I agree that "your" initialization logic should be placed inside $onInit and that this is expected to run after the framework has done "its" initialization (whatever that means for each framewrok) - which included running $onChanges().

I am sure that not every usecase will be "happy" with any specific order, but hopefully the current implementation makes things easy (or at least not impossible 😛 ) the most common ones 😉

The good thing is that you won't need to rewrite your code when you migrate to ng2 😛

Thx for the conversation btw. Even if you don't reach a definitive agreement, it helps gain better understanding, especially for new concepts that are still in flux.

@adamreisnz
Copy link

Yes I agree. I think it's important for the Angular (or any framework) team to have a good understanding of user`s use cases and scenario's in order to make sure the framework will help users, not fight against them ;)

@TheBranchDriftCatalyst
Copy link

TheBranchDriftCatalyst commented Apr 13, 2017

Ya, this was a major headache for me and would have been easily resolved if there was a warning in the documentation.

@gkalpak
Copy link
Member

gkalpak commented Apr 13, 2017

PRs improving the docs are always welcome 😉

@poshest
Copy link
Contributor

poshest commented May 11, 2017

+1 for $onInit being called before $onChanges. I think that's the way most people would expect it. I also have many use cases for an init function which $onChanges depends on already having been called. I don't have a single use case for the other way around.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants