angular / angular

Deliver web apps with confidence šŸš€
https://angular.dev
MIT License
95.1k stars 24.9k forks source link

ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked on ng 4 #17572

Closed altreze closed 6 years ago

altreze commented 7 years ago

I'm submitting a ...


[ ] Regression (behavior that used to work and stopped working in a new release)
[X ] Bug report #14748 
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

After using an observable object in my template using async, I am receiving : ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: ''. Current value: 'something'

Expected behavior

Please tell us about your environment


Angular version: 4.2.2

Browser:
- [X] Chrome (desktop) version Version 58.0.3029.110 (64-bit)

For Tooling issues:
- Node version: v6.10.3
- Platform: Mac            
elvisbegovic commented 6 years ago

@touqeershafi if you use this.changeDetector.markForCheck(); doesn't it mean you are not using things in right way

rwngallego commented 6 years ago

This is still happening. This is definitely a bug, not an expected behavior and should be fixed.

touqeershafi commented 6 years ago

@istiti i understand that's not a proper way but at-least you can get rid of this error for temporary, But as far as my project is concern i've already added the TODO with bug url.

fugwenna commented 6 years ago

This is still happening. This is definitely a bug, not an expected behavior and should be fixed.

I think I have to agree with @rwngallego ... Before the 4.2.x update, I had already written a number of reusable/ng-content stlye components which depend on @ViewChildren, @ContentChildren and create relationships between parent and child through these channels (getting/setting properties and so forth).

I find myself constantly having to inject the ChangeDetectorRef instance into all of these constructors (including child classes which extend this components), and it does not feel like intended or correct behavior.

ghetolay commented 6 years ago

For the ones using dynamic component like @saverett (using router is using dynamic component) I think it's the problem stated here https://github.com/angular/angular/issues/15634.

For anybody doing changes inside ngAfterViewInit / ngAfterViewChecked I think it's expected.

For the others, (haven't seen any yet here) it's still a mystery to me.

sulhome commented 6 years ago

This is happening for me in the following scenario. Have a parent component which uses a child component. The child component needs data that the parent will supply but this data is coming from a promise result in the parent constructor. So:

parent

constructor {
dataservice.getData().then((result) => {
this.source = result;
});
}

Parent template

<app-child [source]="source"></app-child>

child component

@Input() source:any[];

This code throws this error however adding this to the parent component solved the issue:

constructor(private cdRef: ChangeDetectorRef) {
dataservice.getData().then((result) => {
this.source = result;
 this.cdRef.detectChanges();
});
    }

Now is this a recommended solution or is it a workaround for a bug and what would be the implications of having such change? to me it looks like you are telling Angular to detect changes which I think is something you should avoid so...

mlc-mlapis commented 6 years ago

@sulhome ... your problem is that the promise in the parent's constructor is changing @Input on the child component ... during CD cycle and its confirmation in dev mode. That is the reason of the error.

The line this.cdRef.detectChanges(); adds the second CD cycle so the confirmation after it is succesful and => no error.

So everything works as expected. It is important to understand how it works to use the right constructs and general logic in your code to avoid these type of problems. You can read about it in the deep explanation here: https://hackernoon.com/everything-you-need-to-know-about-the-expressionchangedafterithasbeencheckederror-error-e3fd9ce7dbb4

sulhome commented 6 years ago

@mlc-mlapis so you are saying that I am doing it right by adding this.cdRef.detectChanges(); and that this doesn't have any implications? I thought by listening to ngOnChanges on the child then I should pick the change in the @Input from parent. is that not the right approach to do it? Please note that I have done that and I was still receiving the error but I thought that's what ngOnChanges is designed for

mlc-mlapis commented 6 years ago

@sulhome Yes, this.cdRef.detectChanges(); is what helps but it has also the consequence that you have 2 CDs now ... so it depends how and where you are using OnPush strategy on components to eliminate overhead of one more CD cycle.

If it is true that dataservice.getData takes some data using http then probably the easiest solution would be to have an observable in that service and subsribe to it from the child component. So when the data will be emitted on the stream the child can react to it.

victornoel commented 6 years ago

@ghetolay for the record, there is some cases when it is a bug, see for example #18129 that I opened.

kotran88 commented 6 years ago

I have this issue when I get data from child component with @Output.

at parent componet home.ts

drag_second(trigger){
        console.log("dragged222222"+trigger);
        if(trigger){
          this.isactive=true;
        }else{
          this.isactive=false;
        }
    }

and home.html <div class="upper" [ngClass]="{'isactive':isactive}">

isactive is changed when dragstart and dragend....

error message is ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'true'. Current value: 'false'.

or is there a any other ways that I can change/add class attribute?

Wernerson commented 6 years ago

Same issue in 4.2.x and 4.3.0 but not 4.1.x.

Error only occurs on routes that use ng-template.

Solution: Putting all code in ngAfterViewInit into setTimeout solved the problem for me (thanks/credits to @jingglang).

public ngAfterViewInit(): void {
    setTimeout(() => {
        //my code
    });
}
curtismorte commented 6 years ago

@Wernerson, take a look at JSMike's comment about setTimeout. It's important to note that you are actually just changing the code outside of the lifecycle hook.

ribizli commented 6 years ago

My issue was that I used ActivatedRoute's params Observable to modify some state in the ngOnInit(). It seems, that the Router emits the params synchron, so the update happens during CD. My simple solution for this is currently:

  // !!! see delay(0)
  // results are consumed by async Pipe, where I also had the CD issue (null vs. undefined)
  this.results = this._activatedRoute.params.delay(0)
    .do(params => this.searchParameters = this._createSearchParameters(params))
    .switchMap(p => {
      let key = this.searchParameters.key();
      return this._searchResultCache.results.map(r => r.get(key));
    });

(I know this is not the most beautiful reactive/functional code ever ;) ) So the solution was to delay(0) the parameter emission by one tick.

