ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.95k stars 13.52k forks source link

bug: Angular - Argument of type 'Event' is not assignable to parameter of type 'CustomEvent<XXX>'. #24245

Open JumBay opened 2 years ago

JumBay commented 2 years ago

Prerequisites

Ionic Framework Version

Current Behavior

Wrong definition type breaks the app.

Expected Behavior

Having the right definition type

Steps to Reproduce

Copy/paste the code sample (html and ts) from https://ionicframework.com/docs/api/reorder-group in tab1.page.(html|ts). Then in your IDE you'll notice the given error:

Argument of type 'Event' is not assignable to parameter of type 'CustomEvent'. Type 'Event' is missing the following properties from type 'CustomEvent': detail, initCustomEventngtsc(2345)

Try to build the app: ionic build --prod, it won't build (same error).

It does this with other events as well...

Same issue occurs on ionic 6 rc3

Code Reproduction URL

https://github.com/JumBay/ionic-event-type-issue

Ionic Info

$ ionic info [WARN] Error loading @capacitor/ios package.json: Error: Cannot find module '@capacitor/ios/package'

   Require stack:
   - /usr/local/lib/node_modules/@ionic/cli/lib/project/index.js
   - /usr/local/lib/node_modules/@ionic/cli/lib/index.js
   - /usr/local/lib/node_modules/@ionic/cli/index.js
   - /usr/local/lib/node_modules/@ionic/cli/bin/ionic

[WARN] Error loading @capacitor/android package.json: Error: Cannot find module '@capacitor/android/package'

   Require stack:
   - /usr/local/lib/node_modules/@ionic/cli/lib/project/index.js
   - /usr/local/lib/node_modules/@ionic/cli/lib/index.js
   - /usr/local/lib/node_modules/@ionic/cli/index.js
   - /usr/local/lib/node_modules/@ionic/cli/bin/ionic

Ionic:

Ionic CLI : 6.18.1 (/usr/local/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 5.9.1 @angular-devkit/build-angular : 12.1.4 @angular-devkit/schematics : 12.1.4 @angular/cli : 12.1.4 @ionic/angular-toolkit : 4.0.0

Capacitor:

Capacitor CLI : 3.3.2 @capacitor/android : not installed @capacitor/core : 3.3.2 @capacitor/ios : not installed

Utility:

cordova-res : not installed globally native-run : 1.5.0

System:

NodeJS : v14.18.1 (/usr/local/bin/node) npm : 6.14.15 OS : macOS Monterey

Additional Information

No response

yanbodang commented 2 years ago

I've got the same issue as well.

sean-perkins commented 2 years ago

Thank you for reporting this issue. I'm able to reproduce your issue.

This appears to be a flaw in how we proxy outputs events from our web components into Angular's outputs/observables. Making use of fromEvent is causing Angular Language Service to take the type signature of addEventListener instead of our custom type declarations for our events.

You can temporarily avoid this error by disabling strict templates in your tsconfig.json.

{
  "angularCompilerOptions": {
    "strictTemplates": false
  }
}

alternatively you can type cast to satisfy the compiler:

doReorder(ev: Event) {
  const customEvent = ev as CustomEvent<ItemReorderEventDetail>;
  // replace ev usages with customEvent
}

We'll post back when we discover a better approach for those generated proxies.

sean-perkins commented 2 years ago

Hello @JumBay and @EbenDang, happy Friday!

I wanted to share that we have a resolution in the works (#24314) and are committed to resolve this issue. Unfortunately this resolution results in a breaking change for a less common pattern of event binding. We are getting very close to our Ionic 6 release and don't want to introduce a breaking change this close to release.

We will be following up after Ionic 6 is released to evaluate the full impact of #24314 and explore any changes that could result in a minor change.

JumBay commented 2 years ago

Great, Thanks

DeezCashews commented 2 years ago

Temporary solution if you want to leave strict template type checking enabled is to wrap $event in $any(). Example:

TS:

  doReorder(ev: CustomEvent<ItemReorderEventDetail>) {
    console.log('Dragged from index', ev.detail.from, 'to', ev.detail.to);
  }

Template:

<ion-reorder-group (ionItemReorder)="doReorder($any($event))" disabled="true">
sean-perkins commented 2 years ago

Hello everyone! I'm very excited to share a dev-build that I believe resolves this issue:

6.1.11-dev.11655500065.1d345d4c

Would appreciate any testing and confirmation with that build. It takes advantage of our new type declarations available through Stencil and an Angular workaround (hack) to offer compatibility with the Angular Language Service without emitting duplicate events.

In the OP's scenario, you may need to use the new event types to get full compatability:

import { ItemReorderEventDetail, IonReorderGroupCustomEvent } from '@ionic/core';

doReorder(ev: IonReorderGroupCustomEvent<ItemReorderEventDetail>) {
 // existing code
}

Areas I would explicitly like confirmation on:

  1. Angular Language Service (type checking in your IDE) no longer errors/throws exceptions
  2. No build errors in your application
  3. Events are emitted as expected (no duplicate events)
  4. Application functions correctly for local development and in a production build

I will continue to do more testing in my world after the long weekend, but am super excited to finally get this issue resolved.

Thanks!

hakimio commented 2 years ago

@sean-perkins is the fix part of v6.1.11 release?

sean-perkins commented 2 years ago

@hakimio no, the PR to address this issue is still in a draft state: #25502.

With the changes required to resolve the issue, I'm giving the community plenty of time to test with the dev-build and help verify, before merging and releasing.

geo242 commented 1 year ago

Any update on this?

hakimio commented 1 year ago

@sean-perkins v7 is going to be released soon. Any plans to incorporate fix for this issue in v7?

sean-perkins commented 1 year ago

@hakimio I'm hopeful that we can find a sustainable solution in the v7.x.x release, but it will not be included in the v7.0.0 release.

The proposed solution that I had found resolves this specific issue, was leveraging private APIs that the Angular team advised we do not use (high risk of the component wrappers breaking during a minor version of Angular). We have plans to coordinate our requirements with the Angular team, to collaborate on a solution together. Stencil, Lit, Vanilla WC and other WC solutions all have a similar need to be compatible within an Angular project (or at least have a good developer experience).

tgrux commented 4 months ago

Any update on this getting into a release? I'm still seeing this issue today, though work arounds work fine.