angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
94.71k stars 24.68k forks source link

input signal effects are not run when attaching a directive to an IonButton #55644

Open Eraldo opened 2 weeks ago

Eraldo commented 2 weeks ago

Which @angular/* package(s) are the source of the bug?

core, elements

Description

Expected: Running an effect on an input signal within a directive that is attached to an IonButton should trigger/run the effect code.

Actual: The effect is not run.

It works just fine if attached to other html tags.

  1. Open the reproduction url.
  2. Follow on screen instructions.

Insight: This is where the current behavior is probably sourced => https://github.com/ionic-team/ionic-framework/blob/ba5cebf2542c8fbd6c29af593a31a742e0caba1e/packages/angular/src/directives/proxies.ts#L357

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/stackblitz-starters-gkckxf?file=src%2Fmain.ts

Please provide the environment you discovered this bug in (run ng version)

Angular version: 17.3.4

Anything else?

Issue has been confirmed by "JB Nizet" and "Matthieu Riegler" and I was encouraged to create a bug report here.

Eraldo commented 2 weeks ago

Referenced in Ionic: https://github.com/ionic-team/ionic-framework/issues/29444

Flo0806 commented 1 week ago

Hello. We found the same issue in a new project inside our company. It looks like the problem is really the changeDetectorRef.detach. This simple component isn't shown in effect too, if you add the same directive:

import { ChangeDetectionStrategy, ChangeDetectorRef, Component } from '@angular/core';

@Component({
  selector: 'app-test-button',
  standalone: true,
  imports: [],
  templateUrl: './test-button.component.html',
  styleUrl: './test-button.component.css',
  changeDetection: ChangeDetectionStrategy.OnPush
})
export class TestButtonComponent {
constructor(changeRef: ChangeDetectorRef) {
  changeRef.detach();
}
}

Component and directive:

@Directive({
  selector: '[test]',
  standalone: true,
})
export class TestDirective {
  test = input.required<string>();
  test2 = computed(() => {
    return this.test() + '2';
  });

  constructor() {
    effect(() => {
      console.log('>> EFFECT', this.test2());
    });
  }
}

.........

<app-test-button [test]="inputText">
    Button with Directive
</app-test-button>
JeanMeche commented 1 week ago

The root cause is the injection of the ChangeDetectorRef in directives. When the directive is applied to a component, the CDR injected in the directive is the components CDR.

The effect is scheduled to run first when the component is init and uses the CDR to do that. In this case, the component is never init (because of running detach in the constructor) so the effect is never started.

A workaround for this would be to let the component finish its initialization and detach only in the ngAfterViewInit hook. This way the effect will

pkozlowski-opensource commented 2 days ago

Here is a minimal repro: https://stackblitz.com/edit/stackblitz-starters-7zd1fv?file=src%2Fmain.ts

As @JeanMeche pointed out, the root cause here is a component detaching from the change detection tree. Having said this I do recognize that this is very non-obvious - this is one of the cases we want to look into before stabilizing effects API.

pkozlowski-opensource commented 2 days ago

I've created #55808 to capture the root cause without any external dependencies. Let's continue the discussion in there.

Duplicate of #55808

alxhub commented 16 hours ago

Reopening as this is a separate issue than #55808.