Hope I can help for others having similar issues. Also any concerns are welcomed, how my solution is wrong or dangerous.

rolandoldengarm commented 6 years ago

What's the status of this issue? It's still apparent in Angular 4.2.6. Is this a problem in production builds? As this error will only happen in debug builds.

I hate all the workarounds with manually running change detection, or even running it in a setTimeout()...

zoechi commented 6 years ago

setTimeout() is inefficient, because it causes a new change detection cycle on the whole application, while detectChanges() only checks the current component AFAIK.

adamdport commented 6 years ago

My team had a core misunderstanding of what ngOnInit is and isn't. The docs say

ngOnInit(): Initialize the directive/component after Angular first displays the data-bound properties and sets the directive/component's input properties.CalledĀ once, after theĀ firstĀ ngOnChanges().

We were subscribing to our services (that keep track of application state) from ngOnInit. Our subscriptions are synchronous BehaviorSubjects; they fire at subscription time. So "after Angular first...sets the directive/component's input properties", we were immediately changing the values. Here's a very simple Plunker illustrating that being a problem.

The example in Tour Of Heroes ultimately uses http which is asynchronous, and therefore doesn't cause issues. I think this is the source of our confusion.

What we're doing now is subscribing to our synchronous states from our constructor instead, so that our components' values will be set immediately. Others in this thread have suggested kicking off a second change detection via timeouts, setting values later, or explicitly telling angular to do so. Doing so will keep the error from appearing, but imho it's probably unnecessary.

The article that @maximusk shared is a must read: Everything you need to know about the ExpressionChangedAfterItHasBeenCheckedError error.

If nothing else, this thread should result in an in-depth warning of all this in the official angular docs.

maxkoretskyi commented 6 years ago

The article that @maximusk shared is a must read: Everything you need to know about the ExpressionChangedAfterItHasBeenCheckedError error.

Thank you

fugwenna commented 6 years ago

@adamdport

Does this work in the case of using @ViewChildren @ContentChildren to modify child properties from the parent, since the children are not available until the ngAfterViewInit() lifecycle has been executed?

Martin-Luft commented 6 years ago

Please keep in mind that a part of this issue is a confirmed bug and not a user fault: https://github.com/angular/angular/issues/15634

dgroh commented 6 years ago

The problem @saverett addressed is correct, using *ngIf was causing the error.

photostu commented 6 years ago

*watching...

Splaktar commented 6 years ago

This can happen if you pass objects to a Angular Material dialog and then in the dialog use them directly rather than doing this.myDataInput = Object.assign({}, dialogDataInput). I.e. this.myDataInput = dialogDataInput; and then doing something like this.myDataInput.isDefault = !this.myDataInput.isDefault;. Like explained above, the child component (dialog) is changing values that are exposed in the parent component's view.

ding-kai commented 6 years ago

replace attribute with [attribute]

Wernerson commented 6 years ago

Does anyone know for sure if this is an intended error or a bug? Isn't it correct to load data in ngOnInit? Because if I move the code to the constructor (loading through an Observable) the error isn't thrown...

dgroh commented 6 years ago

@Wernerson . Do you have ngIf in your html? beware that things are going to be rendered after ngOnInit has finished, solong, everything inside ngIf won't work and might lead to this error.

j-nord commented 6 years ago

It has to be reworked. With "ngIf" and custom controls you get this error message. A timeout is certainly not a solution.

zoechi commented 6 years ago

@j-nord if you'd read the comments, you'd know that timeout isn't required.

mawo87 commented 6 years ago

@j-nord / @dgroh I've experienced the same. I've been using an ng-hint which uses an ngIf directive and I cannot get rid of this error. Did you manage to implement a workaround?

Martin-Luft commented 6 years ago

