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

triggering $onChanges for updated one way binding #14378

Closed
jordydejong opened this issue Apr 6, 2016 · 21 comments
Closed

triggering $onChanges for updated one way binding #14378

jordydejong opened this issue Apr 6, 2016 · 21 comments

Comments

@jordydejong
Copy link

I'm really happy with the "new" $onChanges method you can implement in a component's controller. However it only seems to be triggered when the bound variable is overwritten from outside my component, not (for instance) when an item is added to an existing array

It this intended behaviour or a bug? Is there another way of listening to updates to my input bindings, besides doing a $scope.$watch on it?

I'm using Angular 1.5.3

KwintenP has created a helpful plnkr demonstrating this issue
http://stackoverflow.com/questions/36349915/triggering-onchanges-for-updated-one-way-binding

@MetalHexx
Copy link

I am interested to hear about this too. I've been using angular.copy() on the parent component to change the reference in order to trigger a $onChanges event on the child component when the value changes. This smells bad to me, but it works when I need my child component to take action on state change.

@gkalpak
Copy link
Member

gkalpak commented Apr 8, 2016

This is indeed intended. It would be too expensive to check each object deeply for changes. Besides, in most cases, where bindings are used in a template, an "internal" change will automatically update the view.

If you want to account for "deep" changes, you need to manually $watch the object.

Note that the newly introduced lifecycle hooks are trying to stay as close as possible (and reasonable) to their ng2 equivalents. The current behavior matches that of the ng2 OnChanges hook as well.

Closing as this is working as expected and the explanation on the SO answer is quite thorough, but feel free to continue the discussion below.

@gkalpak gkalpak closed this as completed Apr 8, 2016
@KwintenP
Copy link

I've written my SO answer in a proper blogpost: http://blog.kwintenp.com/the-onchanges-lifecycle-hook/ . It also proposes a way to fix the problem using ImmutableJS.

@zbjornson
Copy link
Contributor

I don't see an equivalent to ngDoCheck available in 1.5.x. Some of our bindings are way too big to efficiently copy, so we can't use the immutable technique. We're manually $watch'ing the objects/arrays now, but it would be nice to have a doCheck in 1.5 to ease the upgrade.

@gkalpak
Copy link
Member

gkalpak commented May 13, 2016

Yeah, I've been thinking about adding something equivallent to ChangeDetectionStrategy.OnPush.
PRs are also welcome is someone is feeling adventurous 😃

@KwintenP
Copy link

@zbjornson If you're bindings are to big to efficiently copy, there might be something wrong with the architecture of your application.
IMO, you should only bind objects into dumb components that are used to visualise data. Smart components should do their own data fetching, which removes the need to work with bindings.

@zbjornson
Copy link
Contributor

@KwintenP Thanks, interesting to discuss this. I'm new to Angular2 patterns, but I don't know how you would follow that pattern if you have multiple components relying on the same data. You don't want to duplicate the data, so that seems to preclude having multiple smart components all fetching the same data (and even if they did, how would they know when to refresh their data?). That leads to a single smart component with multiple dumb child components, but those child components need some way to observe inputs, which leads me back to the original problem... Thoughts?

@gkalpak
Copy link
Member

gkalpak commented May 18, 2016

Observables ? (They seem to be the answer to any problem these days 😛 )

