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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ivy] ASSERTION ERROR: Should be run in update mode #32756

Open
fr0 opened this issue Sep 18, 2019 · 33 comments
Open

[Ivy] ASSERTION ERROR: Should be run in update mode #32756

fr0 opened this issue Sep 18, 2019 · 33 comments
Labels
area: core Issues related to the framework runtime core: change detection freq2: medium P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@fr0
Copy link

fr0 commented Sep 18, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/core (Ivy)

Is this a regression?

Yes, works in non-Ivy.

Description

I suspect this happens as a result of having an Input property that is set directly (isExpanded="true") instead of using a binding ([isExpanded]="true"), and having the property setter call changeDetectorRef.detectChanges(). It should be noted that this works in non-ivy.

馃敩 Minimal Reproduction

https://github.com/fr0/angular-test/tree/ivy

  1. Clone repo
  2. git checkout ivy
  3. npm install
  4. npm start
  5. Open http://localhost:4200 in Chrome
  6. View browser console log

馃敟 Exception or Error


core.js:5829 ERROR Error: ASSERTION ERROR: Should be run in update mode
    at throwError (core.js:1164)
    at assertEqual (core.js:1123)
    at refreshView (core.js:12425)
    at detectChangesInternal (core.js:13907)
    at ViewRef.detectChanges (core.js:15373)
    at ExpandableIfComponent.set isExpanded [as isExpanded] (expandable-if.component.ts:60)
    at setInputsFromAttrs (core.js:13579)
    at postProcessDirective (core.js:13335)
    at instantiateAllDirectives (core.js:13234)
    at createDirectivesInstances (core.js:12618)

馃實 Your Environment

Angular Version:




Angular CLI: 9.0.0-next.5
Node: 12.6.0
OS: darwin x64
Angular: 9.0.0-next.7
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-next.5
@angular-devkit/build-angular     0.900.0-next.5
@angular-devkit/build-optimizer   0.900.0-next.5
@angular-devkit/build-webpack     0.900.0-next.5
@angular-devkit/core              9.0.0-next.5
@angular-devkit/schematics        9.0.0-next.5
@angular/cli                      9.0.0-next.5
@ngtools/webpack                  9.0.0-next.5
@schematics/angular               9.0.0-next.5
@schematics/update                0.900.0-next.5
ng-packagr                        5.5.1
rxjs                              6.5.3
typescript                        3.5.3
webpack                           4.40.2
    

@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Sep 18, 2019
@ngbot ngbot bot added this to the needsTriage milestone Sep 18, 2019
@mhevery
Copy link
Contributor

mhevery commented Oct 8, 2019

Please provide minimal reproduction with exact steps on how to reproduce it. We have limited time and can't go explore your application.

@fr0
Copy link
Author

fr0 commented Oct 8, 2019

The error occurs for me under angular 9.0.0-next.5 (which is what the linked ivy branch in the linked repo uses) when I run ng serve and launch http://localhost:4200.

So:

  1. Clone repo
  2. git checkout ivy
  3. npm install
  4. npm start
  5. Open http://localhost:4200 in Chrome
  6. View browser console log

@fr0
Copy link
Author

fr0 commented Oct 9, 2019

I verified that the problem still occurs with 9.0.0-next.8.

@kara
Copy link
Contributor

kara commented Oct 15, 2019

@fr0 Thanks, I was able to reproduce. We'll add this to the list!

@SebastianPodgajny
Copy link

SebastianPodgajny commented Oct 24, 2019

@kara We are using angular without ngZone and this issue is happening all over our code base because we have a lot of manual change detection with cdRef.detectChanges()
Here is another example of this kind of error
image
image
image
image

