From 60448fe5fd83d801816ca413634929123abcd364 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 18 Dec 2024 06:13:34 -0800 Subject: [PATCH 01/18] Implement withSyncEvent action wrapper utility. --- packages/interactivity/src/utils.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index ab6b0074727ee..853287187cee8 100644 --- a/packages/interactivity/src/utils.ts +++ b/packages/interactivity/src/utils.ts @@ -374,3 +374,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 Wrapped event callback. + */ +export const withSyncEvent = ( callback: Function ): Function => { + const wrapped = ( ...args: any[] ) => callback( ...args ); + wrapped.sync = true; + return wrapped; +}; From dba93ec4f7bb617d1f12ba1a06acfa39ff33d4b3 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 18 Dec 2024 06:14:23 -0800 Subject: [PATCH 02/18] Prepare Interactivity API infrastructure for awareness of action prior to evaluating it. --- packages/interactivity/src/directives.tsx | 99 +++++++++++++---------- packages/interactivity/src/hooks.tsx | 71 +++++++++++++++- packages/interactivity/src/index.ts | 1 + packages/interactivity/src/scopes.ts | 4 +- 4 files changed, 129 insertions(+), 46 deletions(-) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index bddd017b1c99d..ecac89a32be2e 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -96,13 +96,19 @@ const cssStringToObject = ( const getGlobalEventDirective = ( type: 'window' | 'document' ): DirectiveCallback => { - return ( { directives, evaluate } ) => { + return ( { directives, resolveEntry, evaluateResolved } ) => { directives[ `on-${ type }` ] .filter( isNonDefaultDirectiveSuffix ) .forEach( ( entry ) => { const eventName = entry.suffix.split( '--', 1 )[ 0 ]; useInit( () => { - const cb = ( event: Event ) => evaluate( entry, event ); + const cb = ( event: Event ) => { + const resolved = resolveEntry( entry ); + if ( ! resolved.value?.sync ) { + // TODO: Wrap event in proxy. + } + evaluateResolved( resolved, event ); + }; const globalVar = type === 'window' ? window : document; globalVar.addEventListener( eventName, cb ); return () => globalVar.removeEventListener( eventName, cb ); @@ -263,51 +269,58 @@ export default () => { } ); // data-wp-on--[event] - directive( 'on', ( { directives: { on }, element, evaluate } ) => { - const events = new Map< string, Set< DirectiveEntry > >(); - on.filter( isNonDefaultDirectiveSuffix ).forEach( ( entry ) => { - const event = entry.suffix.split( '--' )[ 0 ]; - if ( ! events.has( event ) ) { - events.set( event, new Set< DirectiveEntry >() ); - } - events.get( event )!.add( entry ); - } ); + directive( + 'on', + ( { directives: { on }, element, resolveEntry, evaluateResolved } ) => { + const events = new Map< string, Set< DirectiveEntry > >(); + on.filter( isNonDefaultDirectiveSuffix ).forEach( ( entry ) => { + const event = entry.suffix.split( '--' )[ 0 ]; + if ( ! events.has( event ) ) { + events.set( event, new Set< DirectiveEntry >() ); + } + events.get( event )!.add( entry ); + } ); - events.forEach( ( entries, eventType ) => { - const existingHandler = element.props[ `on${ eventType }` ]; - element.props[ `on${ eventType }` ] = ( event: Event ) => { - entries.forEach( ( entry ) => { - if ( existingHandler ) { - existingHandler( event ); - } - let start; - if ( globalThis.IS_GUTENBERG_PLUGIN ) { - if ( globalThis.SCRIPT_DEBUG ) { - start = performance.now(); + events.forEach( ( entries, eventType ) => { + const existingHandler = element.props[ `on${ eventType }` ]; + element.props[ `on${ eventType }` ] = ( event: Event ) => { + entries.forEach( ( entry ) => { + if ( existingHandler ) { + existingHandler( event ); } - } - evaluate( entry, event ); - if ( globalThis.IS_GUTENBERG_PLUGIN ) { - if ( globalThis.SCRIPT_DEBUG ) { - performance.measure( - `interactivity api on ${ entry.namespace }`, - { - // eslint-disable-next-line no-undef - start, - end: performance.now(), - detail: { - devtools: { - track: `IA: on ${ entry.namespace }`, + let start; + if ( globalThis.IS_GUTENBERG_PLUGIN ) { + if ( globalThis.SCRIPT_DEBUG ) { + start = performance.now(); + } + } + const resolved = resolveEntry( entry ); + if ( ! resolved.value?.sync ) { + // TODO: Wrap event in proxy. + } + evaluateResolved( resolved, event ); + if ( globalThis.IS_GUTENBERG_PLUGIN ) { + if ( globalThis.SCRIPT_DEBUG ) { + performance.measure( + `interactivity api on ${ entry.namespace }`, + { + // eslint-disable-next-line no-undef + start, + end: performance.now(), + detail: { + devtools: { + track: `IA: on ${ entry.namespace }`, + }, }, - }, - } - ); + } + ); + } } - } - } ); - }; - } ); - } ); + } ); + }; + } ); + } + ); // data-wp-on-async--[event] directive( diff --git a/packages/interactivity/src/hooks.tsx b/packages/interactivity/src/hooks.tsx index 7899e3eafd228..fa56e8ece6a97 100644 --- a/packages/interactivity/src/hooks.tsx +++ b/packages/interactivity/src/hooks.tsx @@ -74,6 +74,16 @@ interface DirectiveArgs { * context. */ evaluate: Evaluate; + /** + * Function that resolves a given path to value data in the store or the + * context. + */ + resolveEntry: ResolveEntry; + /** + * Function that evaluates resolved value data to a value either in the store + * or the context. + */ + evaluateResolved: EvaluateResolved; } export interface DirectiveCallback { @@ -90,6 +100,17 @@ interface DirectiveOptions { priority?: number; } +interface ResolvedEntry { + /** + * Resolved value, either a result or a function to get the result. + */ + value: any; + /** + * Whether a negation operator should be used on the result. + */ + hasNegationOperator: boolean; +} + export interface Evaluate { ( entry: DirectiveEntry, ...args: any[] ): any; } @@ -98,6 +119,22 @@ interface GetEvaluate { ( args: { scope: Scope } ): Evaluate; } +export interface ResolveEntry { + ( entry: DirectiveEntry ): ResolvedEntry; +} + +interface GetResolveEntry { + ( args: { scope: Scope } ): ResolveEntry; +} + +export interface EvaluateResolved { + ( resolved: ResolvedEntry, ...args: any[] ): any; +} + +interface GetEvaluateResolved { + ( args: { scope: Scope } ): EvaluateResolved; +} + type PriorityLevel = string[]; interface GetPriorityLevels { @@ -229,9 +266,19 @@ const resolve = ( path: string, namespace: string ) => { }; // Generate the evaluate function. -export const getEvaluate: GetEvaluate = +export const getEvaluate: GetEvaluate = ( scopeData ) => { + const resolveEntry = getResolveEntry( scopeData ); + const evaluateResolved = getEvaluateResolved( scopeData ); + return ( entry, ...args ) => { + const resolved = resolveEntry( entry ); + return evaluateResolved( resolved, ...args ); + }; +}; + +// Generate the resolveEntry function. +export const getResolveEntry: GetResolveEntry = ( { scope } ) => - ( entry, ...args ) => { + ( entry ) => { let { value: path, namespace } = entry; if ( typeof path !== 'string' ) { throw new Error( 'The `value` prop should be a string path' ); @@ -241,6 +288,19 @@ export const getEvaluate: GetEvaluate = path[ 0 ] === '!' && !! ( path = path.slice( 1 ) ); setScope( scope ); const value = resolve( path, namespace ); + resetScope(); + return { + value, + hasNegationOperator, + }; + }; + +// Generate the evaluateResolved function. +export const getEvaluateResolved: GetEvaluateResolved = + ( { scope } ) => + ( resolved, ...args ) => { + const { value, hasNegationOperator } = resolved; + setScope( scope ); const result = typeof value === 'function' ? value( ...args ) : value; resetScope(); return hasNegationOperator ? ! result : result; @@ -277,6 +337,11 @@ const Directives = ( { // element ref, state and props. const scope = useRef< Scope >( {} as Scope ).current; scope.evaluate = useCallback( getEvaluate( { scope } ), [] ); + scope.resolveEntry = useCallback( getResolveEntry( { scope } ), [] ); + scope.evaluateResolved = useCallback( + getEvaluateResolved( { scope } ), + [] + ); const { client, server } = useContext( context ); scope.context = client; scope.serverContext = server; @@ -308,6 +373,8 @@ const Directives = ( { element, context, evaluate: scope.evaluate, + resolveEntry: scope.resolveEntry, + evaluateResolved: scope.evaluateResolved, }; setScope( scope ); 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/scopes.ts b/packages/interactivity/src/scopes.ts index 722305f6bee11..d9734b695e75e 100644 --- a/packages/interactivity/src/scopes.ts +++ b/packages/interactivity/src/scopes.ts @@ -7,10 +7,12 @@ import type { h as createElement, RefObject } from 'preact'; * Internal dependencies */ import { getNamespace } from './namespaces'; -import type { Evaluate } from './hooks'; +import type { Evaluate, ResolveEntry, EvaluateResolved } from './hooks'; export interface Scope { evaluate: Evaluate; + resolveEntry: ResolveEntry; + evaluateResolved: EvaluateResolved; context: object; serverContext: object; ref: RefObject< HTMLElement >; From dad7083bc8b85d03955be237e21d7c619117692d Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 2 Jan 2025 14:58:20 -0800 Subject: [PATCH 03/18] Proxy event object when withSyncEvent() is not used. --- packages/interactivity/src/directives.tsx | 37 +++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index ecac89a32be2e..e2fea91a0c4d3 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -50,6 +50,39 @@ function deepClone< T >( source: T ): T { return source; } +function wrapEventAsync( event: Event ) { + const handler = { + get( target: any, prop: any, 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; @@ -105,7 +138,7 @@ const getGlobalEventDirective = ( const cb = ( event: Event ) => { const resolved = resolveEntry( entry ); if ( ! resolved.value?.sync ) { - // TODO: Wrap event in proxy. + event = wrapEventAsync( event ); } evaluateResolved( resolved, event ); }; @@ -296,7 +329,7 @@ export default () => { } const resolved = resolveEntry( entry ); if ( ! resolved.value?.sync ) { - // TODO: Wrap event in proxy. + event = wrapEventAsync( event ); } evaluateResolved( resolved, event ); if ( globalThis.IS_GUTENBERG_PLUGIN ) { From 21e51be1298a268b175a8f1d7d4daf6af6ab1361 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 21 Jan 2025 13:36:23 -0800 Subject: [PATCH 04/18] Ensure generator functions using withSyncEvent() are wrapped correctly to still be recognized as generator functions. --- packages/interactivity/src/utils.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index 75438c0afaccb..3ed58bd4c7d28 100644 --- a/packages/interactivity/src/utils.ts +++ b/packages/interactivity/src/utils.ts @@ -382,6 +382,14 @@ export const isPlainObject = ( * @return Wrapped event callback. */ export const withSyncEvent = ( callback: Function ): Function => { + if ( callback?.constructor?.name === 'GeneratorFunction' ) { + const wrapped = function* ( ...args: any[] ) { + yield* callback( ...args ); + }; + wrapped.sync = true; + return wrapped; + } + const wrapped = ( ...args: any[] ) => callback( ...args ); wrapped.sync = true; return wrapped; From a582f01b2308dc58aa66e072f712cdfa3379f88f Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 21 Jan 2025 13:46:00 -0800 Subject: [PATCH 05/18] Update Interactivity API documentation to reference withSyncEvent(). --- .../interactivity-api/api-reference.md | 46 +++++++++++++++++-- packages/interactivity-router/README.md | 7 +-- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index bbbb565684c57..5aef537a247b3 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. 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 event access 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 should use the `withSyncEvent()` utility wrapper 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 one does 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/interactivity-router/README.md b/packages/interactivity-router/README.md index efb52e59be2b5..e3f96eb71d70b 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 ); - }, + } ), }, } ); ``` From 5d68fcb8226bb2dbc7473757bd5c586df3ee3654 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 21 Jan 2025 13:46:45 -0800 Subject: [PATCH 06/18] Use withSyncEvent() in all built-in actions that require it. --- 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/get-server-context/view.js | 11 ++++++++--- .../interactive-blocks/get-server-state/view.js | 11 ++++++++--- .../interactive-blocks/router-navigate/view.js | 6 +++--- .../interactive-blocks/router-regions/view.js | 6 +++--- .../router-styles-wrapper/view.js | 6 +++--- 9 files changed, 59 insertions(+), 29 deletions(-) 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/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/router-styles-wrapper/view.js b/packages/e2e-tests/plugins/interactive-blocks/router-styles-wrapper/view.js index 5b3b42f2b413e..e15531c09518b 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-styles-wrapper/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/router-styles-wrapper/view.js @@ -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; - }, + } ), }, } ); From 621a9c9eba00c69edaab5ebc89be2ccd93b8954a Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 22 Jan 2025 13:26:18 -0800 Subject: [PATCH 07/18] Minor fixes for withSyncEvent docs. --- docs/reference-guides/interactivity-api/api-reference.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index 5aef537a247b3..5b2e3788141c6 100644 --- a/docs/reference-guides/interactivity-api/api-reference.md +++ b/docs/reference-guides/interactivity-api/api-reference.md @@ -1258,7 +1258,7 @@ store( 'mySliderPlugin', { ### withSyncEvent() -Actions that require synchronous event access 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 should use the `withSyncEvent()` utility wrapper function. Otherwise a deprecation warning will be triggered, and in a future release the behavior will change accordingly. +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. 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: @@ -1267,7 +1267,7 @@ Only very specific event methods and properties require synchronous access, so i * `event.stopImmediatePropagation()` * `event.stopPropagation()` -Here is an example, where one action requires synchronous event access while the other one does not: +Here is an example, where one action requires synchronous event access while the other actions do not: ```js // store From ec3d662764a5953d9e73026a87919f6922a57144 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 29 Jan 2025 13:21:30 -0800 Subject: [PATCH 08/18] Clarify documentation. Co-authored-by: Weston Ruter --- docs/reference-guides/interactivity-api/api-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index 5b2e3788141c6..dc1798860b01f 100644 --- a/docs/reference-guides/interactivity-api/api-reference.md +++ b/docs/reference-guides/interactivity-api/api-reference.md @@ -896,7 +896,7 @@ store( 'myPlugin', { } ); ``` -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. Otherwise a deprecation warning will be triggered, and in a future release the behavior will change accordingly. +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 From bf3dbcc861367a38131ac0943ad49f60e2474912 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 29 Jan 2025 15:22:25 -0800 Subject: [PATCH 09/18] Enhance withSyncEvent implementation and ensure the sync flag is maintained when proxying functions via withScope. --- packages/interactivity/src/utils.ts | 57 ++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index 3ed58bd4c7d28..57e7c68a456ba 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; } /** @@ -381,16 +399,19 @@ export const isPlainObject = ( * @param callback The event callback. * @return Wrapped event callback. */ -export const withSyncEvent = ( callback: Function ): Function => { +export function withSyncEvent( callback: Function ): SyncAwareFunction { + let wrapped: SyncAwareFunction; + if ( callback?.constructor?.name === 'GeneratorFunction' ) { - const wrapped = function* ( ...args: any[] ) { - yield* callback( ...args ); + wrapped = function* ( this: any, ...args: any[] ) { + yield* callback.apply( this, args ); + }; + } else { + wrapped = function ( this: any, ...args: any[] ) { + return callback.apply( this, args ); }; - wrapped.sync = true; - return wrapped; } - const wrapped = ( ...args: any[] ) => callback( ...args ); wrapped.sync = true; return wrapped; -}; +} From 2dd6eabe1a7d7ade2f3ff1834dd7232551498e72 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 29 Jan 2025 15:33:49 -0800 Subject: [PATCH 10/18] Add doc block for wrapEventAsync(). --- packages/interactivity/src/directives.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index e2fea91a0c4d3..7cd1bb075983f 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -50,6 +50,21 @@ 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: any, prop: any, receiver: any ) { From 682ee6f3fb8647e664287df73c77ce79d6cc3e1a Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 29 Jan 2025 15:36:07 -0800 Subject: [PATCH 11/18] Use more specific types for event proxy handler. --- packages/interactivity/src/directives.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index 7cd1bb075983f..c89aca282e079 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -67,7 +67,7 @@ function deepClone< T >( source: T ): T { */ function wrapEventAsync( event: Event ) { const handler = { - get( target: any, prop: any, receiver: any ) { + get( target: Event, prop: string | symbol, receiver: any ) { const value = target[ prop ]; switch ( prop ) { case 'currentTarget': From f0fa92afaaca9b625a341610564c73f0fb9ad2a3 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Mon, 10 Feb 2025 08:09:33 -0800 Subject: [PATCH 12/18] Amend callback in withSyncEvent instead of wrapping it. --- .../interactivity-api/api-reference.md | 2 +- packages/interactivity/src/utils.ts | 19 ++++--------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index dc1798860b01f..99fcd83061fc4 100644 --- a/docs/reference-guides/interactivity-api/api-reference.md +++ b/docs/reference-guides/interactivity-api/api-reference.md @@ -1258,7 +1258,7 @@ 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. +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 `TODO: Add release number here!` / 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: diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index 57e7c68a456ba..7069088a8836b 100644 --- a/packages/interactivity/src/utils.ts +++ b/packages/interactivity/src/utils.ts @@ -397,21 +397,10 @@ export const isPlainObject = ( * Indicates that the passed `callback` requires synchronous access to the event object. * * @param callback The event callback. - * @return Wrapped event callback. + * @return Altered event callback. */ export function withSyncEvent( callback: Function ): SyncAwareFunction { - let wrapped: SyncAwareFunction; - - if ( callback?.constructor?.name === 'GeneratorFunction' ) { - wrapped = function* ( this: any, ...args: any[] ) { - yield* callback.apply( this, args ); - }; - } else { - wrapped = function ( this: any, ...args: any[] ) { - return callback.apply( this, args ); - }; - } - - wrapped.sync = true; - return wrapped; + const syncAware = callback as SyncAwareFunction; + syncAware.sync = true; + return syncAware; } From 2fbfd0e45f89b07f1f531f1013c6a11185c85293 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 13 Feb 2025 09:58:20 -0800 Subject: [PATCH 13/18] Revert "Prepare Interactivity API infrastructure for awareness of action prior to evaluating it." This reverts commit dba93ec4f7bb617d1f12ba1a06acfa39ff33d4b3. --- packages/interactivity/src/directives.tsx | 99 ++++++++++------------- packages/interactivity/src/hooks.tsx | 71 +--------------- packages/interactivity/src/index.ts | 1 - packages/interactivity/src/scopes.ts | 4 +- 4 files changed, 46 insertions(+), 129 deletions(-) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index c89aca282e079..a8d1931591d31 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -144,19 +144,13 @@ const cssStringToObject = ( const getGlobalEventDirective = ( type: 'window' | 'document' ): DirectiveCallback => { - return ( { directives, resolveEntry, evaluateResolved } ) => { + return ( { directives, evaluate } ) => { directives[ `on-${ type }` ] .filter( isNonDefaultDirectiveSuffix ) .forEach( ( entry ) => { const eventName = entry.suffix.split( '--', 1 )[ 0 ]; useInit( () => { - const cb = ( event: Event ) => { - const resolved = resolveEntry( entry ); - if ( ! resolved.value?.sync ) { - event = wrapEventAsync( event ); - } - evaluateResolved( resolved, event ); - }; + const cb = ( event: Event ) => evaluate( entry, event ); const globalVar = type === 'window' ? window : document; globalVar.addEventListener( eventName, cb ); return () => globalVar.removeEventListener( eventName, cb ); @@ -317,58 +311,51 @@ export default () => { } ); // data-wp-on--[event] - directive( - 'on', - ( { directives: { on }, element, resolveEntry, evaluateResolved } ) => { - const events = new Map< string, Set< DirectiveEntry > >(); - on.filter( isNonDefaultDirectiveSuffix ).forEach( ( entry ) => { - const event = entry.suffix.split( '--' )[ 0 ]; - if ( ! events.has( event ) ) { - events.set( event, new Set< DirectiveEntry >() ); - } - events.get( event )!.add( entry ); - } ); + directive( 'on', ( { directives: { on }, element, evaluate } ) => { + const events = new Map< string, Set< DirectiveEntry > >(); + on.filter( isNonDefaultDirectiveSuffix ).forEach( ( entry ) => { + const event = entry.suffix.split( '--' )[ 0 ]; + if ( ! events.has( event ) ) { + events.set( event, new Set< DirectiveEntry >() ); + } + events.get( event )!.add( entry ); + } ); - events.forEach( ( entries, eventType ) => { - const existingHandler = element.props[ `on${ eventType }` ]; - element.props[ `on${ eventType }` ] = ( event: Event ) => { - entries.forEach( ( entry ) => { - if ( existingHandler ) { - existingHandler( event ); - } - let start; - if ( globalThis.IS_GUTENBERG_PLUGIN ) { - if ( globalThis.SCRIPT_DEBUG ) { - start = performance.now(); - } - } - const resolved = resolveEntry( entry ); - if ( ! resolved.value?.sync ) { - event = wrapEventAsync( event ); + events.forEach( ( entries, eventType ) => { + const existingHandler = element.props[ `on${ eventType }` ]; + element.props[ `on${ eventType }` ] = ( event: Event ) => { + entries.forEach( ( entry ) => { + if ( existingHandler ) { + existingHandler( event ); + } + let start; + if ( globalThis.IS_GUTENBERG_PLUGIN ) { + if ( globalThis.SCRIPT_DEBUG ) { + start = performance.now(); } - evaluateResolved( resolved, event ); - if ( globalThis.IS_GUTENBERG_PLUGIN ) { - if ( globalThis.SCRIPT_DEBUG ) { - performance.measure( - `interactivity api on ${ entry.namespace }`, - { - // eslint-disable-next-line no-undef - start, - end: performance.now(), - detail: { - devtools: { - track: `IA: on ${ entry.namespace }`, - }, + } + evaluate( entry, event ); + if ( globalThis.IS_GUTENBERG_PLUGIN ) { + if ( globalThis.SCRIPT_DEBUG ) { + performance.measure( + `interactivity api on ${ entry.namespace }`, + { + // eslint-disable-next-line no-undef + start, + end: performance.now(), + detail: { + devtools: { + track: `IA: on ${ entry.namespace }`, }, - } - ); - } + }, + } + ); } - } ); - }; - } ); - } - ); + } + } ); + }; + } ); + } ); // data-wp-on-async--[event] directive( diff --git a/packages/interactivity/src/hooks.tsx b/packages/interactivity/src/hooks.tsx index fa56e8ece6a97..7899e3eafd228 100644 --- a/packages/interactivity/src/hooks.tsx +++ b/packages/interactivity/src/hooks.tsx @@ -74,16 +74,6 @@ interface DirectiveArgs { * context. */ evaluate: Evaluate; - /** - * Function that resolves a given path to value data in the store or the - * context. - */ - resolveEntry: ResolveEntry; - /** - * Function that evaluates resolved value data to a value either in the store - * or the context. - */ - evaluateResolved: EvaluateResolved; } export interface DirectiveCallback { @@ -100,17 +90,6 @@ interface DirectiveOptions { priority?: number; } -interface ResolvedEntry { - /** - * Resolved value, either a result or a function to get the result. - */ - value: any; - /** - * Whether a negation operator should be used on the result. - */ - hasNegationOperator: boolean; -} - export interface Evaluate { ( entry: DirectiveEntry, ...args: any[] ): any; } @@ -119,22 +98,6 @@ interface GetEvaluate { ( args: { scope: Scope } ): Evaluate; } -export interface ResolveEntry { - ( entry: DirectiveEntry ): ResolvedEntry; -} - -interface GetResolveEntry { - ( args: { scope: Scope } ): ResolveEntry; -} - -export interface EvaluateResolved { - ( resolved: ResolvedEntry, ...args: any[] ): any; -} - -interface GetEvaluateResolved { - ( args: { scope: Scope } ): EvaluateResolved; -} - type PriorityLevel = string[]; interface GetPriorityLevels { @@ -266,19 +229,9 @@ const resolve = ( path: string, namespace: string ) => { }; // Generate the evaluate function. -export const getEvaluate: GetEvaluate = ( scopeData ) => { - const resolveEntry = getResolveEntry( scopeData ); - const evaluateResolved = getEvaluateResolved( scopeData ); - return ( entry, ...args ) => { - const resolved = resolveEntry( entry ); - return evaluateResolved( resolved, ...args ); - }; -}; - -// Generate the resolveEntry function. -export const getResolveEntry: GetResolveEntry = +export const getEvaluate: GetEvaluate = ( { scope } ) => - ( entry ) => { + ( entry, ...args ) => { let { value: path, namespace } = entry; if ( typeof path !== 'string' ) { throw new Error( 'The `value` prop should be a string path' ); @@ -288,19 +241,6 @@ export const getResolveEntry: GetResolveEntry = path[ 0 ] === '!' && !! ( path = path.slice( 1 ) ); setScope( scope ); const value = resolve( path, namespace ); - resetScope(); - return { - value, - hasNegationOperator, - }; - }; - -// Generate the evaluateResolved function. -export const getEvaluateResolved: GetEvaluateResolved = - ( { scope } ) => - ( resolved, ...args ) => { - const { value, hasNegationOperator } = resolved; - setScope( scope ); const result = typeof value === 'function' ? value( ...args ) : value; resetScope(); return hasNegationOperator ? ! result : result; @@ -337,11 +277,6 @@ const Directives = ( { // element ref, state and props. const scope = useRef< Scope >( {} as Scope ).current; scope.evaluate = useCallback( getEvaluate( { scope } ), [] ); - scope.resolveEntry = useCallback( getResolveEntry( { scope } ), [] ); - scope.evaluateResolved = useCallback( - getEvaluateResolved( { scope } ), - [] - ); const { client, server } = useContext( context ); scope.context = client; scope.serverContext = server; @@ -373,8 +308,6 @@ const Directives = ( { element, context, evaluate: scope.evaluate, - resolveEntry: scope.resolveEntry, - evaluateResolved: scope.evaluateResolved, }; setScope( scope ); diff --git a/packages/interactivity/src/index.ts b/packages/interactivity/src/index.ts index b7d68fd200705..9d013e4e744ed 100644 --- a/packages/interactivity/src/index.ts +++ b/packages/interactivity/src/index.ts @@ -27,7 +27,6 @@ export { useCallback, useMemo, splitTask, - withSyncEvent, } from './utils'; export { useState, useRef } from 'preact/hooks'; diff --git a/packages/interactivity/src/scopes.ts b/packages/interactivity/src/scopes.ts index 51996535f0336..96e23c86ade73 100644 --- a/packages/interactivity/src/scopes.ts +++ b/packages/interactivity/src/scopes.ts @@ -7,12 +7,10 @@ import type { h as createElement, RefObject } from 'preact'; * Internal dependencies */ import { getNamespace } from './namespaces'; -import type { Evaluate, ResolveEntry, EvaluateResolved } from './hooks'; +import type { Evaluate } from './hooks'; export interface Scope { evaluate: Evaluate; - resolveEntry: ResolveEntry; - evaluateResolved: EvaluateResolved; context: object; serverContext: object; ref: RefObject< HTMLElement >; From 0dbd15f7abc1a214ad4bfad677a32e204a2bdd77 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 13 Feb 2025 11:18:46 -0800 Subject: [PATCH 14/18] Update evaluate() to no longer invoke functions (except where needed for BC) and move responsibility to the caller. --- packages/interactivity/src/directives.tsx | 71 +++++++++++++++++++---- packages/interactivity/src/hooks.tsx | 16 ++++- 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index a8d1931591d31..4568eba013c3b 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -150,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 ); @@ -176,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, { @@ -254,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( @@ -287,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( @@ -334,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( @@ -380,7 +403,10 @@ export default () => { } entries.forEach( async ( entry ) => { await splitTask(); - evaluate( entry, event ); + const result = evaluate( entry ); + if ( typeof result === 'function' ) { + result( event ); + } } ); }; } ); @@ -408,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|$)`, @@ -448,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 ); @@ -482,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; /* @@ -583,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 ) { @@ -593,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] @@ -615,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..8871110099e51 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,20 @@ 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.' + ); + return ! value( ...args ); + } + return value; + } + const result = value; resetScope(); return hasNegationOperator ? ! result : result; }; From 5a6eca3eba3a57cfe745941122d3d454a94ec90b Mon Sep 17 00:00:00 2001 From: Luis Herranz Date: Tue, 18 Feb 2025 10:00:05 +0100 Subject: [PATCH 15/18] Export withSyncEvent --- packages/interactivity/src/index.ts | 1 + 1 file changed, 1 insertion(+) 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'; From d2a990a993d6563940fe3eb5e2b9a0f85efbaa39 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 18 Feb 2025 11:56:06 -0800 Subject: [PATCH 16/18] Fix evaluate to return scoped function and always reset scope. --- packages/interactivity/src/hooks.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/src/hooks.tsx b/packages/interactivity/src/hooks.tsx index 8871110099e51..3d75fb03aa728 100644 --- a/packages/interactivity/src/hooks.tsx +++ b/packages/interactivity/src/hooks.tsx @@ -251,9 +251,18 @@ export const getEvaluate: GetEvaluate = warn( 'Using a function with a negation operator is deprecated and will stop working in WordPress 6.9. Please use derived state instead.' ); - return ! value( ...args ); + const functionResult = ! value( ...args ); + resetScope(); + return functionResult; } - return value; + // 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(); From a02b87378e98045aaefbc44e3799a65283e32dd0 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 18 Feb 2025 11:56:52 -0800 Subject: [PATCH 17/18] Update custom directives for e2e tests to account for evaluate behavior change. --- .../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 ++++- .../interactive-blocks/tovdom-islands/view.js | 6 ++++- 8 files changed, 58 insertions(+), 13 deletions(-) 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/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, From 145ceaddaab9b9c15756d20f74eb2e56fec319eb Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 20 Feb 2025 11:15:45 -0800 Subject: [PATCH 18/18] Update release version number in documentation. --- docs/reference-guides/interactivity-api/api-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference-guides/interactivity-api/api-reference.md b/docs/reference-guides/interactivity-api/api-reference.md index 99fcd83061fc4..bf2c1370ebcde 100644 --- a/docs/reference-guides/interactivity-api/api-reference.md +++ b/docs/reference-guides/interactivity-api/api-reference.md @@ -1258,7 +1258,7 @@ 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 `TODO: Add release number here!` / 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. +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: