-
Notifications
You must be signed in to change notification settings - Fork 789
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
fix(runtime): narrow onInput & onCapture event types #3135
fix(runtime): narrow onInput & onCapture event types #3135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This change makes sense to me. Since it alters our public API by expecting non-nullable fields on for events:
interface UIEvent extends Event {
readonly detail: number;
readonly view: Window | null;
/** @deprecated */
readonly which: number;
}
interface InputEvent extends UIEvent {
readonly data: string | null;
readonly inputType: string;
readonly isComposing: boolean;
}
I'm a tad concerned about breaking changes. I would suspect most folks are using type assertions to trick the compiler, but I can't say for certain that's what's happening. Out of abundance of caution, I'm going to merge this into the v3 branch.
Thanks again!
Gotta fix a small configuration bug for CI to run, hold tight! |
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
this commit updates the typings for the analysis task. the team re-enabled these tasks/tests (for the most part) in #3263. at the same time, a commit that updates typings for onInput events was sitting in the v3.0.0-dev branch, #3135 upon merging main into v3.0.0-dev, the build broke. this commit fixes the typings to align with the latter commit.
libraries/applications using Stencil, will have the input event strongly typed to `InputEvent` instead of the general `Event` type.
this commit updates the typings for the analysis task. the team re-enabled these tasks/tests (for the most part) in #3263. at the same time, a commit that updates typings for onInput events was sitting in the v3.0.0-dev branch, #3135 upon merging main into v3.0.0-dev, the build broke. this commit fixes the typings to align with the latter commit.
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build
) was run locally and any changes were pushednpm test
) were run locally and passednpm run test.karma.prod
) were run locally and passednpm run prettier
) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
In order to remove the type casting for the
ionInput
event types, the emitted type forDomAttributes
will need to matchInputEvent
.GitHub Issue Number: ionic-team/ionic-framework#24111
What is the new behavior?
Libraries/applications using Stencil, will have the
input
event strongly typed toInputEvent
instead of the generalEvent
type.Does this introduce a breaking change?
Testing
Other information