-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Description
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
- Introduce
withSyncEvent
and trigger deprecations when usingevent
without it (via proxiedevent
object). This is essentially to prepare the ecosystem for the async first change. - Have all actions run async, unless they use
withSyncEvent
. We can consider still passing the proxiedevent
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 asdata-wp-on
. In other words, there's no longer a reason to usedata-wp-on-async
. - The same applies to the other
data-wp-on*
variants forwindow
anddocument
.
- At the same time, we also start treating
- Trigger deprecations warnings when using
data-wp-on-async
and itswindow
anddocument
equivalents.- The documentation on
data-wp-on-async
should be removed (or at least clearly marked as outdated/deprecated).
- The documentation on
- (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
Labels
Type
Projects
Status