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

Dynamic component detectChanges trigger ngDoCheck but not refresh view #39524

Closed
JiaLiPassion opened this issue Oct 31, 2020 · 4 comments
Closed
Assignees
Labels
area: core Issues related to the framework runtime core: change detection P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone

Comments

@JiaLiPassion
Copy link
Contributor

JiaLiPassion commented Oct 31, 2020

This is a related issue of #36667.
Reproduce live demo is here.
https://stackblitz.com/edit/angular-vz1dca?file=src%2Fapp%2Fapp.module.ts
https://stackblitz.com/edit/angular-7h6eer?file=src%2Fapp%2Fapp.module.ts

code is this.

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 {}

The reason is for dynamic component, there is a host view as a placeholder, the hooks are registered on the host view not the component view, when calling componentRef.changeDetectorRef.detectChanges(), it will run change detection from root host view, and trigger the doCheck hook, but when it really want to refresh child component, since it is a onPush component and not marked dirty, it will not be refreshed.

I will try to resolve this issue in #39463 too.

@JiaLiPassion JiaLiPassion self-assigned this Oct 31, 2020
@JiaLiPassion JiaLiPassion added area: core Issues related to the framework runtime core: change detection labels Oct 31, 2020
@ngbot ngbot bot added this to the needsTriage milestone Oct 31, 2020
@alexzuza
Copy link
Contributor

alexzuza commented Nov 2, 2020

Yes, it would be great if it would work this way.

As workaround people usually use smth like:

cmptRef.injector.get(ChangeDetectorRef).detectChanges();

https://stackblitz.com/edit/angular-kbmdjf?file=src%2Fapp%2Fapp.module.ts

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
Copy link
Contributor Author

@alexzuza , yeah, and in fact this is a bug, since if you are calling change detection from the parent of this dynamic component, the ngDoCheck will also be triggered and the component is not updated. I am trying to make it work as expected.

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.
@AndrewKushnir AndrewKushnir added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 4, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 4, 2020
@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.
Labels
area: core Issues related to the framework runtime core: change detection P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

4 participants