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

Add CustomEvent to EventListener #1535

Closed
wants to merge 2 commits into from
Closed

Add CustomEvent to EventListener #1535

wants to merge 2 commits into from

Conversation

mkcode
Copy link

@mkcode mkcode commented Apr 1, 2023

Type definition for EventListener and EventListenerObject is overly narrow and does not allow for CustomEvent.

Submitting this PR on behalf of issue: microsoft/TypeScript#28357

MDN and WhatWG seem to specify that this is allowed:

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2023

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

@mkcode
Copy link
Author

mkcode commented Apr 1, 2023

@saschanaz - Following up on your comment: microsoft/TypeScript#28357 (comment)

I believe that extending the EventMap interfaces would not solve the issue for generic EventTargets.

This way does, but may break other things.

@saschanaz
Copy link
Contributor

Event | CustomEvent is equivalent to CustomEvent, and thus this makes every unknown event interfaces as CustomEvents. IMO that's too aggressive, as:

  1. Any "custom events" also can use the base Event instead of CustomEvent or any custom subclasses of Event
  2. Any new event added in new versions of browsers will collide with the CustomEvent definition (Event as AnySubclassedEvent works but CustomEvent as AnySubclassedEvent does not)

I believe that extending the EventMap interfaces would not solve the issue for generic EventTargets.

You should still be able to add your own addEventListener signature in that case and follow the pattern of the existing types.

@mkcode
Copy link
Author

mkcode commented Apr 1, 2023

Yep. This is much more complicated than the DX that I want of just specifying the parameter type of the callback function:

object.addEventListener('my-event', (event: CustomEvent<CustomType>) => { event.detail }))

But, I think you are right that what I wrote is actually just a bad TypeScript pattern. The type definition belongs in the object definition using the methods you described, not in callback handlers.

Thank you. I appreciate your time and detailed response!

@mkcode mkcode closed this Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants