Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passive Listener Violation In Chrome #15140

Closed
stefanpenner opened this issue Apr 14, 2017 · 13 comments · Fixed by #19227
Closed

Passive Listener Violation In Chrome #15140

stefanpenner opened this issue Apr 14, 2017 · 13 comments · Fixed by #19227
Labels

Comments

@stefanpenner
Copy link
Member

[Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive.

Our event dispatcher always attaches this events, regardless if they are used or not: https://github.com/emberjs/ember.js/blob/master/packages/ember-views/lib/system/event_dispatcher.js#L62-L65

We should most likely lazily install them as they are required.

@stefanpenner stefanpenner changed the title Passive Listener Violation Passive Listener Violation In Chrome Apr 14, 2017
@evanaj
Copy link

evanaj commented May 26, 2017

I faced the same issue. How can I solve it?

@wellington1993
Copy link

I have the same problem:

[Violation] Added non-passive event listener to a scroll-blocking 'mousewheel' event. Consider marking event handler as 'passive' to make the page more responsive.
16:29:39.142

@wellington1993
Copy link

Is it a related question: #12783 ???

@krisselden
Copy link
Contributor

@wellington1993 you can remove events via https://emberjs.com/api/classes/Ember.Application.html#property_customEvents and if your app needs these events use passive listeners for mousewheel, touchstart, touchmove (only events that can block scroll via preventDefault matter for passive listening), you add on didInsertElement and remove on willDestroyElement of a component.

@pixelhandler
Copy link
Contributor

@evanaj @krisselden @stefanpenner @wellington1993 is this still an issue, perhaps we should close; what do you think?

@btecu
Copy link
Contributor

btecu commented Jan 31, 2019

@pixelhandler this is still an issue with the latest version:
image

@st-h
Copy link
Contributor

st-h commented Aug 21, 2019

It seems that ember >= 3.11 now provides a way to add passive event listeners via the on modifier: https://blog.emberjs.com/2019/07/15/ember-3-11-released.html

@rwjblue
Copy link
Member

rwjblue commented Aug 22, 2019

Confirm, though it doesn't yet avoid adding the non-passive listener to body (really the rootElement).

@filipe-torres
Copy link

filipe-torres commented Oct 11, 2019

My approach to solve this problem was add event on didInsertElement and remove them on willDestroyElement lifecycle functions, setting {passive: true} as last parameter when adding the event listener:

export default Component({

  didInsertElement() {
    this._super(...arguments);
    this.functionName = () => { 
        // function body
    };
    this.element.addEventListener('wheel', this.functionName, {passive: true});
  },

  willDestroyElement() {
    this._super(...arguments);
    this.element.removeEventListener('wheel', this.functionName);
  }
});

@pzuraq
Copy link
Contributor

pzuraq commented Oct 11, 2019

@filipe-torres this is essentially what {{on}} does, and we highly recommend you use the modifier over component lifecycle hooks, since those no longer exist in the Octane programming model.

kturney added a commit to kturney/ember-intl that referenced this issue Jan 30, 2020
now `[Violation] Added non-passive event listener to a scroll-blocking X event. Consider marking event handler as 'passive' to make the page more responsive.` won't get logged during tests.
see emberjs/ember.js#15140
jasonmit pushed a commit to ember-intl/ember-intl that referenced this issue Jan 31, 2020
* update to modern ember-qunit setup

ran `ember-qunit-codemod convert-module-for-to-setup-test`

* only use ember-cli-shims on neccessary ember versions

fixes deprecation warning `ember.globals-resolver` https://deprecations.emberjs.com/v3.x#toc_ember-deprecate-globals-resolver

* fix passive listener violations in chrome

now `[Violation] Added non-passive event listener to a scroll-blocking X event. Consider marking event handler as 'passive' to make the page more responsive.` won't get logged during tests.
see emberjs/ember.js#15140
bors added a commit to rust-lang/crates.io that referenced this issue Apr 2, 2020
Disable global, non-passive `touchstart` and `touchmove` event listeners

see emberjs/ember.js#15140

r? @locks
@rwjblue
Copy link
Member

rwjblue commented Oct 16, 2020

We need to figure out the canonical list of events that issue this warning.

Some additional references:

What we need is an RFC that is exactly like emberjs/rfcs#486 but for these events. For reference, I think that the work that @simonihmig did implementation wise in #18214 is exactly what we need to do...

@simonihmig
Copy link
Contributor

What we need is an RFC that is exactly like emberjs/rfcs#486 but for these events

The events in that RFC were a bit tricky as they were non-bubbling events that as such don't play well with EventDispatcher's event delegation approach and as such were harder to support. But touchstart, touchmove and whatever other events might be related to this issue don't have this problem. Rather the issue here is that (non-passive) event listeners are attached eagerly, even if those events are not actually used in Ember components.

So, @rwjblue what do you think about not deprecating these, but rather attaching event listeners lazily only when those events are actually used, i.e. reviving your previous work in #17911?

I would guess that this could practically solve the problem here, as in ~99% of apps (my own wild guess), the offending events are not used (at least in that old Ember.Component event method way). And if they are used in the app, you can easily rewrite the usage to attach event listeners manually (similar to RFC486's transition path) - which would enable the lazy approach to not attach non-passive event listeners.

@rwjblue
Copy link
Member

rwjblue commented Oct 19, 2020

Ya, that definitely seems fine to me. Might want to make only these lazy (since I suspect most apps won't use them) be making all of them lazy.

Ultimately, I expect all of the event dispatcher code to be gone when @ember/component is no longer used...

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

Successfully merging a pull request may close this issue.