Assuming you do need to have a separate copy of the passed in object (which I don't think is the most common case), I wonder how else would you handle the one-to-many situation.

@zbjornson
Copy link
Contributor

Observables would work, sure. Not as tidy as magically watching objects but probably higher performance.

With my initial comment, I envisioned binding to object/array references (not copies) and overriding doCheck with a custom change detector. In our case, we have a particular property that can be checked in the same way ngRepeat's track by works. Another example would be watching just the length of an array.

// (in Angular 1.5)
myMod.component('foo', {
  template: '...',
  bindings: {
    myArr: "<"
  },
  controller: function () {
    this.doCheck = function () {
      // specific logic for checking myArr changes
      // maybe something like what's in ngRepeat's watch, https://github.com/angular/angular.js/blob/fa79eaa816aa27c6d1b3c084b8372f9c17c8d5a3/src/ng/directive/ngRepeat.js#L426
    };
  }
});

Again I'm not versed with Angular 2 and don't know if this is the correct approach, but doCheck is something I'd be interested in back-porting to Angular 1.5. If agreed, I can work on a PR.

Somewhat related, I don't know if Angular internally always checks all bindings or if it's able to check specific ones. If it can check specific ones, then likewise having binding-specific change detectors would be nice (whereas currently there's a single doCheck for all inputs). This could even allow something like:

  bindings: {
    myArr: "< track by _id"
  },

@gkalpak
Copy link
Member

gkalpak commented May 19, 2016

The "track by" feature is interesting. Internally it is possible to implement it (using private APIs, such as $$watchDelegate or maybe interceptorFn).

You could also use an immutable library (e.g. Immutable.js). It might in fact be easier, because you wouldn't have to manually update _id.

I'm not sure the doCheck feature would work in this case, because it would still not see any difference if the collection was the same (by reference).

@gkalpak
Copy link
Member

gkalpak commented May 19, 2016

Of cource, you can have your own "cheap" watchers in your child components, but it is less declarative, more error-prone and requires more boilerplate:

.component('child', {
  bindings: {
    myArr: '<'
  },
  controller: function ChildCtrl($scope) {
    var self = this;
    $scope.$watch(getArrId, doStuffWith); 

    function getArrId() {
      return self.myArr._id;
    }

    function doStuff() {
      // `myArr` has been modified (even if the reference is the same).
      // Do what needs to be done
    }
  }
})

@MetalHexx
Copy link

Correct me if I'm wrong, but as understand it, $scope.watch is not available in angular 2 components. As such, any solution using watches seems counterproductive to the goal of creating components that will be upgradeable to angular 2.

@zbjornson
Copy link
Contributor

zbjornson commented May 19, 2016

Just to clarify, with track by I meant using that to specify a property on each object in an array of objects (e.g. track by _id with binding [{name: 'foo', _id: 1}, {name: 'bar', _id: 2}] would track the _ids on each like ngRepeat does), but it would be awesome to overload it to also track a property if a non-array object is bound (e.g. {name: 'foo', version: 1} could track just version). We can manually do this with scope.$watch as @gkalpak exemplified, but as @MetalHexx said this doesn't get us to something that upgrades to Angular 2 easily. -- Should I propose this feature in a new ticket?

As far as doCheck, the docs say its purpose is to "Detect and act upon changes that Angular can or won't detect on its own. Called every change detection run." That thus sounds like the closest thing to using a custom watchExpression in scope.$watch. I haven't looked at it internally -- am I mistaken in my understanding?

Aside, I totally forgot that objectEquality=true in scope.$watch essentially uses the immutable approach under the hood (via angular.copy).

@gkalpak
Copy link
Member

gkalpak commented May 19, 2016

Aside, I totally forgot that objectEquality=true in scope.$watch essentially uses the immutable approach under the hood (via angular.copy).

Just to be clear, deep-watching+angular.copy is not equivalent to using immutable structures. It might be functionally, but won't be as performant/efficient. Most immutable libraries utilize smart algorithms under the hood so they don't actually copy whole objects (although it might seem like they do).

I had confused doCheck() with markForCheck()+ChangeDetectionStrategy.OnPush. So, doCheck() doesn't seem at all different than scope.$watch(), does it 😁

The track by feature wouldn't be ng2 compatible either, would it ?

@zbjornson
Copy link
Contributor

zbjornson commented May 19, 2016

Cool on the lib optimizations. Need to read more about that...

Yep, doCheck seems quite similar to scope.$watch, at least in capabilities. Is that a sign of support for a backport PR? :)

It would complete the set of available lifecycle hooks for Angular 1.5 components -- ngOnInit, ngOnChanges, ngOnDestroy and ngAfterViewInit/ngAfterContentInit are all in 1.5 already, which basically leaves ngDoCheck.

Not sure on ng2, maybe:

@Component()
class MyComponent {
  @Input('my-arr track by _id')  myArr: [MyObj];
  @Input('my-obj track by version') myObj: MyObj;
}

@gkalpak
Copy link
Member

gkalpak commented May 19, 2016

It might be worth investigating the track by option for ng2. Could you submit a feature request on the ng2 repo ? (Or maybe there is another way to do that already that we have missed - apart for observables/immutables.)

TBH, at this point I would not add a cool feature in ng1 that promotes non-ng2-compatible practices.
Let's find out what is the recommended ng2 way (or get that feature into ng2 first 😃)

Regarding backporting doCheck:

If we decide it makes sense, we need to ensure that its semantics and behavior is as close a (reasonably) possible to ng2 (else it doesn't make sense). Right now there are some inconsistencies between the lifecycle hooks guide and the API docs (which are incomplete anyway), but if anyone feels like diving into the source and finding out exactly what it does (and how it relates to/affects other lifecycle hooks and component behavior), I would be interested to hear what they find out 😃

Then we can decide if it makes sense to backport it.

@zbjornson
Copy link
Contributor

zbjornson commented May 19, 2016

at this point I would add a cool feature in ng1 that promotes non-ng2-compatible practices

I assume that should say "would NOT add"? (If so, I totally agree!) :)

All sounds good, thanks for the discussion. Ticket coming up.

(edit) and I'll look at the doCheck stuff soon.

@gkalpak
Copy link
Member

gkalpak commented May 20, 2016

I assume that should say "would NOT add"?

Correct ! Fixed 😃

@aryanisml
Copy link

Hi I am using angular 1.5.9, and component based approached, There is small requirement to capture old as well as new value in texbox ,I guess $watch angular not recommended to use in 1.5.x. so looking the things and came to know that $onChanges can be used for that, request to plunker example for that so i can proceed further

@blowsie
Copy link

blowsie commented Sep 12, 2017

I am using ngResource and having to manually trigger an update using angular.copy().

   ctrl.campaign.$activate();
   ctrl.campaign = angular.copy(ctrl.campaign);

Should i be triggering $onChanges another way or using $doCheck instead?

@gkalpak
Copy link
Member

gkalpak commented Sep 12, 2017

Should i be triggering $onChanges another way or using $doCheck instead?

@blowsie, it really depends on your requirements. $onChanges() is more performant (as it doesn't have to execute on every $digest (as $doCheck() does). It works best with "immutable" objects, which means you need to create a new instance every time you want to modify it (instead of modifying it "in-place").

You don't need a deep clone (i.e. what angular.copy() returns); a shallow one would do just fine (and can be much cheaper):

$ctrl.campaign = angular.extend({}, $ctrl.campaign);

Note: This approach might not be enough if you are not using plain objects.

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

7 participants