2

I want to perform some operations when input parameters are being changed. Lets say I have a DatePicker component that has a type input variable, and I want to perform some actions with another date variable when type is being changed. How to do that?

export class DatePicker {

    @Input()
    date: Date;

    @Output()
    dateChange = new EventEmitter();

    @Input()
    set type(type: string) {
        if (type === "today") {
            this.date = new Date();
            this.dateChange(this.date); // because of this change change detector will throw error
        }
    }

}

Error: Expression has changed after it was checked.

pleerock
  • 18,322
  • 16
  • 103
  • 128
  • I suggest you make the first `@Input() date: any;` and then process the Input variable – null canvas Jun 26 '16 at 10:02
  • `this.dateChange(this.date);` should be `this.dateChange.emit(this.date);` (missing `emit`) – Günter Zöchbauer Jul 09 '16 at 19:35
  • you've a temporal coupling here that should be avoided. it is required that `date` is set before `type`. Either merge both into a single `date: Date | string` or put them in an object. Even `dateChange` should have a listener already in-place, which sounds like it is better to find a different approach. – André Werlang Jul 09 '16 at 19:37
  • there are lot of use cases when you have to do something when input changes, more less coupled then this example. But you can't do anything and that is a problem. Also [this](https://github.com/angular/angular/issues/7782) discussion related to this issue – pleerock Jul 10 '16 at 09:22

1 Answers1

-3

update

Angular2 causes this error when it looks like change detection itself has side effects that causes model change which usually indicates a bug or a design flaw that causes Angular2 applications to work inefficiently.

To hide such an issue you can just enable prodMode.

A workaround for model changes in lifecycle methods call ChangeDetectorRef.detectChanges() to make it explicit that this model change is intentional

export class DatePicker {

    constructor(private cdRef:ChangeDetectorRef) {}

    @Input()
    date: Date;

    @Output()
    dateChange = new EventEmitter();

    @Input()
    set type(type: string) {
        if (type === "today") {
            this.date = new Date();
            this.dateChange(this.date); 
            this.cdRef.detectChanges();
        }
    }
}

original

You can either use setTimeout() setTimeout() is a sledgehammer method because it causes a change detection cycle for the whole application.

@Input()
set type(type: string) {
    if (type === "today") {
        this.date = new Date();
        setTimeout(() => this.dateChange(this.date)); 
    }
}

This is necessary when type is updated by change detection because Angular2 doesn't like when change detection causes changes.

Another way is to use ngOnChanges() but this is also called by change detection and also needs the setTimeout() Workaround

export class DatePicker implements OnChanges {

    @Input()
    date: Date;

    @Output()
    dateChange = new EventEmitter();

    @Input()
    set type:string;

    ngOnChanges(changes:SimpleChanges) {
      if(changes['type']) {
        if (type === "today") {
            this.date = new Date();
            setTimeout(() => this.dateChange(this.date));
        }
      }
    }
}

The difference between these two approaches is that the first executes the code for every change, the last only for changes caused by bindings.

Günter Zöchbauer
  • 623,577
  • 216
  • 2,003
  • 1,567
  • 1
    using setTimeout looks like a monkey patch. Same monkey patch we were using in angular1 with setTimeout and apply. We need something better – pleerock Jul 03 '16 at 14:45
  • I heard that quite a few times about `setTimeout()`. I don't see an issue with `setTimeout()` as long as other code doesn't depend on a specific time to be passed or not yet have to be passed. All it does is to schedule the change to happen after the current change detection cycle is completed and invoke another change detection after the change. – Günter Zöchbauer Jul 03 '16 at 14:48
  • You can try injecting `private cdRef:ChangeDetectorRef` and call `this.cdRef.detectChanges()` after `this.dateChange(this.date)` – Günter Zöchbauer Jul 03 '16 at 14:54
  • i think it was the first solution I tried when I had this problem, not sure now, need to check, but as I remember it caused errors too – pleerock Jul 03 '16 at 15:35
  • setTimeout can't be a solution to a synchronous task. – André Werlang Jul 09 '16 at 18:34
  • @Werlang what do you mean by that? – Günter Zöchbauer Jul 09 '16 at 19:15
  • I mean introducing asynchronicity in a perfectly synchronous sequence is a bad smell. It is not usual that you don't know why a `setTimeout()` works in such cases. It may work now, but you'll never know for how long. That's why you heard about this quite a few times. – André Werlang Jul 09 '16 at 19:29
  • It has a bad reputation, I know why, but many people who argue it's bad seem not to have any idea why they think it's bad. I think in this case where change detection causes a change it's a good idea to use it. It allows the current change detection to complete before another change happens. This makes it easier to reason about the program and makes it **more** reliable, not less. – Günter Zöchbauer Jul 09 '16 at 19:38
  • I disagree it makes it easier to reason about. An error pops out on my screen, I won't try to just circunvent it. – André Werlang Jul 09 '16 at 19:51
  • Any better ideas for the concrete case? – Günter Zöchbauer Jul 09 '16 at 19:52
  • and as I put as a comment in the question, there's a design issue going on here first and foremost – André Werlang Jul 09 '16 at 19:52
  • I guess you are right about that but the question doesn't provide enough context. – Günter Zöchbauer Jul 09 '16 at 19:55