jquery-mousewheel
jquery-mousewheel copied to clipboard
Please make non-passive event listeners
I believe I'm seeing a warning error in Google Chrome saying:
[Violation] Added non-passive event listener to a scroll-blocking 'wheel' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952
This is pointing to the following code line: https://github.com/jquery/jquery-mousewheel/blob/fcb425e872c5b1eb45cbc4c8a5dcf4865b991bab/jquery.mousewheel.js#L40
This requires changes to jQuery, planned for jQuery 4.0.0 right now. See https://github.com/jquery/jquery-mousewheel/issues/168.
@mgol Do you have a link to an open issue, so we can track the progress. The one you linked to is closed and all the other issues linking to that one are also closed. I know you guys would have an open issue with a Milestone tag of 4.0.0
in the jQuery repo somewhere?
That’s the jQuery issue: https://github.com/jquery/jquery/issues/2871
This is an interesting case, since my proposed workaround employs special event handlers and the mousewheel plugin uses those already. You could make the mousewheel listeners passive, but currently it would force all jQuery mousewheel listeners on the same node to be passive regardless of whether they wanted it or not. It seems like most of the time there would not be multiple listeners though. So it might make sense for the mousewheel plugin to detect support for passive listeners if they're available.
@dmethvin Lots of mousewheel
use cases implement custom scroll (something akin to fullPage) so we can't just use passive handlers if available. If we wanted, we could allow users to set an option that would switch the behavior. I'll reopen the issue to reconsider.
If we wanted, we could allow users to set an option that would switch the behavior.
I think that would be the best solution.
For our CMS we use the mousewheel to scroll through a bunch of menu options. When doing some testing and setting the event listeners to passive it did reduce some jank issues and improve performance. So it would be great to have an option to turn it on/off as per use case.
Any thoughts on the best interface to do this? There is no way in the jQuery special-event system to receive its own arguments via .on()
. The easiest way is to define an event namespace with special meaning, for example:
// Attach a non-passive handler to #myBox, but only if
// it doesn't have a passive mousewheel event already
$("#myBox").on("mousewheel", function(e) { ... });
// Attach a passive handler to #yourBox, but only if
// it doesn't have a non-passive mousewheel event already
$("#yourBox").on("mousewheel.passive", function(e) { ... });
This makes the .passive
namespace magic for mousewheel
events, it has no special meaning to jQuery itself or to any other event type.
Another possibility is to pass it in the data
argument and hope no mousewheel handlers are using or want to use that feature in a conflicting way.
// Attach a passive handler to #yourBox, but only if
// it doesn't have a non-passive mousewheel event already
$("#yourBox").on("mousewheel", { passive: true }, function(e) { ... });
I think the first option is a minor version bump and the second option is a major version bump due to our co-opting the user's data
object in a potentially incompatible way.
+1 for the method:
// Attach a passive handler to #yourBox, but only if
// it doesn't have a non-passive mousewheel event already
$("#yourBox").on("mousewheel.passive", function(e) { ... });
I was playing around with your example: https://jsbin.com/bupesajoza/edit?html,js,output
Works well with no jank at all.
Stupid question, will this method be v3.4.1 and v4.0.0 compatible ?
It's hard to tell at this point how many breaking changes the event subsystem will have in jQuery 4.0 although we'll try to minimize them as much as possible within our constraints. It will certainly be compatible with 3.4.1; jQuery 4.0 will have a different native way to register passive event handlers & it's possible the mousewheel plugin will need adjustments, we'll see.
@dmethvin I like the namespace proposal, although technically it's a breaking change as someone may have been using such a namespace already. Maybe we should bump the major?
BTW, just to clarify - whatever solution we choose here, it's going to be slightly clunky. For a plugin it's not a huge deal, though - we can always do a breaking change & bump the major if we deem that necessary. With jQuery it's more difficult as it affects the whole broad ecosystem so we try to avoid temporary hacky APIs there. Whatever may work in the mousewheel plugin would not necessarily work in jQuery Core.
Sounds all good to me, we use Semantic Versioning for everything anyway. I guess when v4.0.0 comes out you could post two versions on the readme for this plugin just to let people know.
I think it shouldn't be an issue to support both jQuery 3 & 4 with the same plugin version; it's just that we're not sure yet if we will need any changes in the plugin to be compatible with jQuery 4.
Any updates on this? The error/warning also shows in Lighthouse (google dev)
Does not use passive listeners to improve scrolling performance
Consider marking your touch and wheel event listeners as `passive` to improve your page's scroll performance. Learn more.
This solve to me:
Line 40: this.addEventListener( toBind[ --i ], handler, { passive: false } );