Chrome Lighthouse Audit results - Does not use passive listeners to improve scrolling performance

[lpmi-13]

Describe the bug
While running a lighthouse audit in Chrome, I'm seeing the "Does not use passive listeners to improve scrolling performance" warning, and it's giving me a 7 point hit on the best practices metric.

I'm assuming there might be some ability to pass this explicitly during the set up of the event handlers, but I was unable to find anywhere in the documentation or source code that pointed me in the right direction.

The component I'm using makes use of the useSwipeable hook rather than the class component, if that matters.

It's also quite possible that I should ignore this aspect of the lighthouse audit.

Steps or Sandbox to reproduce
run lighthouse audit on https://ipinder.netlify.com/

Expected behavior
I'm assuming there's a simple way to force the event handler to use passive:true, but I was unable to find how to do it in the docs. I would like to avoid this warning in the lighthouse audit.

** Device/Browser **
Chrome 79.0.3945.130
react": "^16.12.0"
react-dom": "^16.12.0"
react-router-dom": "^5.1.2"
react-swipeable": "^5.5.0"

[lpmi-13]

I'm hoping this can be resolved with a simple PR to update the documentation, which I'm more than happy to do...just need a nudge in the right direction.

[hartzis]

Heyo @lpmi-13 , thanks for setting up the issue. I was able to run lighthouse against https://ipinder.netlify.com/ and see the "7 point hit".

I was also able to replicate this on the react-swipeable docs page and it does appear to be react-swipeable as the culprate.

This is incredibly frustrating that goolge, via lighthouse, have taken such intense measures towards this issue.

I remember dealing with this passive listener issue a lot in v4 and now it comes back to haunt me, lol.

This is something we could tackle a solution for in v6 and I will add it to the project board.

In the meantime, If you would like to make a PR to add some documention and notes about this issue to the readme please do.

NOTES:

• Look into solving this similiar to react-use-gesture
  • https://github.com/react-spring/react-use-gesture/blob/ee2380274090f822789eaa34e7cfaf65fbd15c22/src/Controller.ts#L131-L136

[hartzis]

Add some more details and information in PR #170

• fix: [#167] - passive event listener #170 (comment)

[hartzis]

Effecting dependent packages too:
xiaolin/react-image-gallery#510

[nosovk]

if you need fast solution you can reffer to patched swiper version
#184
there is PR with passive listeners added and additional build script to allow direct installation from github.
just change from

"react-swipeable": "^5.5.1",

to

"react-swipeable": "github:FormidableLabs/react-swipeable#pull/184/head",

in yours package.json

or add it to project using

npm install github:FormidableLabs/react-swipeable#pull/184/head

[hartzis]

@nosovk Thanks for the intermediary solution to help people.

We are at work on v6 that will include a fix/update for this too.

• V6 #185

[hartzis]

Update/Thoughts

Currently leaning towards mimicking react-use-gesture's great work and api and creating a new option for domTarget

• https://use-gesture.netlify.app/docs/options#domtarget

This has a few benefits that I see:

• gives the consumer of react-swipeable full control over when/if they'd like to preventDefault to block a scrolling
  • This would additionally require new option(s) for event options, again similiar to react-use-gesture's eventOptions
    • https://use-gesture.netlify.app/docs/options#generic-options, see eventOptions.passive and eventOptions.capture
• Opens up the possibility of applying react-swipeable to the window/document more easily
  • this has been a feature request in the past that could open a lot of possibilities to track swipes at the root of a page

v6 proposal

const myRef = React.useRef(null)
// This will add a "swipedUp" listener the div
const bind = useScroll({
    onSwiping: ({ event }) => event.preventDefault(),
    onSwipedUp: () => console.log('swiped up, and scrolling prevented'),
  },
  // maybe move config/options to second argument two?
  {
    domTarget: myRef,
    eventOptions: { passive: false }
})

React.useEffect(bind, [bind])

return <div ref={myRef} />

v5 current

const handlers = useSwipeable({
  onSwiped: (eventData) => eventHandler,
  preventDefaultTouchmoveEvent: true,
 })
return (<div {...handlers}> You can swipe here </div>)

Cons:

• kind of a lot more work to support "just canceling" a page scroll when tracking swipes

---

💬 Would love any thoughts/feedback on this approach.

[nosovk]

in common there is only a one possible misuse - actually calling event.preventDefault() without additional configuration of eventOptions. Chrome will warn about incorrect usage of preventDefault in console.
But default passive:true can cause problems during update to v6. Especially if preventDefault wrapped in some conditional logic.
Writing more code is not a problem, but explaining when to add additional config is a challenge. Ideal is to provide some eslint plugin that will warn about executing preventDefault without passive: false option

[hartzis]

Writing more code is not a problem, but explaining when to add additional config is a challenge.

@nosovk Great call out.

In v4 of react-swipeable the code handled when to apply passive: false when preventDefaultTouchMove was true.

react-swipeable/src/Swipeable.js

Lines 112 to 116 in 637ef02

	setupTouchmoveEvent() {
	if (this.element && this.hasPassiveSupport) {
	this.element.addEventListener('touchmove', this.eventMove, { passive: false });
	}
	}

We could go that route again.

Benefits here:

• keep the same property users may be used to
• avoid having to describe complex scenarios and use cases to do what react-swipeable could abstract for the end user inside itself

[hartzis]

📓 Related to issue #180

[hartzis]

We have a tentative solution with #210.

Ran the lighthouse report and went from 26 -> 27, so it appears lighthouse is not even dinging us as much as it used to when we saw as much as 7 points.

[hartzis]

Hopefully this is fixed with #185 . If anyone stumbles on this with v6 lets open a new issue.