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

EXCEPTION: Expression has changed after it was checked. #6005

Closed
tandu opened this Issue Dec 18, 2015 · 145 comments

Comments

Projects
None yet
@tandu

tandu commented Dec 18, 2015

What's wrong here? http://plnkr.co/edit/nm8OkrpZCIp4cvA6TbpO?p=preview

@AerisG222

This comment has been minimized.

AerisG222 commented Dec 18, 2015

I have been facing a similar issue, and appears to be due to how a child is initialized, and how a parent might update the child as part of its initialization process. I have a similar sample with an ugly workaround using zone: http://plnkr.co/edit/GI45u805pnFUtFeORnxn?p=preview, with the workaround looking like the following:

constructor(private _zone : NgZone) { }

ngAfterViewInit() : void {
this._zone.overrideOnTurnDone(() => {
this.child.title = 'title from parent';
});
}

@drew-moore

This comment has been minimized.

Contributor

drew-moore commented Dec 18, 2015

This is not a bug, it's a feature of dev mode working as intended. Calling enableProdMode( ) - see updated plunk when bootstrapping the app prevents the exception from being thrown.

That said, it's being thrown for good reason: In short, after every round of change detection, dev mode immediately performs a second round to verify that no bindings have changed since the end of the first, as this would indicate that changes are being caused by change detection itself.

In your plunk, you have the binding [attr.spinner]=isLoading, and isLoading changes as a result of your call to this.load(), which occurs when ngAfterViewInit is fired, which happens as a part of the initial change detection turn. That in itself isn't problematic though - the problem is that this.load() changes a binding but does not trigger a new round of change detection, which means that this binding will not be updated until some future round of change detection.

@tandu

This comment has been minimized.

tandu commented Dec 18, 2015

I need to setup my component in afterViewInit because I have ChildView that is initialized here. What is the correct way to make that change to isLoading? Wrap call to this.load by setTimeout?

@drew-moore

This comment has been minimized.

Contributor

drew-moore commented Dec 18, 2015

You just need to do something, anything, that triggers another round of change detection during that method - emit an event, whatever. Wrapping it in a timeout (queue flashback to ng1 :-P) is one way that'd work, but feels super messy to me in ng2.

@tandu

This comment has been minimized.

tandu commented Dec 18, 2015

It doesn't look user friendly. I think of that method as of the place where I can complete my component initialization when its children are defined and now it turns out that it has some custom rules. I think you should internally force new change detection round so that it is hidden inside ng2 from the user.

@robwormald

This comment has been minimized.

Member

robwormald commented Dec 19, 2015

could you post a more representative sample of what you're trying to achieve here? if its your image spinner, you'd be better using a standard binding.

@robwormald

This comment has been minimized.

Member

robwormald commented Dec 19, 2015

@AerisG222 ditto for you. is there any reason you can't use a simple binding to achieve this? overriding zone behavior is typically not going to be recommended.

@AerisG222

This comment has been minimized.

AerisG222 commented Dec 19, 2015

@robwormald Yea, in that case I could do the property binding. The real case I have also involves another property which is a bit more complicated - an array of custom objects, but that too certainly could be set via property binding.

I suppose if there was no such thing as @ViewChild, I wouldn't care too much, as this would be the only reasonable way to pass the information down. However, I was very excited to see @ViewChild so that I could work off of that to component references in code. This simplifies the markup and allows more control via code, which is additionally valuable with typescript as there is more tooling support (like intellisense and compile time checking). It also simplifies the container component as it does not have to track member variables which are intended only to be used as state for the child.

Another minor point is when looking through an application using the property binding syntax, you have to inspect the template to see who is ultimately consuming the data. This would be more obvious when wired via code.

@tandu

This comment has been minimized.

tandu commented Dec 19, 2015

I have cleaned the code to show only what matters for the example. The idea is to show spinner only while image is loading and then remove spinner and show image. Image is loaded only when element is visible on screen.

inflate.ts

import {Component, Input, Renderer, ElementRef, AfterViewInit, ViewChild} from 'angular2/core';

@Component({
  selector: '[inflate]',
  templateUrl: './inflate.html',
  styleUrls: ['./inflate.css']
})
export class Inflate implements AfterViewInit {
  @Input('inflate') url: string;
  @ViewChild('holder') holder: ElementRef;
  isLoaded = false;
  isLoading = false;

  constructor(private renderer: Renderer) {}

  ngAfterViewInit() {
    setTimeout(_=> this.inflate()); // BUGFIX: https://github.com/angular/angular/issues/6005#issuecomment-165911194
  }

  inflate() {
    let bounds = <ClientRect>this.holder.nativeElement.getBoundingClientRect();
    if(bounds.bottom > 0 && bounds.top < window.innerHeight) {
      this.isLoading = true;
      let img = new Image();
      img.src = this.url;
      img.onload = _=> {
        this.isLoaded = true;
        this.isLoading = false;
        this.renderer.setElementStyle(this.holder,
          'background-image', 'url("' + this.url + '")');
      };
    }
  }
}

