Skip to content
This repository has been archived by the owner on Jul 11, 2019. It is now read-only.

add / removeEventListener passive options? #11

Closed
wadackel opened this issue Oct 6, 2016 · 4 comments
Closed

add / removeEventListener passive options? #11

wadackel opened this issue Oct 6, 2016 · 4 comments

Comments

@wadackel
Copy link
Contributor

wadackel commented Oct 6, 2016

Thank you for awesome library!

I want to improve the performance of window.onscroll. must use the addEventListener passive option for that.

There are several proposals in the options of how to specify, what do you think?

Pattern 1: Wrapped function

// App.js
import EventListener, { withOptions } from "react-event-listener";

class Example extends Component {
  handleScroll(e) {
    // ...
  }

  render() {
    return (
      <EventListener
        target={window}
        onScroll={withOptions(this.handleScroll, {capture: false, passive: true})}
      />
    );
  }
}

Pass a handler in object form, there is a pattern to specify the options.
I believe that this pattern is good.

  • Pros
    • In the future it can be extended if there was additional flag.
    • It can be implemented with backwards compatibility.
  • Cons
    • ??

Pattern 2: Capture like

<EventListener
  target={window}
  onScrollPassive={this.handleScroll}
/>
  • Pros
    • Simple syntax & API.
    • It can be implemented with backwards compatibility.
  • Cons
    • Additional flag is broken and there. (Example: onScrollCapturePassiveFooBar)

Pattern 3: Pass in component props

<EventListener
  passive
  target={window}
  onScrollPassive={this.handleScroll}
/>
  • Pros
    • Simple syntax & API.
    • In the future it can be extended if there was additional flag.
  • Cons
    • passive options would be across all of the handler. (Example: onResize)

Side note: Polyfill sample of passive options

IE8 for the simple sample has been excluded.

// Check compatible
let supportsPassive = false;

try {
  const options = Object.defineProperty({}, 'passive', {
    get() {
      supportsPassive = true;
    }
  });
  window.addEventListener('test', null, options);
} catch (e) {}

// Usage
function on(target, type, handler, options) {
  let optionsOrCapture = options;

  if (!supportsPassive) {
    optionsOrCapture = options.capture;
  }

  target.addEventListener(type, handler, optionsOrCapture);
}

function off(target, type, handler, options) {
  // ...
}
@oliviertassinari
Copy link
Owner

IMHO, the pattern 1 is far better. It could also be something like:

      <EventListener
        target={window}
        onScroll={{
          handler: this.handleScroll,
          options: {
            capture: false,
            passive: true,
         }
        }
      />

@wadackel
Copy link
Contributor Author

wadackel commented Oct 6, 2016

Implementation of withOptions was thinking the following such simple things.

export function withOptions(handler, options) {
  return { hanlder, options };
}

I think that your code may be simple !

@oliviertassinari
Copy link
Owner

oliviertassinari commented Oct 6, 2016

@tsuyoshiwada In that case, I don't see the harm in having a withOptions helper 👍 .
I'm not sure users will us it. But anyway.

@wadackel
Copy link
Contributor Author

wadackel commented Oct 6, 2016

Thanks!
It is also likely to be easy to extend by cutting as a Function 😄

It sends a PR After implementing this!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants