WebReflection / hyperHTML

A Fast & Light Virtual DOM Alternative
ISC License
3.06k stars 112 forks source link

passive events support #321

Closed ikabirov closed 5 years ago

ikabirov commented 5 years ago

Is it possible to use passive for scroll/touchstart events? Now we have chrome violation: [Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952

yuretz commented 5 years ago

You could always do it in non-declarative way of course, by using node references returned by wire() or via document.querySelectorAll(). If you want to do it declaratively, you could probably use define() to create a custom attribute intent where you would pass an event handler and call your element.addEventListener(..., {passive:true}) from inside the intent callback. But in this case I'm not sure where would you call the removeEventListener() from... Maybe @WebReflection could shed some light on what would be the proper way of implementing it, once he's back from holiday?

yuretz commented 5 years ago

If there was an optional, second clean-up callback parameter in the define(), that would have been a good place to remove the event listeners added in the first one...

ikabirov commented 5 years ago

Use passive events by default looks like a good idea. It's good for application performance. But original html events attributes always non passive. Anyway, let's wait for @WebReflection

WebReflection commented 5 years ago

passive by default is not an option because that is not the HTML/DOM default (least surprise), but the hint about the intent sounds reasonable to me:

hyperHTML.define(
  'passive-events',
  (node, info) => {
    for (var key in info) {
      if (key.slice(0, 2) === 'on') {
        var method = info[key] ? 'add' : 'remove';
        node[method + 'EventListener'](key.slice(2), info[key], {passive: true});
      }
    }
    return '';
  }
);

With that, you can have:

html`<div passive-events=${{
  ontouchstart: callbackOrContextOrNull,
  onscroll: callbackOrContextOrNull
}} />`

that makes configuration of events pretty trivial, as long as callbacks/context/values are not generated at runtime, otherwise you need a WeakMap to relate the node, and store previous values there and do the add/remove dance only when previous values are different.

I hope you got the idea.

ikabirov commented 5 years ago
  var method = info[key] ? 'add' : 'remove';
  node[method + 'EventListener'](key.slice(2), info[key], {passive: true});

its doesn't work. When remove we must have old function reference, not null.

I hope you got the idea.

yep, thanks!

WebReflection commented 5 years ago

you are right, then it's a bit more complicated, but not too much.

const wm = new WeakMap;
hyperHTML.define(
  'passive-events',
  (node, info) => {
    var saved = wm.get(node) || wm.set(node, {}).get(node);
    for (var key in info) {
      if (key.slice(0, 2) === 'on' && saved[key] !== info[key]) {
        var type = key.slice(2);
        var options = {passive: true};
        if (saved[key])
          node.removeEventListener(type, saved[key], options);
        saved[key] = info[key];
        if (saved[key])
          node.addEventListener(type, saved[key], options);
      }
    }
    return '';
  }
);
ikabirov commented 5 years ago

got it!