inflate.html

<image-holder #holder></image-holder>
<spinner *ngIf="isLoading"></spinner>
@tandu

This comment has been minimized.

tandu commented Dec 19, 2015

I need to keep image not in host because while it fades-in with animation on load complete it shouldn't affect host background opacity.

@svi3c

This comment has been minimized.

svi3c commented Dec 19, 2015

I've got a potentially related problem with a nested directive.
http://plnkr.co/edit/myX2Alx9jie2q0FDIME7?p=preview
Here I'm binding to ngStyle in the progressBar.ts.
I'm logging the styles object before returning to prove that they are equal.
I also get the ExpressionChangedAfterItHasBeenCheckedException .

@pkozlowski-opensource

This comment has been minimized.

Member

pkozlowski-opensource commented Dec 19, 2015

@svi3c you are actually returning a different object. Each function call will return a new object instance that is "equal" to the previous one in terms of keys / values but not in terms of instances / references.

There are number of ways of dealing with this situation:

You might want to check https://github.com/ng-bootstrap/core/blob/master/src/progressbar/progressbar.ts as another example of a progressbar directive for ng2 (using bootstrap's HTML / CSS)

@svi3c

This comment has been minimized.

svi3c commented Dec 19, 2015

@pkozlowski-opensource Thank you very much!
I only read https://angular.io/docs/ts/latest/guide/template-syntax.html#!#ngstyle and I did not stumble upon change detection strategies yet. That's great! :)

@pcroc

This comment has been minimized.

pcroc commented Jan 4, 2016

@drew-moore:

You just need to do something, anything, that triggers another round of change detection during that method - emit an event, whatever.

Can you advise what is the correct and simplest way to re-trigger change detection within ngAfterViewInit - assuming all that is needed is a property to be updated. ApplicationRef.tick() doesn't work here as that causes 'ApplicationRef.tick is called recursively' exception; emitting an event just to trigger a change detection feels wrong (and didn't work for me), and so does setTimeout.

Based on the comments here it seems to be a fairly common requirement to update a bound property in ngAfterViewInit (due to a dependency on a child component), so Angular 2 seems to be missing a simple way to do this.

If updating a bound property in ngAfterViewInit is the wrong thing to be doing, what is the alternative?

@Birowsky

This comment has been minimized.

Birowsky commented Jan 30, 2016

Here's a quick one:

Challenge:
Set the dom element's height (via a certain formula) based on it's rendered width.

Expectation:
When element get's rendered, get the width and based on it, calculate it's height.

Reality:
setTimeout?!

  ngAfterViewInit() {
    window.setTimeout(() =>
      this.elementHeight = (this.nativeElement.offsetWidth * this.randomImage.dim.h) / this.randomImage.dim.w
    )
  }
@zoechi

This comment has been minimized.

Contributor

zoechi commented Jan 30, 2016

@Birowsky Plunker?

@stefankorun

This comment has been minimized.

stefankorun commented Jan 30, 2016

@zoechi Here is an example: http://plnkr.co/edit/MML5uHEFQdL0k0TA5HtY

You can toggle the setTimeout in app/app.component.ts#21 in the ngAfterViewInit lifecycle hook.

@bennadel

This comment has been minimized.

bennadel commented Jan 31, 2016

I have also run into something similar when trying to implement the writeValue( newValue ) method of a custom ngModel valueAccessor. When I try to apply the newValue to the component, it tells me the value has changed (in a bad way). Like you all have found, wrapping it in a setTimeout() "fixes" it; but, not in a good way.

@bryanforbes

This comment has been minimized.

bryanforbes commented Feb 1, 2016

I have another example. It's quite a bit simpler in that a @HostBinding() is set up and is informed by a QueryList.

@Necroskillz

This comment has been minimized.

Necroskillz commented Feb 2, 2016

Same as my issue #5950.

@drewlio

This comment has been minimized.

drewlio commented Feb 3, 2016

Some recommendations here on how to do this cleanly.
http://stackoverflow.com/questions/34827334/triggering-angular2-change-detection-manually

However, I'm still wrapping in setTimeout because I couldn't get any of them to do what I need. It looks like I could ChangeDetectorRef.detectChanges() at the end of a callback and have trigger the CD.... Am I missing something? Maybe it'll help one of you guys.

@ElsewhereGames

This comment has been minimized.

ElsewhereGames commented Feb 5, 2016

I am using the HTML5 media source extensions to create an audio context based on a video element, so use of @ViewChild is required. Based on the audio context I am trying to display some info, like number of channels. However, I ran into this issue as well. What is the right way to bypass this?

