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 the typescript type definition for fire #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stof
Copy link
Contributor

@stof stof commented Oct 4, 2021

Allowing any EventTarget matches the Flow type definitions

Allowing any `EventTarget` matches the Flow type definitions
@dgraham
Copy link
Owner

dgraham commented Oct 5, 2021

Because EventTarget is so general, it requires casting in event listeners to use it, so this definition is intentionally specific to only dispatch events on elements. Other target objects can always call dispatchEvent directly. What type of object do you want to pass in as a target to fire?

@stof
Copy link
Contributor Author

stof commented Oct 5, 2021

@dgraham this is about the type in fire, not in listeners. And using EventTarget is already what is being done in Flow type definitions since years. You requested that change yourselves in #32 (comment)

And even if fire is more specific, it does not avoid the need for casting in listeners anyway, as it does not change the type of event.target

@stof
Copy link
Contributor Author

stof commented May 19, 2022

@dgraham any chance to merge that ?

@stof
Copy link
Contributor Author

stof commented Jun 1, 2023

@dgraham any news here

Copy link

@devlopersabbir devlopersabbir left a comment

Choose a reason for hiding this comment

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

I think it should be merged.

@stof
Copy link
Contributor Author

stof commented Sep 26, 2024

@dgraham is there any chance to get this merged ?

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.

3 participants