-
Notifications
You must be signed in to change notification settings - Fork 290
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: CustomEvent generic type #1759
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
Test failure here looks to be unrelated:
|
Yep, don't worry about that. it's a known issue with the windows-debug job |
JSG_TS_OVERRIDE(<T = any> extends Event { | ||
constructor(type: string, init?: CustomEventCustomEventInit); | ||
get detail(): T; | ||
}); |
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.
Apologies for commenting after this was merged, but I think this could be simplified to:
JSG_TS_OVERRIDE(<T = any> extends Event { | |
constructor(type: string, init?: CustomEventCustomEventInit); | |
get detail(): T; | |
}); | |
JSG_TS_OVERRIDE(<T = any> { | |
get detail(): T; | |
}); |
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.
Can confirm that produces the same types output:
export class CustomEvent<T = any> extends Event {
constructor(type: string, init?: CustomEventCustomEventInit);
get detail(): T;
}
Opened a quick PR to improve: #1766
Thanks for catching!
Fixes #1504
This overrides the
CustomEvent
types to better match other implementations.This is my first time contributing to
workerd
, and really using JSG at all, so apologies if there's anything weird with this change. If there's a better way to do this, please just let me know.Old output:
New output:
TypeScript
lib.dom.d.ts
reference: https://github.com/microsoft/TypeScript/blob/1d6d962d3132a901f5fb48be4a389c45faf5a74e/src/lib/dom.generated.d.ts#L5935