Angular CLI: 9.0.0-next.15
Node: 12.10.0
OS: darwin x64
Angular: 9.0.0-next.13
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... platform-server, router

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.900.0-next.15
@angular-devkit/build-angular      0.900.0-next.15
@angular-devkit/build-ng-packagr   0.900.0-next.15
@angular-devkit/build-optimizer    0.900.0-next.15
@angular-devkit/build-webpack      0.900.0-next.15
@angular-devkit/core               9.0.0-next.15
@angular-devkit/schematics         9.0.0-next.15
@angular/cdk                       9.0.0-next.0
@angular/cli                       9.0.0-next.15
@ngtools/webpack                   9.0.0-next.15
@schematics/angular                8.0.1
@schematics/update                 0.900.0-next.15
ng-packagr                         9.0.0-rc.0
rxjs                               6.5.3
typescript                         3.6.4
webpack                            4.41.2

@pkozlowski-opensource
Copy link
Member

@SebastianPodgajny the case you are mentioning seem to be different as compared to the originally reported here (different error message, different case).

Could you please open a new issue and provide a reduced reproduction scenario? Thnx!

@pkozlowski-opensource
Copy link
Member

A typical scenario, where this error occurs is something along those lines:

    @Input()
    public set readonly(value: boolean) {
        // somewhere in other function:
        this.changeDetectorRef.detectChanges();
    }

Basically we are invoking an input setter in the creation mode (this might happen if an input is set from a static attribute). 2 possible solutions here:

  • ignore CD calls in creation mode (most of the time we will have update pass anyway);
  • more directive inputs setting (from static atrs) to the update mode (?)

@fxck
Copy link

fxck commented Mar 2, 2020

Any update on this?

@jackfuchs
Copy link

Any update on this?

PR is still open
#33463

@fxck
Copy link

fxck commented Mar 4, 2020

Yeah, but there was no activity on that PR for 6 months now. Last comment from @pkozlowski-opensource is only about a month old.

Anyway the problem, in my case anyway, seems to have disappeared when I removed OnPush from components that relied on markDirty to trigger CD.

@matiskiba
Copy link

Is there a solution planned for this issue? It prevents me from moving to angular 9.
The problem I'm experiencing is with OnPush combined with setters (as described above)

@MatteoMeil
Copy link

MatteoMeil commented Mar 18, 2020

Same issue here.
I experienced the issue using this.changeDetectorRef.detectChanges() in a sync operation.

Some background: I'm building an Electron Application using Angular 9.0.6 + Electron 8 + @angular-guru/electron-builder (as builder)

Using the code below triggers the error:

import { execSync } from 'child_process';
import { ChangeDetectorRef, Component } from '@angular/core';
// ... code to construct component
constructor(private changeDetectorRef: ChangeDetectorRef) {
 
 const data = execSync(`some command line executable`);
 this._init(JSON.parse(data.toString('utf8')));
}

private _init(data) {
  // some processing with data 
  this.changeDetectorRef.detectChanges(); // this line throws the error
} 

Removing that line, or changing to as below, solved my problem:

import { exec } from 'child_process';
import { ChangeDetectorRef, Component } from '@angular/core';
// ... code to construct component
constructor(private changeDetectorRef: ChangeDetectorRef) {
  exec(`some command line executable`, (data) => {
    this._init(JSON.parse(data));
  });
}

private _init(data: Button[]) {
  // some processing
  this.changeDetectorRef.detectChanges();
}

Hope this could be helpful for someone.
My environment:

Angular CLI: 9.0.6
Node: 12.16.1
OS: win32 x64

Angular: 9.0.6
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.6
@angular-devkit/build-angular     0.900.6
@angular-devkit/build-optimizer   0.900.6
@angular-devkit/build-webpack     0.900.6
@angular-devkit/core              9.0.6
@angular-devkit/schematics        9.0.6
@angular/cdk                      9.1.2
@ngtools/webpack                  9.0.6
@schematics/angular               9.0.6
@schematics/update                0.900.6
rxjs                              6.5.4
typescript                        3.7.5
webpack                           4.41.2

@admir86
Copy link

admir86 commented May 9, 2020

same problem.
any updates?

@mvaljento
Copy link

