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

ComponentRef Change detector doesn't trigger detecting for view #36667

Closed
RolginRoman opened this issue Apr 16, 2020 · 12 comments
Closed

ComponentRef Change detector doesn't trigger detecting for view #36667

RolginRoman opened this issue Apr 16, 2020 · 12 comments
Assignees
Labels
area: core Issues related to the framework runtime core: change detection core: dynamic view creation freq2: medium P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix workaround2: non-obvious
Milestone

Comments

@RolginRoman
Copy link

🐞 bug report

Affected Package

The issue is caused by package @angular/core

Is this a regression?

Cannot say

Description

Component created by #ComponentFactoryResolver.
ComponentFactoryResolver#createComponent returns ComponentRef that does have changeDetectorRef.
After any changes to component's input by code (not by template) and invocation of such ComponentRef#changeDetectorRef.detectChanges component's view won't be updated.

example from repro: dynamic-wrapper.component.ts#createComponent.

componentRef.changeDetectorRef.detectChanges()

This is root issue.

To testing purposes ChangeDetectorRef was be added to component's constructor and invoked by
consumer of this ComponentFactoryResolver. CDR from component's injector works as expected

example from repro: dynamic-wrapper.component.ts#createComponent

componentRef.instance.cdr.detectChanges()

🔬 Minimal Reproduction

StackBlitz

GitHub repro repository:
https://github.com/RolginRoman/ng-dynamic-components-cdr-repro

🔥 Exception or Error


No exception

🌍 Your Environment

Angular Version:



 Angular version 9.1.0

Anything else relevant?

Seems like browser, OS version or anything else doesn't matter

@atscott atscott added the area: core Issues related to the framework runtime label Apr 16, 2020
@ngbot ngbot bot added this to the needsTriage milestone Apr 16, 2020
@pkozlowski-opensource
Copy link
Member

This is a confirmed bug that still happens in the Angular v9 with ivy enabled (the same bug is present in VE). Here is a simplified reproduce scenario: https://stackblitz.com/edit/angular-i8tus5?file=package.json

What is going on here is that changeDetectorRef on a ComponentRef points to the change detector of the root (host) view of a dynamically created component. Then, inside the host view we've got the actual component view, but the component view is OnPush thus we never refresh it!

While it works as implemented (and as was implemented in VE), this behaviour is very confusing to the users and I would consider this to be broken from a user perspective. To actually understand why it doesn't work one needs to deeply understand inner workings of Angular (concept of the host view, OnPush etc.). I'm not sure what is the best long terms solution here but the fix won't be trivial.

The good news is that work-arrounds exists - the simplest one would be to remove ChangeDetectionStrategy.OnPush from the dynamically created component - this is what I would recommend for now @RolginRoman .

@pkozlowski-opensource
Copy link
Member

BTW, this issue will be very visible for people trying to test OnPush components - in fact this issue was already reported for TestBed as #12313 - it is currently highly up-voted and commented on issue biting people in practice.

@RolginRoman
Copy link
Author

@pkozlowski-opensource thanks for your feedback! 🙂

Is it leads us to 'not to use onPush with dynamic components at all'?
From my perspective such behaviour completely breaks many applications of dynamically-created components (from simple to advanced cases).
Component (or dev 😊) should be smart enough to understand where component will be created (from template or from such factories)

yeap, without OnPush it can work but it can affect app performance if there are many such components, can't it? Possible WA for it: detach CD from components and control CD cycles by self?

@montella1507
Copy link

I was researching a bug in our application for 5 days... Now i can confirm that ComponentRef.ChangeDetectorRef .detectChanges doesnt work, but ComponentRef.instance.cdr (injected CDR via construcotr) detectChanges works...

@montella1507
Copy link

Workarroud Is to use component's injector to get Real ChangeDetectorRef.

ComponentRef.injector.get(ChangeDetectorRef)

@lacolaco
Copy link
Contributor

lacolaco commented Aug 21, 2020

This problem is still happening with v10.0. Repro is here .

This problem happens often on using CDK Portal and Overlay. Dynamic view creation is not a corner case.

@jelbourn jelbourn added P4 A relatively minor issue that is not relevant to core functions and removed severity1: confusing labels Oct 1, 2020
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Oct 27, 2020
Close angular#39462, angular#36667

In `bootstrap` and `dynamic` components, there is a hostview to contain
the component view, if the component is OnPush, trigger change detection on the
hostview will not check on the OnPush rootComponent,
so we need to mark child components for check.

The cases are:

case 1: AppComponent is OnPush