@Wernerson yes, a part of this issue is an intended error or a bug: https://github.com/angular/angular/issues/15634

dgroh commented 6 years ago

@mawo87 it worked for me with hidden as it does not remove the element from the DOM. For our requirement this was ok and was also a clean solution.

j-nord commented 6 years ago

@mawo87 First you have to find the variable which is meant by the error message. @Angular Team: It would be nice to have the name of the variable in the error message. This is not easy to find by larger masks and Webpack. If you found the command where it happens, the command in the follow line will help:

this.cdr.detectionChanges();

See the example from vermilion928 from 19.Jun.

mawo87 commented 6 years ago

Thanks @dgroh and @j-nord. @j-nord I was also looking at the detectionChanges method, but I might not use this approach as in the following article it was advised against it: https://hackernoon.com/everything-you-need-to-know-about-the-expressionchangedafterithasbeencheckederror-error-e3fd9ce7dbb4

alignsoft commented 6 years ago

In my case, I'm using a PageTitle service and subscribing to it in the parent component, and then as child components are loaded into view, they set the PageTitle in the service and the parent gets the value and displays it via the subscribe. This worked without any errors in 4.1.x, but throws the ExpressionChangedAfterItHasBeenCheckedError error in 4.2x and later.

Everything still works as expected, and the errors don't display in deployed/production builds (due to the lack of a second change detection cycle), but in development there's an awful lot of red in my console.

I'm still extremely unclear as to what the appropriate (even short term) fix is. My parent component subscriptions are in the constructor, and the child component updates via the service are in ngOnInit.

I've read the 'everything you need to know about...' post, which was informative, but doesn't lead me to believe I'm doing anything incorrectly, and does't provide a working solution to the problem.

rolandoldengarm commented 6 years ago

@alignsoft if you would set the pagetitle in the child components in the ngOnInit, then I believe the error is valid... See the article that was linked before.

fugwenna commented 6 years ago

@alignsoft I agree, there has been a lot of discussion around this error, but without real clear instruction on whether its valid or not... is this more of a "warning" to the dev about standards and data binding/manipulation?

In my specific case, I'm dealing with parent/child components using @ViewChild and @ContentChild, having to call detectChanges() over and over to stop the error from showing in the console...

alignsoft commented 6 years ago

@rolandoldengarm I've tried subscribing in the parent constructor and setting the values in AfterViewInit in the child components and still get the error. Is that the correct way to handle this based on the flow and change detection cycle, and the error is potentially a bug?

kavi87 commented 6 years ago

Please, it would be great to have some kind of official answer here ...

JSMike commented 6 years ago

Looking at this again. Looks like my example from https://github.com/angular/angular/issues/17572#issuecomment-315096085 should have been using AfterContentInit instead of AfterViewInit when trying to manipulate ContentChildren as part of the component lifecycle.

Here's the updated plnkr with AfterContentInit and no more error: http://plnkr.co/edit/rkjTmqclHmqmpuwlD0Y9?p=preview

Wernerson commented 6 years ago

@JSMike I changed AfterViewInit to AfterContentInit, still the same error. Even if I update my data inside OnInit the error occurs. As far as I know this is according to the Angular lifecycle correct (right?) to initialize data during OnInit. Together with the fact that this error didn't occur in past versions I assume this is a bug. However I would like to have some kind of statement from the development team if that is so..? :D

JSMike commented 6 years ago

@Wernerson you shouldn't be updating data inside ngOnInit unless the data is available at that lifecycle hook. You should move your logic to be in ngAfterContentInit if it has to do with content children.

Wernerson commented 6 years ago

@JSMike Didn't help, the error still occures. I just subscribe to a state change in the application after I received the data requested (asynchronously) in ngAfterContentInit.

JSMike commented 6 years ago

@Wernerson do you have a plnkr example?

soldiertt commented 6 years ago

Basic sample with StackBlitz A child component emitting an event and parent component updating local variable. Get error in js console : ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'undefined'. Current value: 'hello world'.

JSMike commented 6 years ago

Found out, thengAfterContentInit only works if the children are static, it still fails when creating children using *ngFor http://plnkr.co/edit/rkjTmqclHmqmpuwlD0Y9?p=preview

JSMike commented 6 years ago

@soldiertt you just need detectChanges() https://stackblitz.com/edit/angular-qfu74y

soldiertt commented 6 years ago

@JSMike if i do this, ngAfterViewInit of child component is not triggered : https://stackblitz.com/edit/angular-bdwmgf

JSMike commented 6 years ago

@soldiertt that seems odd, but looks like a separate issue that should be reported.

Wernerson commented 6 years ago

@JSMike My setup is fairly complex with services, third party libraries, confidential information, etc. etc. Unfortunately I couldn't reproduce the bug in a plunker :/

ichorville commented 6 years ago

Injection of ChangeDetectorRef worked for me!