From c300edfebb48f79f6f0f6643ce04dd73303c5fcb Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 20 Feb 2025 12:44:27 -0800 Subject: [PATCH] Introduce `withSyncEvent` action wrapper utility and proxy `event` object whenever it is not used (#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 * 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 dba93ec4f7bb617d1f12ba1a06acfa39ff33d4b3. * 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 Co-authored-by: Luis Herranz --- .../interactivity-api/api-reference.md | 46 ++++++- packages/block-library/src/image/view.js | 15 ++- packages/block-library/src/navigation/view.js | 11 +- packages/block-library/src/query/view.js | 11 +- packages/block-library/src/search/view.js | 11 +- .../interactive-blocks/directive-each/view.js | 7 +- .../interactive-blocks/directive-init/view.js | 6 +- .../directive-on-document/view.js | 6 +- .../directive-on-window/view.js | 6 +- .../directive-priorities/view.js | 26 +++- .../interactive-blocks/directive-run/view.js | 8 +- .../directive-watch/view.js | 6 +- .../get-server-context/view.js | 11 +- .../get-server-state/view.js | 11 +- .../router-navigate/view.js | 6 +- .../interactive-blocks/router-regions/view.js | 6 +- .../interactive-blocks/tovdom-islands/view.js | 6 +- packages/interactivity-router/README.md | 7 +- packages/interactivity/src/directives.tsx | 119 ++++++++++++++++-- packages/interactivity/src/hooks.tsx | 25 +++- packages/interactivity/src/index.ts | 1 + packages/interactivity/src/utils.ts | 52 ++++++-- 22 files changed, 334 insertions(+), 69 deletions(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index bbbb565684c57..bf2c1370ebcde 100644 --- 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 index 3c9a729538813..71a492a570b2a 100644 --- 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 index 9da7ab70d84f3..fd1fe33537b2f 100644 --- 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 index e23294a24e02e..fff12b16eac65 100644 --- 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 index 0e4c462a2e321..617e179b1dc88 100644 --- 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,7 +36,7 @@ const { actions } = store( }, }, actions: { - openSearchInput( event ) { + openSearchInput: withSyncEvent( ( event ) => { const ctx = getContext(); const { ref } = getElement(); if ( ! ctx.isSearchInputVisible ) { @@ -39,7 +44,7 @@ const { actions } = store( 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 index 7577810b6bb87..98fd1fdb5593d 100644 --- 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 index a8c70a4a90720..c27fe8d534d86 100644 --- 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 index b9689ac978f85..f7918f3c6bf53 100644 --- 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 index ef72e266e1075..0c29b09e5a70c 100644 --- 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 index 77f2f25c5f9a4..dd4cad1c32ed6 100644 --- 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 index 125ac39204230..3b623baa43a09 100644 --- 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 index ad035811a0bcd..bb533ef9a208a 100644 --- 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 index 83f016e2eac16..d9eb2005cef88 100644 --- 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 index db2992ec4a586..23cd0c328aee6 100644 --- 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'; diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js index bd1d6e1164779..266a989ada739 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { store } from '@wordpress/interactivity'; +import { store, withSyncEvent } from '@wordpress/interactivity'; const { state } = store( 'router', { state: { @@ -18,7 +18,7 @@ const { state } = store( 'router', { }, }, actions: { - *navigate( e ) { + navigate: withSyncEvent( function* ( e ) { e.preventDefault(); state.navigations.count += 1; @@ -38,7 +38,7 @@ const { state } = store( 'router', { if ( state.navigations.pending === 0 ) { state.status = 'idle'; } - }, + } ), toggleTimeout() { state.timeout = state.timeout === 10000 ? 0 : 10000; }, diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-regions/view.js b/packages/e2e-tests/plugins/interactive-blocks/router-regions/view.js index f3468eb88aff0..a3a35d792755c 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-regions/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/router-regions/view.js @@ -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: { @@ -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(); }, diff --git a/packages/e2e-tests/plugins/interactive-blocks/tovdom-islands/view.js b/packages/e2e-tests/plugins/interactive-blocks/tovdom-islands/view.js index 8016e931624a1..b4fc12a91a4f2 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/tovdom-islands/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/tovdom-islands/view.js @@ -14,7 +14,11 @@ directive( ( { directives: { 'show-mock': showMock }, element, evaluate } ) => { const entry = showMock.find( ( { suffix } ) => suffix === null ); - if ( ! evaluate( entry ) ) { + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } + if ( ! result ) { element.props.children = h( 'template', null, diff --git a/packages/interactivity-router/README.md b/packages/interactivity-router/README.md index b79e6b310e239..3491ad3b459a4 100644 --- a/packages/interactivity-router/README.md +++ b/packages/interactivity-router/README.md @@ -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. @@ -31,7 +32,7 @@ store( 'my-namespace/myblock', { '@wordpress/interactivity-router' ); yield actions.navigate( e.target.href ); - }, + } ), }, } ); ``` diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index bddd017b1c99d..4568eba013c3b 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -50,6 +50,54 @@ function deepClone< T >( source: T ): T { return source; } +/** + * Wraps event object to warn about access of synchronous properties and methods. + * + * For all store actions attached to an event listener the event object is proxied via this function, unless the action + * uses the `withSyncEvent()` utility to indicate that it requires synchronous access to the event object. + * + * At the moment, the proxied event only emits warnings when synchronous properties or methods are being accessed. In + * the future this will be changed and result in an error. The current temporary behavior allows implementers to update + * their relevant actions to use `withSyncEvent()`. + * + * For additional context, see https://github.com/WordPress/gutenberg/issues/64944. + * + * @param event Event object. + * @return Proxied event object. + */ +function wrapEventAsync( event: Event ) { + const handler = { + get( target: Event, prop: string | symbol, receiver: any ) { + const value = target[ prop ]; + switch ( prop ) { + case 'currentTarget': + warn( + `Accessing the synchronous event.${ prop } property in a store action without wrapping it in withSyncEvent() is deprecated and will stop working in WordPress 6.9. Please wrap the store action in withSyncEvent().` + ); + break; + case 'preventDefault': + case 'stopImmediatePropagation': + case 'stopPropagation': + warn( + `Using the synchronous event.${ prop }() function in a store action without wrapping it in withSyncEvent() is deprecated and will stop working in WordPress 6.9. Please wrap the store action in withSyncEvent().` + ); + break; + } + if ( value instanceof Function ) { + return function ( this: any, ...args: any[] ) { + return value.apply( + this === receiver ? target : this, + args + ); + }; + } + return value; + }, + }; + + return new Proxy( event, handler ); +} + const newRule = /(?:([\u0080-\uFFFF\w-%@]+) *:? *([^{;]+?);|([^;}{]*?) *{)|(}\s*)/g; const ruleClean = /\/\*[^]*?\*\/| +/g; @@ -102,7 +150,15 @@ const getGlobalEventDirective = ( .forEach( ( entry ) => { const eventName = entry.suffix.split( '--', 1 )[ 0 ]; useInit( () => { - const cb = ( event: Event ) => evaluate( entry, event ); + const cb = ( event: Event ) => { + const result = evaluate( entry ); + if ( typeof result === 'function' ) { + if ( ! result?.sync ) { + event = wrapEventAsync( event ); + } + result( event ); + } + }; const globalVar = type === 'window' ? window : document; globalVar.addEventListener( eventName, cb ); return () => globalVar.removeEventListener( eventName, cb ); @@ -128,7 +184,10 @@ const getGlobalAsyncEventDirective = ( useInit( () => { const cb = async ( event: Event ) => { await splitTask(); - evaluate( entry, event ); + const result = evaluate( entry ); + if ( typeof result === 'function' ) { + result( event ); + } }; const globalVar = type === 'window' ? window : document; globalVar.addEventListener( eventName, cb, { @@ -206,7 +265,10 @@ export default () => { start = performance.now(); } } - const result = evaluate( entry ); + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } if ( globalThis.IS_GUTENBERG_PLUGIN ) { if ( globalThis.SCRIPT_DEBUG ) { performance.measure( @@ -239,7 +301,10 @@ export default () => { start = performance.now(); } } - const result = evaluate( entry ); + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } if ( globalThis.IS_GUTENBERG_PLUGIN ) { if ( globalThis.SCRIPT_DEBUG ) { performance.measure( @@ -286,7 +351,13 @@ export default () => { start = performance.now(); } } - evaluate( entry, event ); + const result = evaluate( entry ); + if ( typeof result === 'function' ) { + if ( ! result?.sync ) { + event = wrapEventAsync( event ); + } + result( event ); + } if ( globalThis.IS_GUTENBERG_PLUGIN ) { if ( globalThis.SCRIPT_DEBUG ) { performance.measure( @@ -332,7 +403,10 @@ export default () => { } entries.forEach( async ( entry ) => { await splitTask(); - evaluate( entry, event ); + const result = evaluate( entry ); + if ( typeof result === 'function' ) { + result( event ); + } } ); }; } ); @@ -360,7 +434,10 @@ export default () => { .filter( isNonDefaultDirectiveSuffix ) .forEach( ( entry ) => { const className = entry.suffix; - const result = evaluate( entry ); + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } const currentClass = element.props.class || ''; const classFinder = new RegExp( `(^|\\s)${ className }(\\s|$)`, @@ -400,7 +477,10 @@ export default () => { directive( 'style', ( { directives: { style }, element, evaluate } ) => { style.filter( isNonDefaultDirectiveSuffix ).forEach( ( entry ) => { const styleProp = entry.suffix; - const result = evaluate( entry ); + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } element.props.style = element.props.style || {}; if ( typeof element.props.style === 'string' ) { element.props.style = cssStringToObject( element.props.style ); @@ -434,7 +514,10 @@ export default () => { directive( 'bind', ( { directives: { bind }, element, evaluate } ) => { bind.filter( isNonDefaultDirectiveSuffix ).forEach( ( entry ) => { const attribute = entry.suffix; - const result = evaluate( entry ); + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } element.props[ attribute ] = result; /* @@ -535,7 +618,10 @@ export default () => { } try { - const result = evaluate( entry ); + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } element.props.children = typeof result === 'object' ? null : result.toString(); } catch ( e ) { @@ -545,7 +631,13 @@ export default () => { // data-wp-run directive( 'run', ( { directives: { run }, evaluate } ) => { - run.forEach( ( entry ) => evaluate( entry ) ); + run.forEach( ( entry ) => { + let result = evaluate( entry ); + if ( typeof result === 'function' ) { + result = result(); + } + return result; + } ); } ); // data-wp-each--[item] @@ -567,7 +659,10 @@ export default () => { const [ entry ] = each; const { namespace } = entry; - const iterable = evaluate( entry ); + let iterable = evaluate( entry ); + if ( typeof iterable === 'function' ) { + iterable = iterable(); + } if ( typeof iterable?.[ Symbol.iterator ] !== 'function' ) { return; diff --git a/packages/interactivity/src/hooks.tsx b/packages/interactivity/src/hooks.tsx index 7899e3eafd228..3d75fb03aa728 100644 --- a/packages/interactivity/src/hooks.tsx +++ b/packages/interactivity/src/hooks.tsx @@ -231,6 +231,7 @@ const resolve = ( path: string, namespace: string ) => { // Generate the evaluate function. export const getEvaluate: GetEvaluate = ( { scope } ) => + // TODO: When removing the temporarily remaining `value( ...args )` call below, remove the `...args` parameter too. ( entry, ...args ) => { let { value: path, namespace } = entry; if ( typeof path !== 'string' ) { @@ -241,7 +242,29 @@ export const getEvaluate: GetEvaluate = path[ 0 ] === '!' && !! ( path = path.slice( 1 ) ); setScope( scope ); const value = resolve( path, namespace ); - const result = typeof value === 'function' ? value( ...args ) : value; + // Functions are returned without invoking them. + if ( typeof value === 'function' ) { + // Except if they have a negation operator present, for backward compatibility. + // This pattern is strongly discouraged and deprecated, and it will be removed in a near future release. + // TODO: Remove this condition to effectively ignore negation operator when provided with a function. + if ( hasNegationOperator ) { + warn( + 'Using a function with a negation operator is deprecated and will stop working in WordPress 6.9. Please use derived state instead.' + ); + const functionResult = ! value( ...args ); + resetScope(); + return functionResult; + } + // Reset scope before return and wrap the function so it will still run within the correct scope. + resetScope(); + return ( ...functionArgs: any[] ) => { + setScope( scope ); + const functionResult = value( ...functionArgs ); + resetScope(); + return functionResult; + }; + } + const result = value; resetScope(); return hasNegationOperator ? ! result : result; }; diff --git a/packages/interactivity/src/index.ts b/packages/interactivity/src/index.ts index 9d013e4e744ed..b7d68fd200705 100644 --- a/packages/interactivity/src/index.ts +++ b/packages/interactivity/src/index.ts @@ -27,6 +27,7 @@ export { useCallback, useMemo, splitTask, + withSyncEvent, } from './utils'; export { useState, useRef } from 'preact/hooks'; diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index d894d37a7b84b..7069088a8836b 100644 --- a/packages/interactivity/src/utils.ts +++ b/packages/interactivity/src/utils.ts @@ -30,6 +30,10 @@ declare global { } } +interface SyncAwareFunction extends Function { + sync?: boolean; +} + /** * Executes a callback function after the next frame is rendered. * @@ -135,11 +139,14 @@ export function withScope< ? Promise< Return > : never; export function withScope< Func extends Function >( func: Func ): Func; +export function withScope< Func extends SyncAwareFunction >( func: Func ): Func; export function withScope( func: ( ...args: unknown[] ) => unknown ) { const scope = getScope(); const ns = getNamespace(); + + let wrapped: Function; if ( func?.constructor?.name === 'GeneratorFunction' ) { - return async ( ...args: Parameters< typeof func > ) => { + wrapped = async ( ...args: Parameters< typeof func > ) => { const gen = func( ...args ) as Generator; let value: any; let it: any; @@ -171,17 +178,28 @@ export function withScope( func: ( ...args: unknown[] ) => unknown ) { return value; }; + } else { + wrapped = ( ...args: Parameters< typeof func > ) => { + setNamespace( ns ); + setScope( scope ); + try { + return func( ...args ); + } finally { + resetNamespace(); + resetScope(); + } + }; } - return ( ...args: Parameters< typeof func > ) => { - setNamespace( ns ); - setScope( scope ); - try { - return func( ...args ); - } finally { - resetNamespace(); - resetScope(); - } - }; + + // If function was annotated via `withSyncEvent()`, maintain the annotation. + const syncAware = func as SyncAwareFunction; + if ( syncAware.sync ) { + const syncAwareWrapped = wrapped as SyncAwareFunction; + syncAwareWrapped.sync = true; + return syncAwareWrapped; + } + + return wrapped; } /** @@ -374,3 +392,15 @@ export const isPlainObject = ( typeof candidate === 'object' && candidate.constructor === Object ); + +/** + * Indicates that the passed `callback` requires synchronous access to the event object. + * + * @param callback The event callback. + * @return Altered event callback. + */ +export function withSyncEvent( callback: Function ): SyncAwareFunction { + const syncAware = callback as SyncAwareFunction; + syncAware.sync = true; + return syncAware; +}