From 84fbd4e098941c21ca91adb828a44b362a112c6e Mon Sep 17 00:00:00 2001 From: Luis Herranz Date: Fri, 21 Jul 2023 13:44:11 +0200 Subject: [PATCH] Improve the Interactivity API priority levels logic (#52323) * Remove the Directive component Co-authored-by: David Arenas * Move ref to first Recursive component * Pass all the directives to the directive callbacks * Don't create components for non-registered directives * Just store directive names in priorityLevels * Rename internal priority level variables --------- Co-authored-by: David Arenas --- .../directive-priorities/render.php | 4 + packages/interactivity/src/hooks.js | 111 ++++++++---------- .../directive-priorities.spec.ts | 17 +++ 3 files changed, 70 insertions(+), 62 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/directive-priorities/render.php b/packages/e2e-tests/plugins/interactive-blocks/directive-priorities/render.php index a5f0b045ab0d6..13e15b8e14171 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/directive-priorities/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/directive-priorities/render.php @@ -18,3 +18,7 @@ data-wp-test-context > + +
+
+
diff --git a/packages/interactivity/src/hooks.js b/packages/interactivity/src/hooks.js index e309990482ebc..81e2d0713fd0e 100644 --- a/packages/interactivity/src/hooks.js +++ b/packages/interactivity/src/hooks.js @@ -2,7 +2,7 @@ * External dependencies */ import { h, options, createContext, cloneElement } from 'preact'; -import { useRef, useMemo } from 'preact/hooks'; +import { useRef, useCallback } from 'preact/hooks'; /** * Internal dependencies */ @@ -12,10 +12,10 @@ import { rawStore as store } from './store'; const context = createContext( {} ); // WordPress Directives. -const directiveMap = {}; +const directiveCallbacks = {}; const directivePriorities = {}; -export const directive = ( name, cb, { priority = 10 } = {} ) => { - directiveMap[ name ] = cb; +export const directive = ( name, callback, { priority = 10 } = {} ) => { + directiveCallbacks[ name ] = callback; directivePriorities[ name ] = priority; }; @@ -47,69 +47,50 @@ const getEvaluate = // Separate directives by priority. The resulting array contains objects // of directives grouped by same priority, and sorted in ascending order. -const usePriorityLevels = ( directives ) => - useMemo( () => { - const byPriority = Object.entries( directives ).reduce( - ( acc, [ name, values ] ) => { - const priority = directivePriorities[ name ]; - if ( ! acc[ priority ] ) acc[ priority ] = {}; - acc[ priority ][ name ] = values; - - return acc; - }, - {} - ); - - return Object.entries( byPriority ) - .sort( ( [ p1 ], [ p2 ] ) => p1 - p2 ) - .map( ( [ , obj ] ) => obj ); - }, [ directives ] ); - -// Directive wrapper. -const Directive = ( { type, directives, props: originalProps } ) => { - const ref = useRef( null ); - const element = h( type, { ...originalProps, ref } ); - const evaluate = useMemo( () => getEvaluate( { ref } ), [] ); - - // Add wrappers recursively for each priority level. - const byPriorityLevel = usePriorityLevels( directives ); - return ( - - ); +const getPriorityLevels = ( directives ) => { + const byPriority = Object.keys( directives ).reduce( ( obj, name ) => { + if ( directiveCallbacks[ name ] ) { + const priority = directivePriorities[ name ]; + ( obj[ priority ] = obj[ priority ] || [] ).push( name ); + } + return obj; + }, {} ); + + return Object.entries( byPriority ) + .sort( ( [ p1 ], [ p2 ] ) => p1 - p2 ) + .map( ( [ , arr ] ) => arr ); }; // Priority level wrapper. -const RecursivePriorityLevel = ( { - directives: [ directives, ...rest ], +const Directives = ( { + directives, + priorityLevels: [ currentPriorityLevel, ...nextPriorityLevels ], element, evaluate, originalProps, + elemRef, } ) => { - // This element needs to be a fresh copy so we are not modifying an already - // rendered element with Preact's internal properties initialized. This - // prevents an error with changes in `element.props.children` not being - // reflected in `element.__k`. - element = cloneElement( element ); + // Initialize the DOM reference. + // eslint-disable-next-line react-hooks/rules-of-hooks + elemRef = elemRef || useRef( null ); + + // Create a reference to the evaluate function using the DOM reference. + // eslint-disable-next-line react-hooks/rules-of-hooks, react-hooks/exhaustive-deps + evaluate = evaluate || useCallback( getEvaluate( { ref: elemRef } ), [] ); + + // Create a fresh copy of the vnode element. + element = cloneElement( element, { ref: elemRef } ); // Recursively render the wrapper for the next priority level. - // - // Note that, even though we're instantiating a vnode with a - // `RecursivePriorityLevel` here, its render function will not be executed - // just yet. Actually, it will be delayed until the current render function - // has finished. That ensures directives in the current priorty level have - // run (and thus modified the passed `element`) before the next level. const children = - rest.length > 0 ? ( - 0 ? ( + ) : ( element @@ -118,8 +99,8 @@ const RecursivePriorityLevel = ( { const props = { ...originalProps, children }; const directiveArgs = { directives, props, element, context, evaluate }; - for ( const d in directives ) { - const wrapper = directiveMap[ d ]?.( directiveArgs ); + for ( const directiveName of currentPriorityLevel ) { + const wrapper = directiveCallbacks[ directiveName ]?.( directiveArgs ); if ( wrapper !== undefined ) props.children = wrapper; } @@ -133,12 +114,18 @@ options.vnode = ( vnode ) => { const props = vnode.props; const directives = props.__directives; delete props.__directives; - vnode.props = { - type: vnode.type, - directives, - props, - }; - vnode.type = Directive; + const priorityLevels = getPriorityLevels( directives ); + if ( priorityLevels.length > 0 ) { + vnode.props = { + directives, + priorityLevels, + originalProps: props, + type: vnode.type, + element: h( vnode.type, props ), + top: true, + }; + vnode.type = Directives; + } } if ( old ) old( vnode ); diff --git a/test/e2e/specs/interactivity/directive-priorities.spec.ts b/test/e2e/specs/interactivity/directive-priorities.spec.ts index 1734d6f5aecc3..56745bfad0c43 100644 --- a/test/e2e/specs/interactivity/directive-priorities.spec.ts +++ b/test/e2e/specs/interactivity/directive-priorities.spec.ts @@ -81,4 +81,21 @@ test.describe( 'Directives (w/ priority)', () => { ].join( ', ' ) ); } ); + + test( 'should not create a Directives component if none of the directives are registered', async ( { + page, + } ) => { + const nonExistentDirectives = page.getByTestId( + 'non-existent-directives' + ); + expect( + await nonExistentDirectives.evaluate( + // This returns undefined if type is a component. + ( node ) => { + return ( node as any ).__k.__k.__k[ 0 ].__k[ 0 ].__k[ 0 ] + .type; + } + ) + ).toBe( 'div' ); + } ); } );