Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the Interactivity API priority levels logic #52323

Merged
merged 7 commits into from Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -18,3 +18,7 @@
data-wp-test-context
></div>
</div>

<div data-testid="non-existent-directives">
<div data-wp-interactive ><div data-wp-non-existent-directive></div></div>
</div>
111 changes: 49 additions & 62 deletions packages/interactivity/src/hooks.js
Expand Up @@ -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
*/
Expand All @@ -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;
};

Expand Down Expand Up @@ -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 (
<RecursivePriorityLevel
directives={ byPriorityLevel }
element={ element }
evaluate={ evaluate }
originalProps={ originalProps }
/>
);
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: [ thisPriorityLevel, ...restPriorityLevels ],
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 ? (
<RecursivePriorityLevel
directives={ rest }
restPriorityLevels.length > 0 ? (
<Directives
directives={ directives }
priorityLevels={ restPriorityLevels }
element={ element }
evaluate={ evaluate }
originalProps={ originalProps }
elemRef={ elemRef }
/>
) : (
element
Expand All @@ -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 thisPriorityLevel ) {
const wrapper = directiveCallbacks[ directiveName ]?.( directiveArgs );
if ( wrapper !== undefined ) props.children = wrapper;
}

Expand All @@ -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 );
Expand Down
17 changes: 17 additions & 0 deletions test/e2e/specs/interactivity/directive-priorities.spec.ts
Expand Up @@ -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' );
} );
} );