```
import { Component, NgModule, ViewContainerRef, ComponentFactoryResolver, Injector,
 ChangeDetectionStrategy } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

@component({
  selector: 'my-app',
  template: `
    {{name}}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  name = 'initial value';
  ngOnInit() {

    setTimeout(() => {
      this.name = 'new value'
    }, 2000);
  }
}

@NgModule({
  imports:      [ BrowserModule ],
  declarations: [ AppComponent],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }
```

case 2: Dynamic component

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

So in this PR, for `RootViewRef.detectChanges`, we should always markViewDirty for
root component in the hostview.
@JiaLiPassion JiaLiPassion self-assigned this Oct 27, 2020
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Oct 27, 2020
Close angular#39462, angular#36667

In `bootstrap` and `dynamic` components, there is a hostview to contain
the component view, if the component is OnPush, trigger change detection on the
hostview will not check on the OnPush rootComponent,
so we need to mark child components for check.

The cases are:

case 1: AppComponent is OnPush

```
import { Component, NgModule, ViewContainerRef, ComponentFactoryResolver, Injector,
 ChangeDetectionStrategy } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

@component({
  selector: 'my-app',
  template: `
    {{name}}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  name = 'initial value';
  ngOnInit() {

    setTimeout(() => {
      this.name = 'new value'
    }, 2000);
  }
}

@NgModule({
  imports:      [ BrowserModule ],
  declarations: [ AppComponent],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }
```

case 2: Dynamic component

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

So in this PR, for `RootViewRef.detectChanges`, we should always markViewDirty for
root component in the hostview.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Oct 27, 2020
Close angular#39462, angular#36667

In `bootstrap` and `dynamic` components, there is a hostview to contain
the component view, if the component is OnPush, trigger change detection on the
hostview will not check on the OnPush rootComponent,
so we need to mark child components for check.

The cases are:

case 1: AppComponent is OnPush

```
import { Component, NgModule, ViewContainerRef, ComponentFactoryResolver, Injector,
 ChangeDetectionStrategy } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

@component({
  selector: 'my-app',
  template: `
    {{name}}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  name = 'initial value';
  ngOnInit() {

    setTimeout(() => {
      this.name = 'new value'
    }, 2000);
  }
}

@NgModule({
  imports:      [ BrowserModule ],
  declarations: [ AppComponent],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }
```

case 2: Dynamic component

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

So in this PR, for `RootViewRef.detectChanges`, we should always markViewDirty for
root component in the hostview.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Oct 28, 2020
Close angular#39462, angular#36667

In `bootstrap` and `dynamic` components, there is a hostview to contain
the component view, if the component is OnPush, trigger change detection on the
hostview will not check on the OnPush rootComponent,
so we need to mark child components for check.

The cases are:

case 1: AppComponent is OnPush

```
import { Component, NgModule, ViewContainerRef, ComponentFactoryResolver, Injector,
 ChangeDetectionStrategy } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

@component({
  selector: 'my-app',
  template: `
    {{name}}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  name = 'initial value';
  ngOnInit() {

    setTimeout(() => {
      this.name = 'new value'
    }, 2000);
  }
}

@NgModule({
  imports:      [ BrowserModule ],
  declarations: [ AppComponent],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }
```

case 2: Dynamic component

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

So in this PR, for `RootViewRef.detectChanges`, we should always markViewDirty for
root component in the hostview.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Oct 31, 2020
Close angular#39462, angular#36667

In `bootstrap` and `dynamic` components, there is a hostview to contain
the component view, if the component is OnPush, trigger change detection on the
hostview will not check on the OnPush rootComponent,
so we need to mark child components for check.

The cases are:

case 1: AppComponent is OnPush

```
import { Component, NgModule, ViewContainerRef, ComponentFactoryResolver, Injector,
 ChangeDetectionStrategy } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

@component({
  selector: 'my-app',
  template: `
    {{name}}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  name = 'initial value';
  ngOnInit() {

    setTimeout(() => {
      this.name = 'new value'
    }, 2000);
  }
}

@NgModule({
  imports:      [ BrowserModule ],
  declarations: [ AppComponent],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }
```

case 2: Dynamic component

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

So in this PR, for `RootViewRef.detectChanges`, we should always markViewDirty for
root component in the hostview.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Nov 2, 2020
…orrectly

Close angular#36667, angular#39524.

`dynamic` and `bootstrap` components has a parent `host view` as the placeholder.
And also all the hooks are registered to the tView of the `host view`, not the component view.
Running change detection from the `host view` has several issues.

1. componentRef.changeDetectorRef.detectChanges()/markForCheck() will not update the component.

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
       // cmptRef.changeDetectorRef is the RootViewRef of the hostView
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

2. The hooks (such as ngOnCheck) is executed, but the component is not updated.

```
import {
  Component,
  NgModule,
  ViewContainerRef,
  ComponentFactoryResolver,
  Injector,
  ChangeDetectionStrategy,
  DoCheck
} from "@angular/core";
import { BrowserModule } from "@angular/platform-browser";

import { timer } from "rxjs";

@component({
  selector: "dynamic",
  template: `
    {{ name }}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic implements DoCheck {
  name = "initial name";

  ngDoCheck() {
    console.log("docheck dynamic");
  }
}

@component({
  selector: "my-app",
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br />
    <b>Actual</b>:<br />
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent {
  constructor(
    private _vcRef: ViewContainerRef,
    private _cfr: ComponentFactoryResolver,
    private _injector: Injector
  ) {}

  ngOnInit() {
    const cmptRef = this._cfr
      .resolveComponentFactory(Dynamic)
      .create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
      cmptRef.instance.name = "changed name";
      console.log("changing name");
      cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}

@NgModule({
  imports: [BrowserModule],
  declarations: [AppComponent, Dynamic],
  entryComponents: [Dynamic],
  bootstrap: [AppComponent]
})
export class AppModule {}

```

This PR update several points.

1. When create a dynamic component, we create a host view and a component view.
We consider these 2 views a single unit,
if the component is OnPush, we set the Dirty flag to the host view to make it a an OnPush View,
 and set the CheckAlways flag to the component.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Nov 3, 2020
…orrectly

Close angular#36667, angular#39524.

`dynamic` and `bootstrap` components has a parent `host view` as the placeholder.
And also all the hooks are registered to the tView of the `host view`, not the component view.
Running change detection from the `host view` has several issues.

1. componentRef.changeDetectorRef.detectChanges()/markForCheck() will not update the component.

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
       // cmptRef.changeDetectorRef is the RootViewRef of the hostView
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

2. The hooks (such as ngOnCheck) is executed, but the component is not updated.

```
import {
  Component,
  NgModule,
  ViewContainerRef,
  ComponentFactoryResolver,
  Injector,
  ChangeDetectionStrategy,
  DoCheck
} from "@angular/core";
import { BrowserModule } from "@angular/platform-browser";

import { timer } from "rxjs";

@component({
  selector: "dynamic",
  template: `
    {{ name }}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic implements DoCheck {
  name = "initial name";

  ngDoCheck() {
    console.log("docheck dynamic");
  }
}

@component({
  selector: "my-app",
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br />
    <b>Actual</b>:<br />
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent {
  constructor(
    private _vcRef: ViewContainerRef,
    private _cfr: ComponentFactoryResolver,
    private _injector: Injector
  ) {}

  ngOnInit() {
    const cmptRef = this._cfr
      .resolveComponentFactory(Dynamic)
      .create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
      cmptRef.instance.name = "changed name";
      console.log("changing name");
      cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}

@NgModule({
  imports: [BrowserModule],
  declarations: [AppComponent, Dynamic],
  entryComponents: [Dynamic],
  bootstrap: [AppComponent]
})
export class AppModule {}

```

This PR update several points.

1. When create a dynamic component, we create a host view and a component view.
We consider these 2 views a single unit,
if the component is OnPush, we set the Dirty flag to the host view to make it a an OnPush View,
 and set the CheckAlways flag to the component.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Nov 4, 2020
…orrectly

Close angular#36667, angular#39524.

`dynamic` and `bootstrap` components has a parent `host view` as the placeholder.
And also all the hooks are registered to the tView of the `host view`, not the component view.
Running change detection from the `host view` has several issues.

1. componentRef.changeDetectorRef.detectChanges()/markForCheck() will not update the component.

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

@component({
  selector: 'my-app',
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br>
    <b>Actual</b>:<br>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent  {
  constructor(private _vcRef: ViewContainerRef,
     private _cfr: ComponentFactoryResolver, private _injector: Injector) {}

  ngOnInit() {
    const cmptRef = this._cfr.resolveComponentFactory(Dynamic).create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
        cmptRef.instance.name = 'changed name';
        console.log('changing name');
       // cmptRef.changeDetectorRef is the RootViewRef of the hostView
        cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}
```

2. The hooks (such as ngOnCheck) is executed, but the component is not updated.

```
import {
  Component,
  NgModule,
  ViewContainerRef,
  ComponentFactoryResolver,
  Injector,
  ChangeDetectionStrategy,
  DoCheck
} from "@angular/core";
import { BrowserModule } from "@angular/platform-browser";

import { timer } from "rxjs";

@component({
  selector: "dynamic",
  template: `
    {{ name }}
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic implements DoCheck {
  name = "initial name";

  ngDoCheck() {
    console.log("docheck dynamic");
  }
}

@component({
  selector: "my-app",
  template: `
    <b>Expected</b>: "initial name" changes to "changed name" after 2s<br />
    <b>Actual</b>:<br />
  `,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class AppComponent {
  constructor(
    private _vcRef: ViewContainerRef,
    private _cfr: ComponentFactoryResolver,
    private _injector: Injector
  ) {}

  ngOnInit() {
    const cmptRef = this._cfr
      .resolveComponentFactory(Dynamic)
      .create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
      cmptRef.instance.name = "changed name";
      console.log("changing name");
      cmptRef.changeDetectorRef.detectChanges();
    }, 2000);
  }
}

@NgModule({
  imports: [BrowserModule],
  declarations: [AppComponent, Dynamic],
  entryComponents: [Dynamic],
  bootstrap: [AppComponent]
})
export class AppModule {}

```

This PR update several points.

1. When create a dynamic component, we create a host view and a component view.
We consider these 2 views a single unit,
if the component is OnPush, we set the Dirty flag to the host view to make it a an OnPush View,
 and set the CheckAlways flag to the component.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Feb 15, 2021
…nd onPush not dirty component

Close angular#36667

Calling `componentRef.detectChanges()` sometimes not work as expected.

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

const componetRef = cfr.createComponent(Dynamic);
componentRef.changeDetectorRef // host view
componentRef.injector.get(ChangeDetectorRef) // component view
componentRef.instance.cdRef // componentView

componentRef.instance.name = 'something new';
// componentRef.instance.cdRef.markForCheck();
componentRef.detectChanges(); // does not work without `markForCheck`
componentRef.changeDetectorRef.detectChanges() // Users expect that Bar gets refreshed.
componentRef.injector.get(ChangeDetectorRef).detectChanges()
componentRef.instance.cdRef.detectChanges()

// Example: https://stackblitz.com/edit/angular-drqzhw?file=src%2Fapp%2Fapp.module.ts
ngOnInit() {
    const cmptRef = this._cfr
      .resolveComponentFactory(Dynamic)
      .create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
      cmptRef.instance.name = "changed name";
      console.log("changing name");
      cmptRef.changeDetectorRef.detectChanges(); // does not work
    });

    setTimeout(() => {
      cmptRef.instance.name = "second changed name";
      console.log("changing name");
      cmptRef.injector.get(ChangeDetectorRef).detectChanges(); // works
    }, 2000);
}
```

Since when the component is `boostrap` or `dynamic`, we created a hostView to host the component.
And the `componentRef.changeDetectorRef` references to that hostView instead of the componentView itself.
And when the component is `OnPush` and not dirty, calling `changeDetectorRef.detectChanges()` will bypass
the component which is confusing.

So this commit mark the root component dirty when calling `changeDetectorRef.detectChanges()` from the root hostView.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Feb 15, 2021
…nd onPush not dirty component

Close angular#36667

Calling `componentRef.detectChanges()` sometimes not work as expected.

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

const componetRef = cfr.createComponent(Dynamic);
componentRef.changeDetectorRef // host view
componentRef.injector.get(ChangeDetectorRef) // component view
componentRef.instance.cdRef // componentView

componentRef.instance.name = 'something new';
// componentRef.instance.cdRef.markForCheck();
componentRef.detectChanges(); // does not work without `markForCheck`
componentRef.changeDetectorRef.detectChanges() // Users expect that Bar gets refreshed.
componentRef.injector.get(ChangeDetectorRef).detectChanges()
componentRef.instance.cdRef.detectChanges()

// Example: https://stackblitz.com/edit/angular-drqzhw?file=src%2Fapp%2Fapp.module.ts
ngOnInit() {
    const cmptRef = this._cfr
      .resolveComponentFactory(Dynamic)
      .create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
      cmptRef.instance.name = "changed name";
      console.log("changing name");
      cmptRef.changeDetectorRef.detectChanges(); // does not work
    });

    setTimeout(() => {
      cmptRef.instance.name = "second changed name";
      console.log("changing name");
      cmptRef.injector.get(ChangeDetectorRef).detectChanges(); // works
    }, 2000);
}
```

Since when the component is `boostrap` or `dynamic`, we created a hostView to host the component.
And the `componentRef.changeDetectorRef` references to that hostView instead of the componentView itself.
And when the component is `OnPush` and not dirty, calling `changeDetectorRef.detectChanges()` will bypass
the component which is confusing.

So this commit mark the root component dirty when calling `changeDetectorRef.detectChanges()` from the root hostView.

This commit also resolves the following issue.

```
const myComp = TestBed.createComponent(OnPushComponent);
myComp.componentInstance.abc = 123;
myComp.detectChanges() // Does not work

// We could document that users should do this instead
// (and/or add a prop to ComponentFixture called `componentInstanceCDR`):
myComp.componentRef.injector.get(ChangeDetectorRef).detectChanges();
// myComp.componentInstanceCDR.detectChanges();
```

Now calling `myComp.detectChanges()` will work as expect and run CD on the OnPushComponent.
JiaLiPassion added a commit to JiaLiPassion/angular that referenced this issue Feb 15, 2021
…nd onPush not dirty component

Close angular#36667

Calling `componentRef.detectChanges()` sometimes not work as expected.

```
@component({
  selector: 'dynamic',
  template: `{{name}}`,
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class Dynamic  {
  name = 'initial name';
}

const componetRef = cfr.createComponent(Dynamic);
componentRef.changeDetectorRef // host view
componentRef.injector.get(ChangeDetectorRef) // component view
componentRef.instance.cdRef // componentView

componentRef.instance.name = 'something new';
// componentRef.instance.cdRef.markForCheck();
componentRef.detectChanges(); // does not work without `markForCheck`
componentRef.changeDetectorRef.detectChanges() // Users expect that Bar gets refreshed.
componentRef.injector.get(ChangeDetectorRef).detectChanges()
componentRef.instance.cdRef.detectChanges()

// Example: https://stackblitz.com/edit/angular-drqzhw?file=src%2Fapp%2Fapp.module.ts
ngOnInit() {
    const cmptRef = this._cfr
      .resolveComponentFactory(Dynamic)
      .create(this._injector);
    this._vcRef.insert(cmptRef.hostView);

    setTimeout(() => {
      cmptRef.instance.name = "changed name";
      console.log("changing name");
      cmptRef.changeDetectorRef.detectChanges(); // does not work
    });

    setTimeout(() => {
      cmptRef.instance.name = "second changed name";
      console.log("changing name");
      cmptRef.injector.get(ChangeDetectorRef).detectChanges(); // works
    }, 2000);
}
```

Since when the component is `boostrap` or `dynamic`, we created a hostView to host the component.
And the `componentRef.changeDetectorRef` references to that hostView instead of the componentView itself.
And when the component is `OnPush` and not dirty, calling `changeDetectorRef.detectChanges()` will bypass
the component which is confusing.

So this commit mark the root component dirty when calling `changeDetectorRef.detectChanges()` from the root hostView.

This commit also resolves the following issue.

```
const myComp = TestBed.createComponent(OnPushComponent);
myComp.componentInstance.abc = 123;
myComp.detectChanges() // Does not work

// We could document that users should do this instead
// (and/or add a prop to ComponentFixture called `componentInstanceCDR`):
myComp.componentRef.injector.get(ChangeDetectorRef).detectChanges();
// myComp.componentInstanceCDR.detectChanges();
```

Now calling `myComp.detectChanges()` will work as expect and run CD on the OnPushComponent.
@jgomesmv
Copy link

I'm also facing this issue on angular 11.0.8. You can find a minimal reproduction here:
https://stackblitz.com/edit/angular-ivy-componentfactory-onpush-issue

Hope it helps!

@davoam
Copy link

davoam commented Sep 24, 2021

Workarroud Is to use component's injector to get Real ChangeDetectorRef.

ComponentRef.injector.get(ChangeDetectorRef)

In my case the following code helped

this.componentRef.injector.get(ChangeDetectorRef).detectChanges();

@hoeni
Copy link

hoeni commented Dec 13, 2021

Hell, this took me some time to find out this is an Angular issue (NG 13), not mine. But at least the workaround to get a fresh CDR from the injector worked well for me!

@montella1507
Copy link

@pkozlowski-opensource as far as this bug is not yet fixed - can we atleast add note - warning, for example there:

https://angular.io/guide/dynamic-component-loader

that component's change detector is often not resolved 100% correctly and its recommended to use components injector to resolve Change detector?

@pkozlowski-opensource
Copy link
Member

This was just fixed via #46641 - the ComponentRef has a new method - setInput that correctly sets an input and makes sure that change detection is properly invoked. More details in #46641

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.