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

Usage of ChangeDetectionStrategy.OnPush #4746

Closed
rosendi opened this issue Oct 14, 2015 · 14 comments

Comments

Projects
None yet
7 participants
@rosendi
Copy link

commented Oct 14, 2015

I've just read article by @vsavkin "CHANGE DETECTION IN ANGULAR 2" and I'am little bit confused on the usage of ChangeDetectionStrategy.OnPush. As I understood component has to be changed if and only if the some property has been replaced. But in the example below the result is list of Dimitri and Laura divs, although names array hasn't been replaced.

@Component({
  selector: 'app',
  changeDetection: ChangeDetectionStrategy.OnPush
})

@View({
  template: `
    <div *ng-for="#name of names">{{ name.name }}</div>
  `
})

class App {
  @Input() names: { name: string } = { name: "Dimitri" }

  constructor() {
    setTimeout(() => {
      this.models.push({ name: "Laura" });
    }, 1000)
  }
}
@rolandjitsu

This comment has been minimized.

Copy link

commented Oct 15, 2015

this.models is probably this.names. Also @Input() names: { name: string } = { name: "Dimitri" } if names is an array, would be better defined as @Input() names: { name: string }[] = [{ name: "Dimitri" }]. And the array has been modified whenever you do a .push() or things that alter the array, so that would in theory trigger the change detection.

@rosendi

This comment has been minimized.

Copy link
Author

commented Oct 15, 2015

I've tried many variants in order to understand how change detection works, so sorry for small bugs. According to article OnPush strategy forces that component depends only on its input properties, and they are immutable. Therefore names should be immutable, for example Immutable.List<string>. Immutable data cannot be changed once created, push instead return a new immutable collection.

  this.names = this.names.push({ name: "Laura" })

In my previous example I use mutable collection with ChangeDetectionStrategy.OnPush. If I understand it correctly, change detection occurs only if the some property (names in our case) has been replaced, not mutated.

@rosendi

This comment has been minimized.

Copy link
Author

commented Oct 15, 2015

just found a live example from angular source code - http://plnkr.co/edit/GC512b?p=preview.

@rolandjitsu

This comment has been minimized.

Copy link

commented Oct 15, 2015

Yes, but doing this.names = this.names.push({ name: "Laura" }) is actually changing the input, so that would trigger the change detection, but simply doing this.names.push({ name: "Laura" }) should not if the definition says that it will not act on changes to the data as it is expected to be immutable.

@rolandjitsu

This comment has been minimized.

Copy link

commented Oct 15, 2015

In the example you posted this.ref.markForCheck(); manually triggers the change detection. Actually it just lets angular know that when the next event occurs in the current zone, it should trigger a change on the components that are marked for check. At least that is my understanding. You can also see a bit more here, maybe this will be of more use.

@rosendi

This comment has been minimized.

Copy link
Author

commented Oct 19, 2015

@rolandjitsu have u read the article by @vsavkin "CHANGE DETECTION IN ANGULAR 2"?

Here is an example with immutable structures http://plnkr.co/edit/oT8F8m?p=preview. Change detection occurs only if I manually call markForCheck() after data property has been replaced. Maybe I'm wrong, however OnPush strategy should detect input replacement automatically. Please, take a look at the example.

@rolandjitsu

This comment has been minimized.

Copy link

commented Oct 19, 2015

@rosendi yes, I have :) You are right, it should trigger change detection if you replace the input (value). It may be a bug as @mhevery has labeled this, but it may also be expected behaviour. @vsavkin can surely tell us what is going on there.

@rosendi

This comment has been minimized.

Copy link
Author

commented Oct 23, 2015

@rolandjitsu Have a look at the new plunk (http://plnkr.co/edit/9WaxndBHzjT5SrvilRKd?p=preview).

It has two components named Parent and Child. The Parent component is a host component that fils in the data (characters in our case). The Child component has a bound (here is where a binding occurs <child [characters]='characters'>) input property characters. I purposely use mutable data structures to track that object mutation on OnPush strategy doesn’t trigger any change detection. In this scenario OnPush strategy works as expected viz. component has to be changed if and only if some property has been replaced.

The issue still couldn't be closed since in the previous plunk (http://plnkr.co/edit/oT8F8m?p=preview) replacing an object entirely in Child component (Cmp named there) doesn’t trigger change detection, and it might be unexpected behavior.

@vsavkin Could you clear it out?

@vicb

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2015

With the OnPushstrategy, the component will be checked once and the mode will be set to Checked. It will then not be checked before markForCheck() is called. This is what you observe in your first examples.

I think you are expecting a behavior similar as OnPushObserve (ie running the cd only when the property triggers a change event) but this is implemented for Dart only ATM.

We must work on improving the documentation in this area.

Edited on 10/23

@vicb

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2015

@rosendi to answer your last comment: OnPush will only trigger change detection when an input property has been changed as the result of a change detection check.

In your first plunk, you have <child [characters]='characters'></child> so when characters (of Parent) is updated, this will trigger the check for the Child.

In your second plunk there is no way to know that the input has changed (because we do not dirty check the Child unless input properties have changed) and you should call markAsCheckOnce() to explicitly trigger the change detection (from the setTimeoutcallback).

One last thing: if the Childchanges as a result of an event, ng2 would automatically dirty check it..

Hopefully this answers all your questions.

@rosendi

This comment has been minimized.

Copy link
Author

commented Oct 27, 2015

@vicb thank you for your detailed explanation! I close the discussion now until there is no improved documentation.

@rosendi rosendi closed this Oct 27, 2015

@rlegrand

This comment has been minimized.

Copy link

commented Apr 5, 2016

Hi,
Could someone tell me more about CheckOnce and Checked, I don't find any information about that.
Is it only used internally and should we only consider Detached/OnPush/CheckAlways?

Thanks in advance

@ericmartinezr

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2016

@rlegrand read this comment #7311 (comment)

@rlegrand

This comment has been minimized.

Copy link

commented Apr 5, 2016

@ericmartinezr Ok thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.