-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Support Passive Event Listeners #6436
Comments
This landed in Chrome 51. Is there any updated plan to support this in React? :O |
How is this possible if React has only one event listener on document, and then delegates to others? |
What's the current status of issue with Passive Events ? |
I just hit a warning in chrome about handling the wheel event, which could be optimized if it were registered as a passive event handler. So having this in React would be neat! |
You'll also want to handle arbitrary options, such as |
FWIW, Facebook listens to active wheel events to block outer scrolling when sidebars or chat windows are scrolled. We can't implement the UI without it. We still want to support this as an option but the problem space is still incomplete so there might evolve alternative solutions to this problem that doesn't involve passive event listeners. So it is still an active design space. |
It's important to keep both active listeners and add support passive ones. Little suggestion: <SomeElement
onScroll={this.onScrollThatCallsPreventDefault}
onScrollPassive={this.onScrollThatJustListens}
...this.props
/> |
@romulof yeah, this is how you register events on the capture phase as well <SomeElement
onClick={this.onClick}
onClickCapture={this.onClickCapture}
onScrollPassive={this.onScrollPassive}
/> so I imagine this would be the proper API to support passive events as well. Side note: a tricky question is - how would you register passive events for the capture phase? I suppose this is not possible, by the nature of passive events. Since they are even not allowed to call |
@radubrehar, |
:) It's not the case, since there are no passive events on the capture phase. |
Sure it doesn't make sense, but I would't count on it. There's also other types event binding, such as Another suggestion: <SomeElement
onScroll={this.onScrollThatCallsPreventDefault}
/>
<SomePassiveElement
onScroll={{
passive: true,
capture: true,
handler: this.onScrollThatJustListens,
}}
/> This way React would have to detect whether the event handler is a function (normal binding), or and object containing binding options and the handler function. |
I think the object approach with options makes more sense than Another approach that comes to mind would be to attach properties to the event handler to allow them to opt out of passive mode (or toggle other options). Something like this: class Foo extends React.Component {
constructor() {
this.handleScroll = this.handleScroll.bind(this);
this.handleScroll.passive = false;
}
handleScroll() {
...
}
render() {
return <div onScroll={this.handleScroll} />;
}
} In theory, this would work pretty nicely with decorators, once they land. |
Thinking about this a little more, I think it would be better to add an event options property to the function, instead of individual options. That would allow React to only have to worry about one property instead of potentially many. So, to adjust my example above: class Foo extends React.Component {
constructor() {
this.handleScroll = this.handleScroll.bind(this);
this.handleScroll.options = { passive: false };
}
handleScroll() {
...
}
render() {
return <div onScroll={this.handleScroll} />;
}
} Another thought that occurred to me is what might this look like if we modified JSX syntax in a way that allowed for these options to be passed in via the JSX. Here's a random example that I haven't put much thought into: return <div onScroll={this.handleScroll, { passive: false }} />; I've also been thinking about whether events should be passive by default or not, and I'm a bit on the fence. On one hand, this would certainly be nice for events like scroll handlers, but I worry that it would cause too much turbulence and unexpected behavior for many click handlers. We could make it so some events are passive by default and others are not, but that would probably just end up being confusing for folks, so probably not a good idea. |
This way is pretty similar to what I proposed earlier, without modifying JSX syntax. return <div onScroll={{ handler: this.handleScroll, passive: true }} />; And documentation would be straightforward: div.propTypes = {
...
onScroll: React.PropTypes.oneOf([
React.PropTypes.func,
React.PropTypes.shape({
handler: React.PropTypes.func.isRequired,
capture: React.PropTypes.bool,
passive: React.PropTypes.bool,
once: React.PropTypes.bool,
}),
}; |
Are react events passive by default? It seems to be that way for touch events, at least. I am not able to |
@joshjg React handlers are passed "synthetic events," which are sort of like native events, but different. By the way, someone with more knowledge should correct what I'm about to say because I haven't actually read the code that does this. I'm not super familiar with the implementation details, but I know that With function stopPropagation (e) {
e.stopPropagation();
e.nativeEvent.stopImmediatePropagation();
} [MDN] This got slightly off the main topic, but the short answer is that React doesn't use passive events, they're just sometimes handled in a strange order. |
@joshjg @benwiley4000 @gaearon Recently the chrome team has changed their approach to document-level touch events, making them passive by default. And since React attaches events at document-level, you get this new behaviour. See https://www.chromestatus.com/features/5093566007214080 This has indirectly changed they way React behaves - I suppose React does not explicitly mention I just hit this as well - so you need to register touch events by hand, with addEventListener |
Something like this has been suggested but here are a couple more ideas. function MyComponent() {
function onScroll(event) { /* ... */ }
onScroll.options = {capture, passive, ...};
return <div onScroll={onScroll} />;
} This one would let you easily opt into passive events or capture events without the need for a breaking change. However, I was intrigued by the idea of a passive by default event listener. I remember preventDefault being a major obstacle (among others) that blocks React from running in a worker. function MyComponent() {
function onScroll(event) { /* ... */ }
onScroll.shouldPreventDefault = (event): boolean => {
// some logic to decide if preventDefault() should be called.
}
onScroll.shouldStopPropagation = (event): boolean => {
// some logic to decide if stopPropagation() should be called.
}
return <div onScroll={onScroll} />;
} It would be hard to ensure this doesn't become a breaking change, but if this was enforced, all the code to decide if an event needed to be |
Until this is resolved I think it would be best if references to |
I wonder about the implications of the change of the event delegation from React v17. Lighthouse has a rule https://web.dev/uses-passive-event-listeners/ that test against non-passive events. Previously, Reproduction: https://codesandbox.io/s/material-demo-forked-e2u72?file=/demo.js, live: https://csb-e2u72.netlify.app/ |
Yeah. Seems concerning. I'll file a new issue. |
Filed #19651 for React 17 discussion. |
Touch support in FDT v2 is wonky. There's multiple issues here, all of which are fixed with this commit: ## problem 1 We're no longer passing down the `touchEnabled` prop to the cell renderers within FDT. But the resizer plugin `<ResizeCell />` still expects it, making it not work for touch devices. ## problem 2 The touch/mouse related event listeners were treated as passive. This is a problem with React not yet having an API for clients to specify event listener options. FDT relies on stopping propagation or preventing default behavior of these events in various places, and they did not work as expected. I'm fixing this by manually registering the handlers via `addEventListener` with the `passive` property turned OFF. See facebook/react#6436 ## problem 3 Sometimes, the first touch events don't seem to work unless the user does a follow-up click. One particular case is when the user clicks on the reorder knob; FDT will render a "drag proxy" which replaces the original cell thus making the original touch event (which relies on the original cell to be in the document tree) to not work properly. The docs explain this nicely: https://developer.mozilla.org/en-US/docs/Web/API/Touch/target > The read-only target property of the Touch interface returns the `EventTarget` on which the touch contact started when it was first placed on the surface, even if the touch point has since moved outside the interactive area of that element or even been removed from the document. **Note that if the target element is removed from the document, events will still be targeted at it, and hence won't necessarily bubble up to the window or document anymore.** If there is any risk of an element being removed while it is being touched, the best practice is to attach the touch listeners directly to the target. ## problem 4 `<ReorderCell />` accidentally renders the children twice in some cases because we accidentally passed its' children to its child renderer... Check this comment -- #706 (comment) -- for details.
Touch support in FDT v2 is wonky. There's multiple issues here, all of which are fixed with this commit: ## problem 1 We're no longer passing down the `touchEnabled` prop to the cell renderers within FDT. But the resizer plugin `<ResizeCell />` still expects it, making it not work for touch devices. ## problem 2 The touch/mouse related event listeners were treated as passive. This is a problem with React not yet having an API for clients to specify event listener options. FDT relies on stopping propagation or preventing default behavior of these events in various places, and they did not work as expected. I'm fixing this by manually registering the handlers via `addEventListener` with the `passive` property turned OFF. See facebook/react#6436 ## problem 3 Sometimes, the first touch events don't seem to work unless the user does a follow-up click. One particular case is when the user clicks on the reorder knob; FDT will render a "drag proxy" which replaces the original cell thus making the original touch event (which relies on the original cell to be in the document tree) to not work properly. The docs explain this nicely: https://developer.mozilla.org/en-US/docs/Web/API/Touch/target > The read-only target property of the Touch interface returns the `EventTarget` on which the touch contact started when it was first placed on the surface, even if the touch point has since moved outside the interactive area of that element or even been removed from the document. **Note that if the target element is removed from the document, events will still be targeted at it, and hence won't necessarily bubble up to the window or document anymore.** If there is any risk of an element being removed while it is being touched, the best practice is to attach the touch listeners directly to the target. ## problem 4 `<ReorderCell />` accidentally renders the children twice in some cases because we accidentally passed its' children to its child renderer... Check this comment -- schrodinger/fixed-data-table-2#706 (comment) -- for details.
https://github.com/WICG/EventListenerOptions/blob/gh-pages/explainer.md
It would be good to have everything be passive by default and only opt-in to active when needed. E.g. you could listen to text input events but only preventDefault or used controlled behavior when you have active listeners.
Similarly, we could unify this with React Native's threading model. E.g. one thing we could do there is synchronously block the UI thread when there are active listeners such as handling keystrokes.
cc @vjeux @ide
The text was updated successfully, but these errors were encountered: