search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.51k stars 4.2k forks source link

Interactivity API: Event directives cannot be passive #58705

Open westonruter opened 9 months ago

westonruter commented 9 months ago

Description

I have a page that has a dozen Image blocks with lightbox enabled. When I open the console, I see many many warnings:

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

[Violation] Added non-passive event listener to a scroll-blocking 'touchmove' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952

At issue here is the Image block's use of these touch event directives:

https://github.com/WordPress/gutenberg/blob/69a82aca49548d212d57fd80e3a67de60d0f58e9/packages/block-library/src/image/index.php#L297-L299

Such scroll event listeners should be passive for performance to ensure smooth interactivity. In fact, Chrome defaults these events to be passive by default (and Firefox too apparently). Safari, however, does not.

Nevertheless, there is no way currently to specify using event directives to specify that event handler should be passive. Such events should always be passive unless they call event.preventDefault(). The Image block only may call preventDefault in the touchmove handler:

https://github.com/WordPress/gutenberg/blob/69a82aca49548d212d57fd80e3a67de60d0f58e9/packages/block-library/src/image/view.js#L205-L226

Preact does not support setting the passive flag when adding event listeners via props: https://github.com/preactjs/preact/issues/428

The workaround I suppose would be to not use event directives at all for the Image block, and to instead call addEventListener at the init action.

👉 In any case, it would be great if perhaps the event directives could somehow default to being passive to better ensure best practices for performance.

Read more from MDN on using passive listeners.

Step-by-step reproduction instructions

  1. Add Image blocks to a post
  2. Enable the lightbox functionality
  3. View the post on the frontend with the console open

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

sirreal commented 2 months ago

Was this resolved by https://github.com/WordPress/gutenberg/pull/61885?

sirreal commented 2 months ago

This is related to https://github.com/WordPress/gutenberg/issues/61634.

westonruter commented 2 months ago

I'd say it is partially resolved. In #61885 passive event listeners were used for data-wp-on-async-window and data-wp-on-async-document. But for other data-wp-on-async-* directives, we cannot currently use passive event listeners due to https://github.com/preactjs/preact/issues/428. However, once Preact supports them, we could immediately make data-wp-on-async-* all passive right away.