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

Move MouseWheel event type detection to BrowserEventConstants #11585

Closed
nhunzaker opened this issue Nov 17, 2017 · 5 comments
Closed

Move MouseWheel event type detection to BrowserEventConstants #11585

nhunzaker opened this issue Nov 17, 2017 · 5 comments

Comments

@nhunzaker
Copy link
Contributor

I think we can move the wheel event support check in BrowserEventEmitter into BrowserEventConstants

That runs the check once, which is nice for wheel and scroll events, which eventually need to be attached locally.

Filing this for myself as a part of #11550. But this could happen on master right now.

More or less, I'm interested in cutting the overhead of event listening as much as possible, and this is really low hanging 🥝.

@nhunzaker
Copy link
Contributor Author

Marked this as a good first issue. The event system can be a beast. I'd be happy to walk through it with an interested contributor.

@cristidrg
Copy link
Contributor

Hello @nhunzaker I'd like to work on this issue.

Any other files you suggest looking at other than the two that you have linked?

@nhunzaker
Copy link
Contributor Author

Totally! If you were curious:

The main consumer of ReactBrowserEventEmitter is ReactDOMFiberComponent. It asks ReactBrowserEventEmitter to listen to events.

Most events are delegated to the document for the related DOM element. So ReactDOMFiberComponent uses an internal function that finds that document before requesting an event to be subscribed.

ReactBrowserEventEmitter then figures out what events need to be attached for a particular event type. This is how React polyfils things like mouseenter and mouseleave, and sets up a bunch of normalization around change events for inputs.

From there, the Emitter enumerates through all of the event types that need to be attached and says "Have I already attached this? If not: start listening!".


That gets to the change I'm suggesting. This function is doing some aliasing of event names for different browsers that I think shouldn't happen at this phase. We can normalize wheel ahead of time by assigning the correct wheel event type in BrowserEventConstants. We might also be able to remove a few others, but that might not be true. Let's take it one step at a time :).

@gaearon
Copy link
Collaborator

gaearon commented Nov 19, 2017

#11594

@nhunzaker
Copy link
Contributor Author

Fixed by #11594. Thanks @cristidrg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants