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

DOM Event Mount Target Considerations #13713

Open
philipp-spiess opened this Issue Sep 23, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@philipp-spiess
Copy link
Collaborator

philipp-spiess commented Sep 23, 2018

Given that we’re considering a rewrite of the event system and are thinking about attaching events to the React root, I thought it would be fitting to explore all our options on where to mount the event listeners a little and combine all the knowledge that is scattered across the repo.

In general, there are three candidates for attaching event listeners:

  1. Document Listeners
  2. React Root Listeners
  3. Element Listeners

Every option comes with shortcomings and I want to summarize what we’ve learned over the years.

Document Listeners

Historically, React always listened at the document level and implemented a synthetic event system to simulate capture and bubble phases inside the React tree. Most event listeners are listening at the bubble phase which means that users can still add capture-level document listeners and see them fire before React will process the event.

Additionally, not all events bubble in the DOM. To support bubbling for all events, React sometimes eagerly adds event listeners (media events, for example) or listens to the capture phase instead.

While it usually works to leave the React event system and attach native listeners when needed, there are certain caveats that come with that. One example is that calling .stopPropagation() on a capture document-level listener has no effect (#12518, #285 (comment)). Another implication of this is that interoperability between other React roots or third-party frameworks will behave unexpectedly (#8117, #8693).

Some browsers have certain optimizations in place that make handling of document listeners complicated (Safari is not properly bubbling when document listeners are used #12717, #12989 and Chrome doesn’t respect preventDefault() on touch start at the document level #11530 (comment)).

We’re also never cleaning up document-level listeners (#7128).

There are certain benefits of this solution as well. Our current event system can use the same "event bus" to implement polyfills that require document based listeners. Implementation of portal-bubbling is easier because we only need to ensure that we’re already listening at the document of the portal target (more on that later).

React Root Listeners (#2043)

One solution to the issues outlined above is the use of React roots as the mount target for events. This would still rely on event delegation and would require a synthetic event system.

Root level listeners would certainly help make bubbling more robust when using multiple React instances since we no longer add all listeners at the same level. This will, however, only work for bubbling. Some events use capturing (scroll, focus, blur, cancel, and close) and would fire in an inverse bubble order (#8117 (comment), #12919 (comment)). Capture handlers between multiple roots will also always fire in the wrong order.

Portal bubbling will also become more complicated since we have to find out if the portal root is inside the react-root to see if we need to attach listeners to the portal root as well. Consider the following example, where we need to add listeners to the #react-root as well as the #portal-root:

<body>
  <div id="react-root"></div>
  <div id="portal-root"></div>
</body>

And compare it with this example, where we don’t need that:

<body>
  <div id="react-root">
    <div id="portal-root"></div>
  </div>
</body>

I’ve compiled a list of previous implementation attempts and the issues that were pointed out as well:

Date PR Issues
Oct 2016 #8117
Jun 2016 #7088
Aug 2014 #2050

It’s probably possible to work around some/all of the issues. The invalid capturing order can be worked around by adding both a bubble and a capture listener for regular events and then only trigger the appropriate phase. The shims can probably be rewritten and if they need the document, additional listeners could be added. iOS tap highlight could be disabled via CSS. To get rid of some of the edge cases around events that don’t bubble in the browser, we should consider deprecating support for this all together.

New Features

Since we’re taking the time to rethink the event system, I also want us to think about: Passive Event Listeners (#6436) and Web Components (#7901, #9242).

I think we can (albeit with an additional implementation effort) support passive event systems while keep using event delegation: We’d add two different listeners (one for capturing and one for bubbling) and handle them as completely different events.

Support for shadow roots is a bit more complex as event delegation doesn’t really make sense there. We can’t easily consider the shadow root the same as a React root or a portal root since we can’t rely on adding listeners to the #react-root if it’s the parent of a shadow root. Consider the following case:

<div id="react-root">
  <!-- Listening on #react-root will catch events inside the #portal-root -->
  <div id="portal-root"></div> 
  <!-- Listening on #react-root will NOT catch events inside the #shadow-root -->
  <my-component id="shadow-root"></my-component>
</div>

There’s a handy comparison of Custom Elements support of different JavaScript framework and React can definitely do better. With the planned changes to the event system, we should work on that.

Element Listeners (sort of #4751)

There’s a more radical approach to changing the event system and that is to get rid of event delegation altogether. This is what happens in Preact, react-dom-lite, Vue.js, and probably other frameworks as well (We should research Ember and Angular at this point).

In this case, it’s trivial to add support for passive event listeners and bubble/capture will behave as expected. It also allows us to completely remove the need of the synthetic event system as we could rely on the browser for this. We also know that some browsers can better optimize certain events if they are attached directly to the element (#1254, #1964).

I can think of at least two major roadblocks for this, though:

  1. Performance
    Event delegation avoids the need to addEventListener() for every element with an event handler. We should figure out how significant this overhead really is - especially while considering the possible bundle size improvement that should make initial-render faster again. This needs to be compared at a real-world application.

    While doing some very rough benchmarking (It’s a microbenchmark so the outcomes are not representative for real apps) of an event-system less React, I noticed that there’s another feature of event delegation that is often overlooked: setState batching. Right now, React knows when all event listeners are called and can batch setState calls until this is the case. I looked a bit into a potential callback mechanism that would fire when all events are processed and before the next frame is drawn but I don’t think this is possible in all browsers. We have to evaluate the performance implications on a real-world application to see how bad this is and how often setState batching is occurring (we could still batch inside the same event listener).

  2. Portal Bubbling
    The big issue I see with portal bubbling is that we still need to use event delegation on the portal roots to catch events that are only listened to in parents. In the write-up about React Fire (#13525), @gaearon mentioned that we could use re-dispatching of native events but that would still require us to set up the listeners first.

Further Considerations

  • The whole synthetic event system currently accounts for ~20% of the React DOM bundle size. Back in July I worked on an example that removed everything event related and added a tiny Preact-like system to find out how much we can save. I didn’t look into edge cases too much but I already got most of the tests to pass.
  • In general we might also consider adding an API for installing global DOM event handlers (#285) since the upcoming concurrent mode will make it more complicated to listen to arbitrary non React-controlled events (because of them possibly being fired during render time slices). This discussion is out of scope for this write-up though but maybe good to keep in mind.

That’s all I have for now. I’m especially curious for ways how we could implement portal bubbling when using element listeners - I’d say this is the biggest uncertainty I have right now. 🙂

@nhunzaker

This comment has been minimized.

Copy link
Collaborator

nhunzaker commented Oct 21, 2018

This is a really wonderful write-up!

We should figure out how significant this overhead really is - especially while considering the possible bundle size improvement that should make initial-render faster again.

This is a great consideration. The load/parse time of the code against the execution cost. I think it'll be important to create a good series of benchmark fixtures, though I worry about how much effort it will take to produce good ones. Does anyone know of existing projects they could be based on?

I noticed that there’s another feature of event delegation that is often overlooked: setState batching. Right now, React knows when all event listeners are called and can batch setState calls

I'm not very familiar with async rendering, however I wonder if batching setState with event listeners remains useful in that mode (assuming medium/low priority setState calls are getting batched anyway). Is this a problem that could eventually go away?

Dispatching inside the same event listener seems straight forward. I'd like to know what event types often get batched together.


I am most excited about local event listener attachment. It seems like it avoids the most browser issues and that the challenges of making it work are within React. We have the most control over those problems.

@amanmahajan7 amanmahajan7 referenced this issue Dec 5, 2018

Merged

Cell Tooltip - Focus Issues #1422

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment