fix(angular): strict template event bindings

[sean-perkins]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

The Angular bindings for the web components are incorrectly type checked by Angular Language Service with strictTemplates enabled. The previous usage of fromEvent, resulted in the types of addEventListener being applied to output events when using template event binding: (ionSlideDidChange)="doSlideChange($event)" instead of CustomEvent.

Issue Number: #24245

What is the new behavior?

The Angular bindings no longer need to use fromEvent to create an observable from the event source. They instead generate with Angular's @Output() decorator, which correctly associates the type checking for the output events.

Does this introduce a breaking change?

• Yes
• No

This change will prevent a non-standard pattern for binding to output events in class implementations, but alternative patterns exist to provide the same solution.

Before this would work:

@ViewChild(IonSlides, { static: true }) slides: IonSlides;

this.slides.ionSlideDidChange.subscribe(() => {
  console.log('slideDidChange');
});

After, implementers will need to use rxjs to convert the DOM event:

import { fromEvent } from 'rxjs';

@ViewChild(IonSlides, { static: true, read: ElementRef }) slides: IonSlides;

fromEvent(this.slides, 'ionSlideDidChange').subscribe(() => {
  console.log('slideDidChange');
});

Note: In both implementations the subscriptions are always hot and should be tore down later. This is the downside of this non-standard pattern. With the template binding syntax, Angular will automatically tear down the subscription for you.

I spoke with Mike & other GDEs and this pattern is very rare.

Other information

Dependent on this PR to adjust the generated proxy output: ionic-team/stencil-ds-output-targets#209

Will need a prerelease version to validate outside of locally packing.

Also this PR will need to rebase with the changes after #24313 (likely rebasing the PR in stencil-ds-output-targets).

[hakimio]

Is anyone ever going to review this?

[sean-perkins]

@hakimio the team has done an internal review of this issue and the PR suggestion that I made.

The problem with my approach is that it causes a breaking change in a pattern that existing Ionic Angular applications could be making use of (there's no way to validate). Merging this change as-is, would only make sense in a major semantic version (i.e Ionic v7).

Instead, we are still diagnosing internally and trying to find a way to satisfy the strict template binding change within Angular while still supporting both versions of event binding:

<ion-input (ionChange)="onChange($event)"></ion-input>

@ViewChild(IonInput) input;

input.ionChange.subscribe(event => {

});

[hakimio]

The fact is that no Angular developer uses this pattern and there is no point in supporting it...

[liamdebeasi]

@sean-perkins We do not have this scheduled in our current sprint. Can we close this for now?

As Sean noted, we are looking to make these changes in a major release of Ionic to account for any potential breaking changes. We are still discussing the best way to go about addressing this issue.

[liamdebeasi]

Closing this until we can schedule this for an upcoming release.