Similar to #8968, but for the wheel
and mousewheel
events. They are now passive by default for root elements in Chrome 73 (currently beta) which means React apps that have custom scrolling/zooming behaviors will run into issues.
The quick fix may be to manually add event listeners with {passive: false}
but has the React team considered if this should be configurable for the React event handler?
Blog post from the Chrome team here: https://developers.google.com/web/updates/2019/02/scrolling-intervention
We鈥檝e definitely considered adding support for passive events and that鈥檚 on our longer term roadmap, but we鈥檇 rather not rush such an important API addition because of the Chrome intervention timings.
Do you mind creating a small repro case to verify it's broken now?
Sure thing, here's a repro:
https://codesandbox.io/s/6zn44nmjvn
In Chrome stable / Safari / etc the box will move but the rest of the page will remain static. In Chrome 73, the entire page rubber bands as you scroll around (the box also moves). Since rubber banding only happens with a trackpad, make sure to test it with one.
Also note all the red errors in the console due to the intervention.
The quick fix may be to manually add event listeners with {passive: false} [...]
I have mixed feelings on it, but I wonder if this is the best approach. We don't need to feature check for event options support either because all scroll events are captured anyway so the object coercing to true is fine:
This at least keeps the React behavior consistent while passive events are being figured out, but definitely presses the need to add more configuration to events.
For me, at least on Linux and ChromeOS, this is easier to reproduce with an actual mouse with a mousewheel (it could be that OSX emits mouse wheel events for trackpads).
I added a linear gradient to the background to better illustrate what's going on (event.preventDefault() should prevent scrolling on the parent, but it doesn't):
https://codesandbox.io/s/x3m6m17jop
Here's a vanilla example that uses { passive: false }
to demonstrate the intended behavior:
https://codepen.io/nhunzaker/pen/rREpGo?editors=1010
The solution could be to just pass { passive: false }
. I can prep a PR for that, even if we decide to hold off.
I think this goes against the wishes of the Chrome team, but it's the only solution I can think of to maintain existing behavior while a passive event API is being figured out.
Talking with @sophiebits: it sounds like we should hold off on making a change until we clamp down a passive event listener API.
We shouldn't undermine changes Chrome believes are important for improving site performance. If they have found that most scroll/wheel event listeners should be passive, that extends to React apps as well. We're shouldn't contribute to making apps slower for most cases when they don't need to be.
Until a passive event API is available to React, one way to work around this is to use a ref to attach a non-passive event listener. (Edit: this is a work in progress. See https://github.com/facebook/react/pull/15036).
I'm not super familiar with using TypeScript with hooks, but I've done my best to form @blixt's example to use a ref to attach a passive listener:
https://codesandbox.io/s/zzqxp1yvy3
I imagine others will come to the issue board confused about this change, so it'll be important to have a clear path forward for future issues. Are there ways to improve on the solution above?
We shouldn't undermine changes Chrome believes are important for improving site performance.
Chrome shouldn't be changing default options that have such a significant impact on usability. Every site that relies on default {passive:false}
behavior is now broken. Even if React pushes a fix, this is still broken for existing sites. The pace at which Chrome is willing to break standards is staggering. The passive
option wasn't in the wild until Chrome 51 (June 2016) and now the default has been changed. Consider me unsympathetic to doing what "Chrome believes is important".
@byronwall I strongly believe the Chrome team thinks very carefully and thoughtfully about the changes they make, and I don't believe it is right to call into question what they should or should not be doing without the full holistic picture of the drive behind the change and the expected outcome. It might be nice to get direct input here from someone who can provide a larger context that can help drive the path forward in a healthy way.
@cherscarlett
I strongly believe the Chrome team thinks very carefully and thoughtfully about the changes they make, and I don't believe it is right to call into question what they should or should not be doing without the full holistic picture of the drive behind the change and the expected outcome.
One must not question the existence of God because one's mind is vanishingly small compared to the mind of God and so one can't possibly ever grasp a hint of His divine majesty.
If God tells you do something you must do that without questioning or hesitation, otherwise you're a heretic and must be judged by the Holy Inquisition and later bunt alive to clean your soul of the sins and earn forgiveness because God loves all of his children.
@catamphetamine Sorry to hide your comment, but it doesn't help us come to a resolution and I want to keep this issue focused for others coming to the React issue board that might be confused.
The Chrome team didn't do this in a vacuum, and took the time to research this thoroughly, as linked in the original issue description (https://developers.google.com/web/updates/2019/02/scrolling-intervention):
Our goal with this change is to reduce the time it takes to update the display after the user starts scrolling by wheel or touchpad without developers needing to change code. Our metrics show that 75% of the wheel and mousewheel event listeners that are registered on root targets (window, document, or body) do not specify any values for the passive option and more than 98% of such listeners do not call preventDefault().
Much like React, Chrome is in a position to improve user experience across a broad base of users. Both teams are aligned in this goal, even if coordinating on a change like this could have been smoother.
Forcing wheel events to be impassive would create additional breaking changes for React users, while undermining performance improvements on the platform. Let's figure out the best API for passive event handlers in React moving forward. However in the interim, let's also come up with a good general purpose solution so that it's easier for React users to handle this change.
more than 98% of such listeners do not call preventDefault()
Translated: we're comfortable breaking 2% of those sites which currently rely on default behavior...
My main issue is with Chrome and not React. Having said that, it's sensible to "unbreak" React applications relying on previously default behavior while a firm API is proposed for declaring passive events. The push for _potential_ performance gains should be balanced against backward compatibility. In this case Chrome has overstepped. Going against their grain until the full API is in place seems a minor transgression.
Forcing wheel events to be impassive would create additional breaking changes for React users
What breaking changes are there? Until a week ago this was the default behavior. It is still the default behavior for non-Chrome browsers. Forgive me if I am missing the larger picture here?
@byronwall I can see how websites are broken anyway be it requiring the manual addition of { passive: false }
or updating React version and rebuilding the bundle: any solution requires equal developer intervention. And in many cases the devs are long gone and no one knows how stuff was built or works. So it's a really bizarre situation.
I guess it better be the { passive: false }
fix then instead of React fixing Chrome stuff.
I agree that Chrome team did indeed break some part of the internet just because they like it more: they've simply grown too confident in their software monopoly. Go Firefox, what can I say.
This is a temporary fix I've been using in the load file index.js
.
import React from 'react';
import ReactDOM from 'react-dom';
import App from './pages/App';
const EVENTS_TO_MODIFY = ['touchstart', 'touchmove', 'touchend', 'touchcancel', 'wheel'];
const originalAddEventListener = document.addEventListener.bind();
document.addEventListener = (type, listener, options, wantsUntrusted) => {
let modOptions = options;
if (EVENTS_TO_MODIFY.includes(type)) {
if (typeof options === 'boolean') {
modOptions = {
capture: options,
passive: false,
};
} else if (typeof options === 'object') {
modOptions = {
passive: false,
...options,
};
}
}
return originalAddEventListener(type, listener, modOptions, wantsUntrusted);
};
const originalRemoveEventListener = document.removeEventListener.bind();
document.removeEventListener = (type, listener, options) => {
let modOptions = options;
if (EVENTS_TO_MODIFY.includes(type)) {
if (typeof options === 'boolean') {
modOptions = {
capture: options,
passive: false,
};
} else if (typeof options === 'object') {
modOptions = {
passive: false,
...options,
};
}
}
return originalRemoveEventListener(type, listener, modOptions);
};
ReactDOM.render(<App />, document.getElementById('root'));
Is it possible we can leave the removeEventListener as is since it does only look at the capture
option (and not the passive
option) according to https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener#Matching_event_listeners_for_removal
While addEventListener() will let you add the same listener more than once for the same type if the options are different, the only option removeEventListener() checks is the capture/useCapture flag. Its value must match for removeEventListener() to match, but the other values don't.
@kheyse-oqton It's _probably_ fine, but it is generally best practice to add and remove event listeners with identical options.
This is a temporary fix I've been using in the load file
index.js
.import React from 'react'; import ReactDOM from 'react-dom'; import App from './pages/App'; const EVENTS_TO_MODIFY = ['touchstart', 'touchmove', 'touchend', 'touchcancel', 'wheel']; const originalAddEventListener = document.addEventListener.bind(); document.addEventListener = (type, listener, options, wantsUntrusted) => { let modOptions = options; if (EVENTS_TO_MODIFY.includes(type)) { if (typeof options === 'boolean') { modOptions = { capture: options, passive: false, }; } else if (typeof options === 'object') { modOptions = { ...options, passive: false, }; } } return originalAddEventListener(type, listener, modOptions, wantsUntrusted); }; const originalRemoveEventListener = document.removeEventListener.bind(); document.removeEventListener = (type, listener, options) => { let modOptions = options; if (EVENTS_TO_MODIFY.includes(type)) { if (typeof options === 'boolean') { modOptions = { capture: options, passive: false, }; } else if (typeof options === 'object') { modOptions = { ...options, passive: false, }; } } return originalRemoveEventListener(type, listener, modOptions); }; ReactDOM.render(<App />, document.getElementById('root'));
I have a issue when the listeners are parsed. I think some listeners are not eliminated from the document.body or something like this, because something is strange, at mousemove event, some functions are executed. These listeners are from stripe or braintree I guess and this running a xhr request on mousemove. When I remove your solution, it works well.
@yspektor That's a great temporary solution, though when defining modOptions you probably ought to include passive before ...options (eg. { passive: false, ...options }). That way it is still possible for third party libraries to add and remove passive listeners to and from the document as needed.
@alexratiu Try reversing the order of passive and ...options in the modOptions declaration (in both functions) and see if that solves your problem. If any third-party libraries add passive listeners to the document prior to the execution of your code, then they will be unable to remove them when needed at a later point.
This Chrome update is causing us problems. I'm building a react component library that is intended to be used by the general public, so polluting the global scope by modifying native methods on the document object is probably not going to be a valid option for us. Unfortunately resorting to native listeners doesn't work for us either since some of our components are using portals and native events don't propagate up the virtual dom through portals the same way that React events do.
Any suggestions?
All works perfectly now.
@yspektor solution is great but my fault was I put the code in componentDidMount not in index
where is invoked ReactDom.Render()
Cheers 馃憤
```
const checkType = (type, options) => {
if (!PASSIVE_EVENTS.includes(type)) return null;
const modOptions = {
boolean: {
capture: options,
passive: false,
},
object: {
...options,
passive: false,
},
};
return modOptions[typeof options];
};
const addEventListener = document.addEventListener.bind();
document.addEventListener = (type, listener, options, wantsUntrusted) => (
addEventListener(type, listener, checkType(type, options) || options, wantsUntrusted)
);
const removeEventListener = document.removeEventListener.bind();
document.removeEventListener = (type, listener, options) => (
removeEventListener(type, listener, checkType(type, options) || options)
);
It's not fair to blame Chrome at all, it's an issue with React that binds listeners to the root element, Chrome's reasoning that most of web applications (if used DOM API correctly) won't be affected.
It's important for people who reach this page to know that they have the right to demand the fix from React.
I understand the performance concerns that led React team to decide to bind listeners to root elements, but a better design would probably be this: let the developer decide whether he wants to attach the listener to root element (the default behavior) or to attach it to the original element (onClick
and onElementClick
for example)
How long will it take for the issue to be fixed? What is the priority? None of the workarounds is 100% acceptable. Writing libraries using native events will be cumbersome for plenty of reasons. I have just faced the same issue as @Spenc84
Ran into this problem with trying to prevent browser zoom on control + wheel and performing a scaling action in our app instead, since native events didn't fit our use case I ended up with the following solution.
const MyComponent = () => {
const [scale, setScale] = useState(10);
useEffect(() => {
const cancelWheel = (event) => event.preventDefault();
document.body.addEventListener('wheel', cancelWheel, {passive: false});
return () => {
document.body.removeEventListener('wheel', cancelWheel);
}
}, []);
const onWheelEvent = (event) => {
setScale(scale + event.deltaY);
};
return <div onWheel={onWheelEvent} />
};
The use effect will bind a non passive event that prevents browser zoom and cleans itself up on unmount, then leaves your react event listeners to do their thing.
@patrickmccallum I like the simplicity of this solution. But won't it end up preventDefaulting
on all wheel events, not just those handled by MyComponent
?
@patrickmccallum I like the simplicity of this solution. But won't it end up
preventDefaulting
on all wheel events, not just those handled byMyComponent
?
Yep, that's correct. It's valid in our use case since we want to be able to trigger an app zoom from anywhere on the page, but it would stop you from scrolling for example a list that overflows. To fix that you'd just attach an if statement to check if the ctrl key is down in the event in cancelWheel.
// Prevent browser zooming
const cancelWheel = (event: WheelEvent): void => {
if (event.ctrlKey) {
event.preventDefault();
}
};
I haven't looked into it much, but this could actually now be fixed in React 17 as per this blog post.
Most helpful comment
Chrome shouldn't be changing default options that have such a significant impact on usability. Every site that relies on default
{passive:false}
behavior is now broken. Even if React pushes a fix, this is still broken for existing sites. The pace at which Chrome is willing to break standards is staggering. Thepassive
option wasn't in the wild until Chrome 51 (June 2016) and now the default has been changed. Consider me unsympathetic to doing what "Chrome believes is important".