@zoechi

This comment has been minimized.

Contributor

zoechi commented Feb 5, 2016

@ElsewhereGames Hard to tell without seeing some actual code. Can you create a Plunker

@ElsewhereGames

This comment has been minimized.

ElsewhereGames commented Feb 8, 2016

@zoechi I don't have an actual bug to report, just stating that not accessing the DOM directly, while desirable, it not always something that can be avoided (in response to @robwormald comment). For the actualy API, see the docs on createMediaElementSource.

@bennadel

This comment has been minimized.

bennadel commented Mar 4, 2016

I posted back in January with an ngModel problem. Now, I'm back again with ViewChildren problem. However, unlike some of these other examples, I'm not actually trying to do anything in my demo. Meaning, my controllers have zero logic - it's all meta data and view bindings. As such, I am not sure where I even have wiggle room to mess things up:

http://www.bennadel.com/blog/3040-i-have-a-fundamental-misunderstanding-of-change-detection-in-angular-2-beta-8.htm

@diegogvieira

This comment has been minimized.

diegogvieira commented Jul 17, 2017

@mlc-mlapis I don't want to be welcome where people can't read and try to bullshit where should not.

@mlc-mlapis

This comment has been minimized.

mlc-mlapis commented Jul 17, 2017

@diegogarciaa ... I just thought that you personally have a problem and it looks that it is not true now. And the reality is also that the most of the problems with Expression has changed after it was checked ... are not bugs but just not right understanding how CD works.

@diegogvieira

This comment has been minimized.

diegogvieira commented Jul 17, 2017

@mlc-mlapis ok... let's understand

http://plnkr.co/edit/nm8OkrpZCIp4cvA6TbpO?p=preview

In this plunk I can understand the variable was bound and checked when ngAfterViewInit was called, therefore when it receives a new value, and do not trigger a new check round, the exceptions is thrown

But for example, If I do have a view container ref... and I use for example:

this.service.GetData().subscribe(
response => this.component.instance.dataModel = response
);

Considere that dataModel is being used as {{ dataModel.Field }} in my html

this.component is a dynamic component being loaded at certain events, how one could avoid the error? Which would be the correct way to pass the data to my component before the lifecicle hooks of change detection

@mlc-mlapis

This comment has been minimized.

mlc-mlapis commented Jul 17, 2017

@diegogarciaa ... yes, this is a typical example where async and sync operation are mixed and meet together in one CD and its confirmation in dev mode. I will not repeat the explanation from the post https://hackernoon.com/everything-you-need-to-know-about-the-expressionchangedafterithasbeencheckederror-error-e3fd9ce7dbb4, so maybe try to read it first.

And look also to https://hackernoon.com/here-is-how-to-get-viewcontainerref-before-viewchild-query-is-evaluated-f649e51315fb.

@diegogvieira

This comment has been minimized.

diegogvieira commented Jul 17, 2017

So the article basically says that I must manually call a round of change detection...

Let's be honest I'm very upset with current status... even the minimal things in angular are so boring and tiring... I can't even breath. FUck you

@mlc-mlapis

This comment has been minimized.

mlc-mlapis commented Jul 17, 2017

@diegogarciaa ... no, it is just an auxiliary solution. You can also use promise.then or use setTimeout ... but the recommended solution in this case is probably a shared service so you will not update directly the property of dynamic component cross the CD cycle and its confirmation.

Or just create an instance of the dynamic component in the moment when you will have the data. So your subscription will invoke the creation of the dynamic component instance.

But what is important -> the primary logic why the error happens -> because unidirectional changes should flow always from top to bottom through the whole tree and after one CD cycle there is a confirmation of the status that unidirectional principle was respected.

@diegogvieira

This comment has been minimized.

diegogvieira commented Jul 17, 2017

This "fuck you" was not personal... just venting my frustations.

We use a directive schema to handle our components. We built a helper that loads a component into a component view reference, and as we understand, the data must be avaliable when a component is being loaded

protected loadPartialView(viewName: string, target: ViewContainerRef) : ComponentRef<any> {
        const factories = Array.from(this.resolver['_factories'].keys());
        const factoryClass = <Type<any>>factories.find((x: any) => x.name === viewName);
        const factory = this.resolver.resolveComponentFactory(factoryClass);
        return target.createComponent(factory);
}

In the example above: I would like to do something like this:

return target.createComponent(factory, dataSource);

Where my dataSource would be avaliable at constructor time

Like redux, that uses a object to handle datasource passing through components, we are thinking about to implement an injectable component that handles data for us, so we can get it before the lifecicle hooks.

@mlc-mlapis

This comment has been minimized.

mlc-mlapis commented Jul 17, 2017

diegogarciaa

I don't think that data must be available before ... a dynamic component is still a component with @Input() and @Output() but the question is when you will change the data.

