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

Introduce withSyncEvent action wrapper utility and proxy event object whenever it is not used #68097

Open
wants to merge 26 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
60448fe
Implement withSyncEvent action wrapper utility.
felixarntz Dec 18, 2024
dba93ec
Prepare Interactivity API infrastructure for awareness of action prio…
felixarntz Dec 18, 2024
5201460
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Jan 2, 2025
dad7083
Proxy event object when withSyncEvent() is not used.
felixarntz Jan 2, 2025
524bf72
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Jan 21, 2025
21e51be
Ensure generator functions using withSyncEvent() are wrapped correctl…
felixarntz Jan 21, 2025
a582f01
Update Interactivity API documentation to reference withSyncEvent().
felixarntz Jan 21, 2025
5d68fcb
Use withSyncEvent() in all built-in actions that require it.
felixarntz Jan 21, 2025
d5de596
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Jan 22, 2025
621a9c9
Minor fixes for withSyncEvent docs.
felixarntz Jan 22, 2025
8a15886
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Jan 29, 2025
ec3d662
Clarify documentation.
felixarntz Jan 29, 2025
b183fcd
Merge branch 'add/64944-with-sync-event' of github.com:WordPress/gute…
felixarntz Jan 29, 2025
bf3dbcc
Enhance withSyncEvent implementation and ensure the sync flag is main…
felixarntz Jan 29, 2025
2dd6eab
Add doc block for wrapEventAsync().
felixarntz Jan 29, 2025
682ee6f
Use more specific types for event proxy handler.
felixarntz Jan 29, 2025
dff811c
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Feb 10, 2025
f0fa92a
Amend callback in withSyncEvent instead of wrapping it.
felixarntz Feb 10, 2025
21f96c4
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Feb 12, 2025
524b30d
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Feb 13, 2025
2fbfd0e
Revert "Prepare Interactivity API infrastructure for awareness of act…
felixarntz Feb 13, 2025
0dbd15f
Update evaluate() to no longer invoke functions (except where needed …
felixarntz Feb 13, 2025
5a6eca3
Export withSyncEvent
luisherranz Feb 18, 2025
dc5fc98
Merge branch 'trunk' into add/64944-with-sync-event
felixarntz Feb 18, 2025
d2a990a
Fix evaluate to return scoped function and always reset scope.
felixarntz Feb 18, 2025
a02b873
Update custom directives for e2e tests to account for evaluate behavi…
felixarntz Feb 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions docs/reference-guides/interactivity-api/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,8 @@ const { state } = store( 'myPlugin', {
} );
```

You may want to add multiple such `yield` points in your action if it is doing a lot of work.

As mentioned above with [`wp-on`](#wp-on), [`wp-on-window`](#wp-on-window), and [`wp-on-document`](#wp-on-document), an async action should be used whenever the `async` versions of the aforementioned directives cannot be used due to the action requiring synchronous access to the `event` object. Synchronous access is required whenever the action needs to call `event.preventDefault()`, `event.stopPropagation()`, or `event.stopImmediatePropagation()`. To ensure that the action code does not contribute to a long task, you may manually yield to the main thread after calling the synchronous event API. For example:

```js
Expand All @@ -885,16 +887,17 @@ function splitTask() {

store( 'myPlugin', {
actions: {
handleClick: function* ( event ) {
handleClick: withSyncEvent( function* ( event ) {
event.preventDefault();
yield splitTask();
doTheWork();
},
} ),
},
} );
```

You may want to add multiple such `yield` points in your action if it is doing a lot of work.
You may notice the use of the [`withSyncEvent()`](#withsyncevent) utility function in this example. This is necessary due to an ongoing effort to handle store actions asynchronously by default, unless they require synchronous event access (which this example does due to the call to `event.preventDefault()`). Otherwise a deprecation warning will be triggered, and in a future release the behavior will change accordingly.


#### Side Effects

Expand Down Expand Up @@ -1253,6 +1256,43 @@ store( 'mySliderPlugin', {
} );
```

### withSyncEvent()

Actions that require synchronous access to the `event` object need to use the `withSyncEvent()` function to wrap their handler callback. This is necessary due to an ongoing effort to handle store actions asynchronously by default, unless they require synchronous event access. Therefore, as of Gutenberg `TODO: Add release number here!` / WordPress 6.8 all actions that require synchronous event access need to use the `withSyncEvent()` utility wrapper function. Otherwise a deprecation warning will be triggered, and in a future release the behavior will change accordingly.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The TODO here can only be addressed once this is ready for merge and we know which Gutenberg version this will be released in.


Only very specific event methods and properties require synchronous access, so it is advised to only use `withSyncEvent()` when necessary. The following event methods and properties require synchronous access:

* `event.currentTarget`
* `event.preventDefault()`
* `event.stopImmediatePropagation()`
* `event.stopPropagation()`

Here is an example, where one action requires synchronous event access while the other actions do not:

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

store( 'myPlugin', {
actions: {
// `event.preventDefault()` requires synchronous event access.
preventNavigation: withSyncEvent( ( event ) => {
event.preventDefault();
} ),

// `event.target` does not require synchronous event access.
logTarget: ( event ) => {
console.log( 'event target => ', event.target );
},

// Not using `event` at all does not require synchronous event access.
logSomething: () => {
console.log( 'something' );
},
},
} );
```

## Server functions

The Interactivity API comes with handy functions that allow you to initialize and reference configuration options on the server. This is necessary to feed the initial data that the Server Directive Processing will use to modify the HTML markup before it's send to the browser. It is also a great way to leverage many of WordPress's APIs, like nonces, AJAX, and translations.
Expand Down
15 changes: 10 additions & 5 deletions packages/block-library/src/image/view.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
/**
* WordPress dependencies
*/
import { store, getContext, getElement } from '@wordpress/interactivity';
import {
store,
getContext,
getElement,
withSyncEvent,
} from '@wordpress/interactivity';

/**
* Tracks whether user is touching screen; used to differentiate behavior for
Expand Down Expand Up @@ -128,7 +133,7 @@ const { state, actions, callbacks } = store(
}, 450 );
}
},
handleKeydown( event ) {
handleKeydown: withSyncEvent( ( event ) => {
if ( state.overlayEnabled ) {
// Focuses the close button when the user presses the tab key.
if ( event.key === 'Tab' ) {
Expand All @@ -141,8 +146,8 @@ const { state, actions, callbacks } = store(
actions.hideLightbox();
}
}
},
handleTouchMove( event ) {
} ),
handleTouchMove: withSyncEvent( ( event ) => {
// On mobile devices, prevents triggering the scroll event because
// otherwise the page jumps around when it resets the scroll position.
// This also means that closing the lightbox requires that a user
Expand All @@ -152,7 +157,7 @@ const { state, actions, callbacks } = store(
if ( state.overlayEnabled ) {
event.preventDefault();
}
},
} ),
handleTouchStart() {
isTouching = true;
},
Expand Down
11 changes: 8 additions & 3 deletions packages/block-library/src/navigation/view.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
/**
* WordPress dependencies
*/
import { store, getContext, getElement } from '@wordpress/interactivity';
import {
store,
getContext,
getElement,
withSyncEvent,
} from '@wordpress/interactivity';

const focusableSelectors = [
'a[href]',
Expand Down Expand Up @@ -106,7 +111,7 @@ const { state, actions } = store(
actions.openMenu( 'click' );
}
},
handleMenuKeydown( event ) {
handleMenuKeydown: withSyncEvent( ( event ) => {
const { type, firstFocusableElement, lastFocusableElement } =
getContext();
if ( state.menuOpenedBy.click ) {
Expand Down Expand Up @@ -137,7 +142,7 @@ const { state, actions } = store(
}
}
}
},
} ),
handleMenuFocusout( event ) {
const { modal, type } = getContext();
// If focus is outside modal, and in the document, close menu
Expand Down
11 changes: 8 additions & 3 deletions packages/block-library/src/query/view.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
/**
* WordPress dependencies
*/
import { store, getContext, getElement } from '@wordpress/interactivity';
import {
store,
getContext,
getElement,
withSyncEvent,
} from '@wordpress/interactivity';

const isValidLink = ( ref ) =>
ref &&
Expand All @@ -22,7 +27,7 @@ store(
'core/query',
{
actions: {
*navigate( event ) {
navigate: withSyncEvent( function* ( event ) {
const ctx = getContext();
const { ref } = getElement();
const queryRef = ref.closest(
Expand All @@ -42,7 +47,7 @@ store(
const firstAnchor = `.wp-block-post-template a[href]`;
queryRef.querySelector( firstAnchor )?.focus();
}
},
} ),
*prefetch() {
const { ref } = getElement();
if ( isValidLink( ref ) ) {
Expand Down
11 changes: 8 additions & 3 deletions packages/block-library/src/search/view.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
/**
* WordPress dependencies
*/
import { store, getContext, getElement } from '@wordpress/interactivity';
import {
store,
getContext,
getElement,
withSyncEvent,
} from '@wordpress/interactivity';

const { actions } = store(
'core/search',
Expand Down Expand Up @@ -31,15 +36,15 @@ const { actions } = store(
},
},
actions: {
openSearchInput( event ) {
openSearchInput: withSyncEvent( ( event ) => {
const ctx = getContext();
const { ref } = getElement();
if ( ! ctx.isSearchInputVisible ) {
event.preventDefault();
ctx.isSearchInputVisible = true;
ref.parentElement.querySelector( 'input' ).focus();
}
},
} ),
closeSearchInput() {
const ctx = getContext();
ctx.isSearchInputVisible = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
/**
* WordPress dependencies
*/
import { store, getContext, getServerContext } from '@wordpress/interactivity';
import {
store,
getContext,
getServerContext,
withSyncEvent,
} from '@wordpress/interactivity';

store( 'test/get-server-context', {
actions: {
*navigate( e ) {
navigate: withSyncEvent( function* ( e ) {
e.preventDefault();
const { actions } = yield import(
'@wordpress/interactivity-router'
);
yield actions.navigate( e.target.href );
},
} ),
attemptModification() {
try {
getServerContext().prop = 'updated from client';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
/**
* WordPress dependencies
*/
import { store, getServerState, getContext } from '@wordpress/interactivity';
import {
store,
getServerState,
getContext,
withSyncEvent,
} from '@wordpress/interactivity';

const { state } = store( 'test/get-server-state', {
actions: {
*navigate( e ) {
navigate: withSyncEvent( function* ( e ) {
e.preventDefault();
const { actions } = yield import(
'@wordpress/interactivity-router'
);
yield actions.navigate( e.target.href );
},
} ),
attemptModification() {
try {
getServerState().prop = 'updated from client';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { store } from '@wordpress/interactivity';
import { store, withSyncEvent } from '@wordpress/interactivity';

const { state } = store( 'router', {
state: {
Expand All @@ -18,7 +18,7 @@ const { state } = store( 'router', {
},
},
actions: {
*navigate( e ) {
navigate: withSyncEvent( function* ( e ) {
e.preventDefault();

state.navigations.count += 1;
Expand All @@ -38,7 +38,7 @@ const { state } = store( 'router', {
if ( state.navigations.pending === 0 ) {
state.status = 'idle';
}
},
} ),
toggleTimeout() {
state.timeout = state.timeout === 10000 ? 0 : 10000;
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { store, getContext } from '@wordpress/interactivity';
import { store, getContext, withSyncEvent } from '@wordpress/interactivity';

const { state } = store( 'router-regions', {
state: {
Expand All @@ -17,13 +17,13 @@ const { state } = store( 'router-regions', {
},
actions: {
router: {
*navigate( e ) {
navigate: withSyncEvent( function* ( e ) {
e.preventDefault();
const { actions } = yield import(
'@wordpress/interactivity-router'
);
yield actions.navigate( e.target.href );
},
} ),
back() {
history.back();
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
/**
* WordPress dependencies
*/
import { store } from '@wordpress/interactivity';
import { store, withSyncEvent } from '@wordpress/interactivity';

const { state } = store( 'test/router-styles', {
state: {
clientSideNavigation: false,
},
actions: {
*navigate( e ) {
navigate: withSyncEvent( function* ( e ) {
e.preventDefault();
const { actions } = yield import(
'@wordpress/interactivity-router'
);
yield actions.navigate( e.target.href );
state.clientSideNavigation = true;
},
} ),
},
} );
7 changes: 4 additions & 3 deletions packages/interactivity-router/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ The package is intended to be imported dynamically in the `view.js` files of int
```js
/* view.js */

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

// This is how you would typically use the navigate() action in your block.
store( 'my-namespace/myblock', {
actions: {
*goToPage( e ) {
// The withSyncEvent() utility needs to be used because preventDefault() requires synchronous event access.
goToPage: withSyncEvent( function* ( e ) {
e.preventDefault();

// We import the package dynamically to reduce the initial JS bundle size.
Expand All @@ -31,7 +32,7 @@ store( 'my-namespace/myblock', {
'@wordpress/interactivity-router'
);
yield actions.navigate( e.target.href );
},
} ),
},
} );
```
Expand Down
Loading
Loading