Same... :(

@zampage
Copy link

zampage commented May 12, 2020

This breaks a big chunk of our application, any updates on this issue?

@carlosleyton
Copy link

same here :c

@BzenkoSergey
Copy link

BzenkoSergey commented May 16, 2020

oh, the same issue (angular 9.1.7 with { ngZone: 'noop' })

resolved it by moving logic out of constructor to ngOnInit

@BenLad17
Copy link

BenLad17 commented Jun 8, 2020

Does anybody know when this will be fixed?

@akshayr-tavisca
Copy link

Any Update on Above Issue?

@futbolsalas15
Copy link

Hello, any update?

This could be a blocker for projects trying to upgrade to Angular 9.

@MartinJHammer
Copy link

@BzenkoSergey It worked! You are a champ!

Looked through my company's project for ALL logic defined in components constructors, and moved anything I found to ngOnInit, and bam, no error.

@martyniukroman
Copy link

oh, the same issue (angular 9.1.7 with { ngZone: 'noop' })

resolved it by moving logic out of constructor to ngOnInit

I believe this is bad practice to have some logic within constructor in any case.
But yeah, worked for me.

@gwartnes
Copy link

For anyone coming here that has a similar issue, what I had previously in my project was a setter on a @ViewChild decorated property, and within the setter I was calling detectChanges(). After upgrading to Angular 9, we were required to add the {static: boolean} property to the @ViewChild decorator, and another dev used static: true. Changing that to false solved my issue.

@sviat9440
Copy link

Temporary fix for me:
replace call detectChanges() to markForCheck().

@qpi
Copy link

qpi commented Dec 16, 2020

I have the same issue under Angular 10 when call detectChanges(). I tried to upgrade to 11 but still the same

@id1945
Copy link

id1945 commented Jan 11, 2021

I found that placing detectChanges() in the constructor caused an error. So I wrote it in the ngOnInt() function and it works properly with my project.
I hope to help someone here

@jelbourn
Copy link
Member

Copying notes on this from an internal tracker:

In the example, the input setter calls detectChanges, which throws because it鈥檚 an unbound input and thus set in creation mode (and refreshView is only callable in update mode.)

The question is - should we allow change detection to run in creation mode at all? In View Engine, we would throw if change detection was called in a directive constructor. But we allowed CD calls in input setters because inputs were always set in update mode.

We could call renderView in detectChangesInternal to fix the refreshView error, but then we would still end up calling change detection in the middle of creation mode, which violates a lot of our assumptions. We would end up calling lifecycle hooks for child components too, which would mess up child component lifecycle hook order.

We need to have a longer conversation about:

whether we should allow sync change detection in setters

whether we should abandon setting unbound inputs in creation mode bc it causes weirdness

michalakadam added a commit to michalakadam/perfect_house that referenced this issue Feb 13, 2021
@sciborrudnicki
Copy link

I use setTimeout(() => this.changeDetector.detectChanges(), 0);

@aksheyjhalani27
Copy link

Any updates so far on this.?

@s3nt1n3lz21
Copy link

This error appeared when I was calling detectChanges() in the constructor after subscribing to some data. I moved the subscription and detectChanges() to ngOnInit() instead and it disappeared.

@blyndcide
Copy link

blyndcide commented Jun 11, 2021

In Angular 12 this is happening even when not in a constructor. Simply calling .detectChanges() in an observable that, I don't know...initially runs too early and fast?, throws this error. Constructor has nothing in the body.

@joepvl
Copy link

joepvl commented Jun 23, 2022

If you're running into this when writing tests and mocking a normally async call using of(yourMockedResult), it could be because your component's code is subscribing to the observable in the constructor and calling changeDetectorRef.detectChanges() in the callback. Because the call is async in your application, this does not pose an issue when running it, but mocking the observable using of(...) is making it sync in this case, which is why Angular complains. A pragmatic solution: use of(yourMockedResult).pipe(delay(0)) instead, which forces the observable to be async. That way, the changeDetectorRef.detectChanges() call will be pushed further along in the event loop, giving Angular a chance to put everything in place. (That said, it's probably a good idea to move it to ngOnInit if you can.)

@brianpooe
Copy link

Some of us cannot move the logic from the constructor to ngOnInit cause the application does not update as expected in that case using onPush strategy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime core: change detection freq2: medium P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.