Closed renchap closed 7 years ago
@renchap,
Thanks for opening this issue. I agree that any warning Chrome provides should be taken seriously. Here is more information on passive event listeners, If you are interested.
react-cursor-position supports the "pan" gesture, for touch environments (pan is analogous to dragging with a mouse). To achieve the pan gesture react-cursor-position (RCP) temporarily prevents page scrolling while the gesture is active. To temporarily disable page scrolling in Chrome 56+, RCP must explicitly specify two event listeners with the passive option set to false. I think that is what triggers the warning in Chrome 59, which you identified. RCP only sets passive false for the element it renders, so the impact to overall page scrolling performance should be minimal. Only a scroll gesture that starts over the element rendered by RCP would be affected.
Please let me know your thoughts.
As I have more than 200 ReactCursorPosition
components in one page, this generated quite a lot of logs :)
If there is no other way of doing this, then it is ok but I wonder why they display this violation when (from the page you posted) 20% of the event handlers can not be made passive ...
Thanks for your input. You raise an interesting point about the warning.
200 instances of react-cursor-position is the highest count on one page that I am aware of. Without exposing any intellectual property, do you mind my asking what your use case is?
Hi @renchap, I'm closing this for now, due to inactivity. Thanks again for raising the issue. I think it is great to have the subject captured here on the project.
@ethanselzer maybe it is possible to somehow suppress these warnings, or optionally disable the "pan" gesture (or any other gesture, in general) for those of us who doesn't need it? I get that the warnings we get are most likely false positives, but it would be nice to get rid of them for the sake of having a tidy console output (I think it's much easier to spot real warning when the output is not cluttered with these warnings)
@flood4life - Thanks for your input. I agree that a noisy console is a poor development experience. I'm sorry about that. The "pan" gesture is a core touchscreen feature of react-cursor-posion. Having an option to disable it would be like disabling support for touchscreens, which does not seem like a good trade off to me.
I have not seen the warnings, so I'm wondering, are you running Chrome in verbose logging mode? And do you get the warnings in a mouse environment?
One thing you could try is to disable passive event listeners in Chrome by pasting the following in the address bar and then toggling the appropriate select menu: chrome://flags/#document-passive-event-listeners
.
Please let me know your thoughts. Thanks!
@ethanselzer yeah, those warnings appear on the verbose logging level. I get those warnings in mouse environment (it's Chrome 61 on an iMac running High Sierra). Disabling that flag only produces more warnings (2 from EventListener.js:31 and 2 from Container.js:94) in addition to the 4 from addEventListener.js:2.
I agree that disabling pan gesture is effectively disabling touch interface, but perhaps there are some use cases where pan (or touch) support is not needed. E.g., I'm using it to track cursor position for displaying a tooltip in a chart. When the mouse moves, the tooltip smoothly follows it. I don't think that pan gesture makes sense in this context.
So maybe it makes sense to allow to opt-out from touch environment support, or from some of the gestures?
@flood4life - Thanks for the information. I was able to reproduce the issue. I am concerned about developer experience, but I'm reluctant to create an option that does not have a runtime benefit.
What do you think about the idea of filtering out the messages by control clicking one of them and selecting Filter > Hide messages from main.[hash].js?
Note: I had a look at Chrome Canary (v63.x) and I do not get the warnings in that version with verbose logging selected. Perhaps the messages will not be carried forward with newer versions of Chrome.
Please let me know your thoughts. Thanks!
@ethanselzer I don't see the warnings in Canary either, so you're probably right about this -- maybe the issue here is the unnecessary warning.
For now, filtering those warnings should suffice. Thanks!
@flood4life - You are welcome! I'm glad we found a solution. If you find react-cursor-position useful, please consider staring it on GitHub. Staring helps to grow the project, which benefits all who use it.
@ethanselzer actually, I've realized this solution doesn't really work at least in Create React App default environment. Chrome can only hide messages from bundle.js
, and, well, it just hides all the messages (including error messages).
So I guess it's ok to just wait for the new Chrome version.
P.S. Just starred the repo!
@flood4life - Thanks for your feedback. I'm sorry the hiding approach is too aggressive in the create-react-app environment. If you don't mind, please try entering /^(?!addEventListener.js).*/
in the filter field (left of the log level menu).
Thanks very much for starring the project. I really appreciate it!
@ethanselzer This does work properly, however, I changed the regex to include the actual line from the original message: /^(?!addEventListener.js:2).*/
.
The only downside is that the regex needs to be entered again if the tab is closed. It doesn't happen too often, though, so I guess this is the optimal solution for now.
ping @renchap -- I guess this might be of use for you!
On Chrome 59 with verbose logging, this violation appears in the console:
[Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive.
I have not looked further than this yet, but Chrome tips are often a good idea to investigate.