types(runtime-dom): improve event types #13804
Conversation
WalkthroughRefined event type annotations in Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/runtime-dom/src/jsx.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (3)
packages/runtime-dom/src/jsx.ts (3)
1333-1333: onEncrypted → MediaEncryptedEvent is correct for HTMLMediaElement.Accurately models Encrypted Media Extensions; good improvement.
Similar to SubmitEvent, MediaEncryptedEvent’s availability depends on the consumer’s lib.dom version. If Vue’s supported TS range already guarantees it, we’re good; otherwise consider documenting the minimum TS/lib.dom version in the release notes for this change.
1341-1341: onProgress → ProgressEvent is correct.Matches media/XHR progress semantics and exposes loaded/total, etc.
1299-1303: Confirmed TypeScript ≥4.4 (root ~5.6.2) supports InputEvent & SubmitEvent
- The root package.json pins TypeScript to “~5.6.2”, which is well above the 4.4 threshold when
SubmitEventwas introduced to lib.dom (stackoverflow.com).InputEventhas been a standard part of lib.dom for several TS releases and is included in TS 5.6.2 as well.All form/input event typings (
InputEventforonBeforeinput/onInput,SubmitEventforonSubmit) are safe to use.
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Size ReportBundles
Usages
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@cszhjh Thanks for the PR. I think we can refer to the following type definition. // /Applications/Visual Studio Code.app/Contents/Resources/app/extensions/node_modules/typescript/lib/lib.dom.d.ts
interface GlobalEventHandlersEventMap {
"abort": UIEvent;
"animationcancel": AnimationEvent;
"animationend": AnimationEvent;
"animationiteration": AnimationEvent;
"animationstart": AnimationEvent;
"auxclick": PointerEvent;
"beforeinput": InputEvent;
"beforematch": Event;
"beforetoggle": ToggleEvent;
"blur": FocusEvent;
"cancel": Event;
"canplay": Event;
"canplaythrough": Event;
"change": Event;
"click": PointerEvent;
"close": Event;
"compositionend": CompositionEvent;
"compositionstart": CompositionEvent;
"compositionupdate": CompositionEvent;
"contextlost": Event;
"contextmenu": PointerEvent;
"contextrestored": Event;
"copy": ClipboardEvent;
"cuechange": Event;
"cut": ClipboardEvent;
"dblclick": MouseEvent;
"drag": DragEvent;
"dragend": DragEvent;
"dragenter": DragEvent;
"dragleave": DragEvent;
"dragover": DragEvent;
"dragstart": DragEvent;
"drop": DragEvent;
"durationchange": Event;
"emptied": Event;
"ended": Event;
"error": ErrorEvent;
"focus": FocusEvent;
"focusin": FocusEvent;
"focusout": FocusEvent;
"formdata": FormDataEvent;
"gotpointercapture": PointerEvent;
"input": Event;
"invalid": Event;
"keydown": KeyboardEvent;
"keypress": KeyboardEvent;
"keyup": KeyboardEvent;
"load": Event;
"loadeddata": Event;
"loadedmetadata": Event;
"loadstart": Event;
"lostpointercapture": PointerEvent;
"mousedown": MouseEvent;
"mouseenter": MouseEvent;
"mouseleave": MouseEvent;
"mousemove": MouseEvent;
"mouseout": MouseEvent;
"mouseover": MouseEvent;
"mouseup": MouseEvent;
"paste": ClipboardEvent;
"pause": Event;
"play": Event;
"playing": Event;
"pointercancel": PointerEvent;
"pointerdown": PointerEvent;
"pointerenter": PointerEvent;
"pointerleave": PointerEvent;
"pointermove": PointerEvent;
"pointerout": PointerEvent;
"pointerover": PointerEvent;
"pointerrawupdate": Event;
"pointerup": PointerEvent;
"progress": ProgressEvent;
"ratechange": Event;
"reset": Event;
"resize": UIEvent;
"scroll": Event;
"scrollend": Event;
"securitypolicyviolation": SecurityPolicyViolationEvent;
"seeked": Event;
"seeking": Event;
"select": Event;
"selectionchange": Event;
"selectstart": Event;
"slotchange": Event;
"stalled": Event;
"submit": SubmitEvent;
"suspend": Event;
"timeupdate": Event;
"toggle": ToggleEvent;
"touchcancel": TouchEvent;
"touchend": TouchEvent;
"touchmove": TouchEvent;
"touchstart": TouchEvent;
"transitioncancel": TransitionEvent;
"transitionend": TransitionEvent;
"transitionrun": TransitionEvent;
"transitionstart": TransitionEvent;
"volumechange": Event;
"waiting": Event;
"webkitanimationend": Event;
"webkitanimationiteration": Event;
"webkitanimationstart": Event;
"webkittransitionend": Event;
"wheel": WheelEvent;
} |
|
@edison1105 Thank you for your guidance, it has been really helpful to me. I truly appreciate it. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/runtime-dom/src/jsx.ts (1)
1307-1307: onError should be Event for element-level handlers (not ErrorEvent)For element attributes like
, , etc., GlobalEventHandlersEventMap specifies Event. ErrorEvent is for Window/Worker “error”. Keep this as Event to avoid misleading fields (e.g., filename, lineno).
Apply this diff:
- onError: ErrorEvent + onError: Event
🧹 Nitpick comments (3)
packages/runtime-dom/src/jsx.ts (3)
1315-1317: Prefer deriving click-family types from lib.dom instead of hard-coding PointerEventUsing GlobalEventHandlersEventMap avoids drift across TS versions and documents intent clearly. It still resolves to PointerEvent in current lib.dom while remaining future-proof.
Suggested diff:
- onAuxclick: PointerEvent - onClick: PointerEvent - onContextmenu: PointerEvent + onAuxclick: GlobalEventHandlersEventMap['auxclick'] + onClick: GlobalEventHandlersEventMap['click'] + onContextmenu: GlobalEventHandlersEventMap['contextmenu']Verification:
- Ensure CI’s TypeScript lib.dom maps these to PointerEvent (as referenced in the discussion).
- Spot-check that handlers typed as
(e: MouseEvent) => voidcontinue to be accepted (bivariance should preserve compatibility, but worth a quick dts test).
1328-1328: Derive abort type from GlobalEventHandlersEventMap for consistencyUIEvent matches current lib.dom for
aborton elements, but referencing the map keeps us aligned if upstream changes.Suggested diff:
- onAbort: UIEvent + onAbort: GlobalEventHandlersEventMap['abort']
1341-1341: Good narrowing: onProgress as ProgressEvent; consider deriving from lib.domTyping to ProgressEvent is correct. For consistency with the approach above, you may want to pull from GlobalEventHandlersEventMap.
Suggested diff:
- onProgress: ProgressEvent + onProgress: GlobalEventHandlersEventMap['progress']
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/runtime-dom/src/jsx.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (3)
packages/runtime-dom/src/jsx.ts (3)
1299-1299: Good change: onBeforeinput now typed as InputEventAligns with lib.dom and prevents over-generic Event usage. No concerns.
1302-1302: Good change: onSubmit now typed as SubmitEventMatches GlobalEventHandlersEventMap and modern browser typings.
1333-1333: Looks correct: “encrypted” as MediaEncryptedEventThis aligns with HTMLMediaElement event typing. Please confirm our supported TS versions include
MediaEncryptedEventin lib.dom (typical in modern TS). If older TS versions are supported, consider a minimal dts test to guard.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
I found two more event type mismatch according to MDN input event submit event interface Event {
// ...
onInput: InputEvent;
onSubmit: SubmitEvent;
// ...
}BTW, I came across with a workaround by modifying interface Event {
// ...
onInput: Event | InputEvent;
onBeforeinput: Event | InputEvent;
// ...
}
type EventHandlers<E> = {
[K in keyof E]?: E[K] extends (...args: any) => any ? E[K] : DistributedEventHandler<E[K]>;
};
type DistributedEventHandler<TEvent> = TEvent extends any ? (payload: TEvent) => void : never;so that both flavor of event handlers are supported, maximize the compatibility: <template>
<input @input="legacy" @beforeinput="brandNew" />
<template>
<script setup lang="ts">
function legacy(ev: Event) { }
function brandNew(ev: InputEvent) { }
</script> |
For the other events marked in 13796 but not modified in this PR, I’m not sure whether they should also be updated. According to MDN, they were previously typed as
MouseEvent, and I’m uncertain if changing them toPointerEventcould cause compatibility issues. For example: Click Event.close #13796
Summary by CodeRabbit