Skip to content
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: stricter type condition for EventHandlers #6855

Merged
merged 1 commit into from Nov 8, 2022

Conversation

johnsoncodehk
Copy link
Member

@johnsoncodehk johnsoncodehk commented Oct 11, 2022

Issue: johnsoncodehk/volar#1985

With @types/node version >= 18.8.1, MouseEvent extends Function is true, but with version <= 18.8.0 MouseEvent extends Function is false.

Close #6899

sxzz
sxzz approved these changes Oct 12, 2022
antfu
antfu approved these changes Oct 12, 2022
@antfu antfu added the ready to merge The PR is ready to be merged. label Oct 12, 2022
@d-koppenhagen
Copy link

Will this fix also be applied for Vue 2.7.x ?

@sodatea sodatea added feat: types p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. labels Oct 14, 2022
@acidjazz

This comment was marked as spam.

@johnsoncodehk
Copy link
Member Author

@acidjazz We may need cautious to check if the change leads to some regressions, so please be patient. :)

Downgrading @types/node to 18.8.0 for now is fine.

@@ -1301,7 +1301,7 @@ export interface Events {
}

type EventHandlers<E> = {
[K in keyof E]?: E[K] extends Function ? E[K] : (payload: E[K]) => void
[K in keyof E]?: E[K] extends (...args: any) => any ? E[K] : (payload: E[K]) => void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is IMO a good change anyways as there is even a eslint rule to warn using Function at all

@sodatea sodatea removed the p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. label Oct 19, 2022
@sodatea
Copy link
Member

sodatea commented Oct 20, 2022

FYI the issue has been fixed in @types/node v18.11.1+ DefinitelyTyped/DefinitelyTyped#62782
so you don't have to wait for this PR to be merged to update your @types/node version.

@iBobik
Copy link

iBobik commented Oct 23, 2022

@sodatea I have 18.11.3 but the error is there.

@yyx990803 yyx990803 merged commit bad3f3c into vuejs:main Nov 8, 2022
9 checks passed
@johnsoncodehk johnsoncodehk deleted the patch-3 branch Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: types ready to merge The PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Breaks with @types/node@18.8.5, Type '...' is not assignable to type 'Event'. ts2322
9 participants