Introduce withSyncEvent action wrapper utility and proxy event ob…

…ject whenever it is not used (WordPress#68097) * Implement withSyncEvent action wrapper utility. * Prepare Interactivity API infrastructure for awareness of action prior to evaluating it. * Proxy event object when withSyncEvent() is not used. * Ensure generator functions using withSyncEvent() are wrapped correctly to still be recognized as generator functions. * Update Interactivity API documentation to reference withSyncEvent(). * Use withSyncEvent() in all built-in actions that require it. * Minor fixes for withSyncEvent docs. * Clarify documentation. Co-authored-by: Weston Ruter <[email protected]> * Enhance withSyncEvent implementation and ensure the sync flag is maintained when proxying functions via withScope. * Add doc block for wrapEventAsync(). * Use more specific types for event proxy handler. * Amend callback in withSyncEvent instead of wrapping it. * Revert "Prepare Interactivity API infrastructure for awareness of action prior to evaluating it." This reverts commit dba93ec. * Update evaluate() to no longer invoke functions (except where needed for BC) and move responsibility to the caller. * Export withSyncEvent * Fix evaluate to return scoped function and always reset scope. * Update custom directives for e2e tests to account for evaluate behavior change. * Update release version number in documentation. --------- Co-authored-by: Weston Ruter <[email protected]> Co-authored-by: Luis Herranz <[email protected]>
carstingaxion · Feb 20, 2025 · c300edf · c300edf
1 parent 0f7193c
commit c300edf
Show file tree

Hide file tree

Showing 22 changed files with 334 additions and 69 deletions.
diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md
@@ -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
@@ -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
 
@@ -1253,6 +1256,43 @@ store( 'mySliderPlugin', {
 } );
 ```
 
+### withSyncEvent()
+
+Actions that require synchronous access to the `event` object need to use the `withSyncEvent()` function to annotate 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 20.4 / WordPress 6.8 all actions that require synchronous event access need to use the `withSyncEvent()` function. Otherwise a deprecation warning will be triggered, and in a future release the behavior will change accordingly.
+
+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.

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
@@ -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
@@ -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' ) {
@@ -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
@@ -152,7 +157,7 @@ const { state, actions, callbacks } = store(
 				if ( state.overlayEnabled ) {
 					event.preventDefault();
 				}
-			},
+			} ),
 			handleTouchStart() {
 				isTouching = true;
 			},

diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js
@@ -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]',
@@ -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 ) {
@@ -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

diff --git a/packages/block-library/src/query/view.js b/packages/block-library/src/query/view.js
@@ -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 &&
@@ -22,7 +27,7 @@ store(
 	'core/query',
 	{
 		actions: {
-			*navigate( event ) {
+			navigate: withSyncEvent( function* ( event ) {
 				const ctx = getContext();
 				const { ref } = getElement();
 				const queryRef = ref.closest(
@@ -42,7 +47,7 @@ store(
 					const firstAnchor = `.wp-block-post-template a[href]`;
 					queryRef.querySelector( firstAnchor )?.focus();
 				}
-			},
+			} ),
 			*prefetch() {
 				const { ref } = getElement();
 				if ( isValidLink( ref ) ) {

diff --git a/packages/block-library/src/search/view.js b/packages/block-library/src/search/view.js
@@ -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',
@@ -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;

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-each/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-each/view.js
@@ -240,7 +240,12 @@ directive(
 	'priority-2-init',
 	( { directives: { 'priority-2-init': init }, evaluate } ) => {
 		init.forEach( ( entry ) => {
-			useInit( () => evaluate( entry ) );
+			useInit( () => {
+				const result = evaluate( entry );
+				if ( typeof result === 'function' ) {
+					result();
+				}
+			} );
 		} );
 	},
 	{ priority: 2 }

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-init/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-init/view.js
@@ -13,7 +13,11 @@ directive(
 	'show-mock',
 	( { directives: { 'show-mock': showMock }, element, evaluate } ) => {
 		const entry = showMock.find( ( { suffix } ) => suffix === null );
-		if ( ! evaluate( entry ) ) {
+		const result = evaluate( entry );
+		if ( ! result ) {
+			return null;
+		}
+		if ( typeof result === 'function' && ! result() ) {
 			return null;
 		}
 		return element;

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-on-document/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-on-document/view.js
@@ -13,7 +13,11 @@ directive(
 	'show-mock',
 	( { directives: { 'show-mock': showMock }, element, evaluate } ) => {
 		const entry = showMock.find( ( { suffix } ) => suffix === null );
-		if ( ! evaluate( entry ) ) {
+		const result = evaluate( entry );
+		if ( ! result ) {
+			return null;
+		}
+		if ( typeof result === 'function' && ! result() ) {
 			return null;
 		}
 		return element;

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-on-window/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-on-window/view.js
@@ -13,7 +13,11 @@ directive(
 	'show-mock',
 	( { directives: { 'show-mock': showMock }, element, evaluate } ) => {
 		const entry = showMock.find( ( { suffix } ) => suffix === null );
-		if ( ! evaluate( entry ) ) {
+		const result = evaluate( entry );
+		if ( ! result ) {
+			return null;
+		}
+		if ( typeof result === 'function' && ! result() ) {
 			return null;
 		}
 		return element;

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-priorities/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-priorities/view.js
@@ -58,10 +58,13 @@ directive(
  */
 directive( 'test-attribute', ( { evaluate, element } ) => {
 	executionProof( 'attribute' );
-	const attributeValue = evaluate( {
+	let attributeValue = evaluate( {
 		namespace,
 		value: 'context.attribute',
 	} );
+	if ( typeof attributeValue === 'function' ) {
+		attributeValue = attributeValue();
+	}
 	useEffect( () => {
 		element.ref.current.setAttribute( 'data-attribute', attributeValue );
 	}, [] );
@@ -76,7 +79,10 @@ directive(
 	'test-text',
 	( { evaluate, element } ) => {
 		executionProof( 'text' );
-		const textValue = evaluate( { namespace, value: 'context.text' } );
+		let textValue = evaluate( { namespace, value: 'context.text' } );
+		if ( typeof textValue === 'function' ) {
+			textValue = textValue();
+		}
 		element.props.children = h( 'p', { 'data-testid': 'text' }, textValue );
 	},
 	{ priority: 12 }
@@ -92,10 +98,22 @@ directive(
 	( { evaluate, element } ) => {
 		executionProof( 'children' );
 		const updateAttribute = () => {
-			evaluate( { namespace, value: 'actions.updateAttribute' } );
+			const result = evaluate( {
+				namespace,
+				value: 'actions.updateAttribute',
+			} );
+			if ( typeof result === 'function' ) {
+				result();
+			}
 		};
 		const updateText = () => {
-			evaluate( { namespace, value: 'actions.updateText' } );
+			const result = evaluate( {
+				namespace,
+				value: 'actions.updateText',
+			} );
+			if ( typeof result === 'function' ) {
+				result();
+			}
 		};
 		element.props.children = h(
 			'div',

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-run/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-run/view.js
@@ -22,9 +22,11 @@ directive(
 		evaluate,
 	} ) => {
 		const entry = showChildren.find( ( { suffix } ) => suffix === null );
-		return evaluate( entry )
-			? element
-			: cloneElement( element, { children: null } );
+		let result = evaluate( entry );
+		if ( typeof result === 'function' ) {
+			result = result();
+		}
+		return result ? element : cloneElement( element, { children: null } );
 	},
 	{ priority: 9 }
 );

diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-watch/view.js b/packages/e2e-tests/plugins/interactive-blocks/directive-watch/view.js
@@ -13,7 +13,11 @@ directive(
 	'show-mock',
 	( { directives: { 'show-mock': showMock }, element, evaluate } ) => {
 		const entry = showMock.find( ( { suffix } ) => suffix === null );
-		if ( ! evaluate( entry ) ) {
+		const result = evaluate( entry );
+		if ( ! result ) {
+			return null;
+		}
+		if ( typeof result === 'function' && ! result() ) {
 			return null;
 		}
 		return element;

diff --git a/packages/e2e-tests/plugins/interactive-blocks/get-server-context/view.js b/packages/e2e-tests/plugins/interactive-blocks/get-server-context/view.js
@@ -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';

diff --git a/packages/e2e-tests/plugins/interactive-blocks/get-server-state/view.js b/packages/e2e-tests/plugins/interactive-blocks/get-server-state/view.js
@@ -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';