... the data must be avaliable when a component is being loaded ...

I suppose that you need to use also components from lazy loaded modules so I use a map directly in a module to have a possibility to access components using just string names.

export default class LazyLoadedModule {
	cmps: Map<{}, {}> = new Map();
	constructor() {
		this.cmps.set('first-component', FirstComponent);
		...
	}
}

What return target.createComponent(factory, dataSource); should mean? Because actual API is createComponent(componentFactory, index, injector, projectableNodes, ngModule).

And maybe the new @angular/cdk module would be interesting for you. Just intro here: https://medium.com/@caroso1222/a-first-look-into-the-angular-cdk-67e68807ed9b.

@jfcere

This comment has been minimized.

jfcere commented Sep 13, 2017

For what it worth and if it can help anybody, instead of using setTimeout and as using ChangeDetectorRef.detectChanges() didn't work for me, I ended up using NgZone.onStable and execute my code by subscribing once on the EventEmitter ...

constructor(
  private zone: NgZone,
) { }

ngAfterViewInit() {
  this.zone.onStable.first().subscribe(() => {
    // your code here
  });
}

I am not fully aware of the consequences of doing this but can it be worst than using setTimeout? If anybody has any comment on that matter it would be really welcome!

@finion

This comment has been minimized.

finion commented Sep 25, 2017

@AerisG222 solution work for me

constructor(private _changeDetectionRef : ChangeDetectorRef) { }

ngAfterViewChecked() : void {
    // your code
    this._changeDetectionRef.detectChanges();
}

@gdbaskara

This comment has been minimized.

gdbaskara commented Oct 3, 2017

this is the html component :

<tr *ngFor="let notes of lastData.note | reverse; let i = index">
   <td>{{ lastData.note[i].date | date : 'dd.MM.yyyy'}} | {{ lastData.note[i].date | date: 'HH:mm' }}</td>
   <td>:</td>
   <td>#BFL15_200817 | {{ lastData.status.name }} by {{lastData.note[i].admin}}</td>
</tr>
I got the error if the data in the array is more than one, Any solution?
@simeyla

This comment has been minimized.

simeyla commented Jul 16, 2018

This is certainly a complex issue, but for some cases (especially when you are modifying properties on a ContentChild) it can be solved by moving the logic to ngAfterContentInit() instead of ngAfterViewInit() or ngOnInit()

@partyka1

This comment has been minimized.

partyka1 commented Aug 2, 2018

It's amazing how simple things can cause so much confusion in angular

@jakeNiemiec

This comment has been minimized.

jakeNiemiec commented Aug 2, 2018

Don't worry, everyone else has noticed as well 😕
image

@intergleam

This comment has been minimized.

intergleam commented Aug 2, 2018

@jakeNiemiec lame job, troll, no one is searching Angularjs anymore
https://trends.google.com/trends/explore?date=all&q=Angular%20tutorial,React%20tutorial
image

@jakeNiemiec

This comment has been minimized.

jakeNiemiec commented Aug 2, 2018

Yes, I would expect that more people would need to google "angular tutorial" out of desperation, hence that initial comment on "confusion".

But don't worry, we can check npm for exact usage stats: http://www.npmtrends.com/react-vs-@angular/cli-vs-vue-vs-@angular/core
image
image

Just compare the amount of issues. I would really like to see angular get better, but I suspect that Vue.js will pass it in the distant future. Having programmed in all 3, I highly recommend Vue to angular devs. It's like angular without the boilerplate.

@intergleam

This comment has been minimized.

intergleam commented Aug 2, 2018

ok answer accepted, I am perfectly fine with those stats.
I imagine there are more people using PHP in India that Java in the whole world. One would be an idiot to use PHP for a large scale enterprise app, so you should use your own brain sometimes, life will be better

@mlc-mlapis

This comment has been minimized.

mlc-mlapis commented Aug 2, 2018

@jakeNiemiec ... 50% of issues being created is mistakenly placed there ... they are support / knowledge issues in fact and should not be there at all. The fun is that the most of the required topics is documented very well in the official docs + the answers are based on the knowledge of JavaScript. But you know, people don't read the docs deep enough.

@simeyla

This comment has been minimized.

simeyla commented Aug 2, 2018

@HostBinding can be a confusing source of this error.

Consider a simple binding on a component, such as @HostBinding('style.background').

That property is checked by the **parent **component's change detection as if it'owned' it. From our perspective we want think of it as the child component 'owning' that property.

So seems like there should be a way for a component to exclude properties for checking in this case? Or is it more complicated than that?

I had to have my parent component run detectChanges (or add an extra div in the child) to avoid the error in the case where the child component was simply changing its own background color (and never doing it anywhere else).

More complete description: https://stackoverflow.com/questions/43375532/expressionchangedafterithasbeencheckederror-explained/51662105#51662105

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment