Skip to content

Introduce withSyncEvent and require Interactivity API actions that use the event object to use it #64944

@felixarntz

Description

@felixarntz

This issue is the result of #64729. It is the first issue of a few sequential ones, with the goal to automatically run Interactivity API store actions asynchronously (i.e. after yielding to the main thread) whenever possible.

What problem does this address?

Currently, the decision whether to run an action asynchronously has to be made by the person using the action in a directive (by using e.g. data-wp-on-async instead of data-wp-on). This is cumbersome, as that decision can be made purely based on the action implementation itself, independently of how it is used in a directive. The decision for whether an action should be run after yielding to the main thread should be handled by the action itself.

Additionally, the concept of yielding to the main thread is still relatively new, and many developers are not too familiar with it. Leaving the decision up to developers will therefore likely not yield (pun intended) to much adoption. On the other hand, knowing whether the action uses the event object is a very simple thing that every JS developer should understand. And because yielding to the main thread is universally a good thing, for better performance it makes sense to automatically do that whenever possible (which is the case unless synchronous methods from the event objects are used in the action).

What is your proposed solution?

Updated as of 2024-11-19:

A withSyncEvent helper should be introduced that should then be adopted by every Interactivity API store actions that use the event object in a way that requires synchronous access (e.g. to call event.preventDefault()). This will eventually allow to yield to the main thread automatically for every single action, unless the action uses withSyncEvent.

Since the latter is technically a breaking change, we'll need to first deprecate synchronous usage of event without using withSyncEvent. We can do so by proxying the event object. If the action does not yet use withSyncEvent but calls one of the synchronous event methods, the event proxy object should trigger deprecation warnings.

Code example for how withEvent could be used:

import { store, withSyncEvent } from '@wordpress/interactivity';

const { state, actions } = store( 'test', {
    actions: {
        toggleSomething() {
            // Logic to contextually toggle an element.
        } ),
        checkFormSubmission: withSyncEvent( (event) => {
            event.preventDefault();

            // Other logic.
        } ),
    },
    // ...
} );

See #64729 (reply in thread) for the original idea.

For reference, this would trigger a deprecation warning going forward, since withSyncEvent is not used even though the action uses event:

import { store } from '@wordpress/interactivity';

const { state, actions } = store( 'test', {
    actions: {
        checkFormSubmission: (event) => { // Not allowed, unless the action is wrapped with `withSyncEvent`.
            event.preventDefault();

            // Other logic.
        },
    },
} );

Overall plan

  1. Introduce withSyncEvent and trigger deprecations when using event without it (via proxied event object). This is essentially to prepare the ecosystem for the async first change.
  2. Have all actions run async, unless they use withSyncEvent. We can consider still passing the proxied event object to other functions, but now the synchronous calls would simply fail with an error, so passing it would be mostly for better DX (more clear error messaging where code still does it wrong).
    • At the same time, we also start treating data-wp-on-async just the same as data-wp-on. In other words, there's no longer a reason to use data-wp-on-async.
    • The same applies to the other data-wp-on* variants for window and document.
  3. Trigger deprecations warnings when using data-wp-on-async and its window and document equivalents.
    • The documentation on data-wp-on-async should be removed (or at least clearly marked as outdated/deprecated).
  4. (Optional:) Remove data-wp-on-async (and the other async directives) entirely.
    • Marking this as optional, since would not be a big maintenance burden to maintain the deprecated state, and we should probably only remove them if/once we're convinced that usage has pretty much phased out completely.

This issue is only about the 1. step from the list above.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status

    Done 😃

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions