Skip to content

Commit

Permalink
Interactivity API: Improve context merges using proxies (#59187)
Browse files Browse the repository at this point in the history 
* First attempt using proxies

* Simplify context logic

* Return true from proxy `set` trap

* Update wp-context and wp-each implementation

* Rename `isObject` to `isPlainObject`

* Refactor and document proxifyContext

* Add test for new parent properties

* Do not proxify assigned objects

* Set the item after proxifying the context

* Rename contextIgnores to contextAssignedObjects

* Add comment to `udpateSignals`

* Fix context values when navigating back and forward

* Add more tests for navigations

* Update tests to cover a tricky case

* Update comment

* Fix context definition in navigation block

* Add test for object overwritten

* Update changelog

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: c4rl0sbr4v0 <cbravobernal@git.wordpress.org>
  • Loading branch information
@DAreRodz @cbravobernal @getdave
3 people authored and getdave committed Feb 27, 2024
1 parent 6a8bf9d commit 8374560
Show file tree
Hide file tree
Showing 6 changed files with 302 additions and 42 deletions.
8 changes: 6 additions & 2 deletions packages/block-library/src/navigation/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,11 @@ private static function get_nav_element_directives( $is_interactive ) {
// When adding to this array be mindful of security concerns.
$nav_element_context = data_wp_context(
array(
'overlayOpenedBy' => array(),
'overlayOpenedBy' => array(
'click' => false,
'hover' => false,
'focus' => false,
),
'type' => 'overlay',
'roleAttribute' => '',
'ariaLabel' => __( 'Menu' ),
Expand Down Expand Up @@ -764,7 +768,7 @@ function block_core_navigation_add_directives_to_submenu( $tags, $block_attribut
) ) {
// Add directives to the parent `<li>`.
$tags->set_attribute( 'data-wp-interactive', 'core/navigation' );
$tags->set_attribute( 'data-wp-context', '{ "submenuOpenedBy": {}, "type": "submenu" }' );
$tags->set_attribute( 'data-wp-context', '{ "submenuOpenedBy": { "click": false, "hover": false, "focus": false }, "type": "submenu" }' );
$tags->set_attribute( 'data-wp-watch', 'callbacks.initMenu' );
$tags->set_attribute( 'data-wp-on--focusout', 'actions.handleMenuFocusout' );
$tags->set_attribute( 'data-wp-on--keydown', 'actions.handleMenuKeydown' );
Expand Down
27 changes: 27 additions & 0 deletions packages/e2e-tests/plugins/interactive-blocks/directive-context/render.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
>
<!-- rendered during hydration -->
</pre>
<button data-testid="parent replace" data-wp-on--click="actions.replaceObj">Replace obj</button>
<button
data-testid="parent prop1"
name="prop1"
Expand Down Expand Up @@ -50,6 +51,14 @@
>
obj.prop5
</button>
<button
data-testid="parent new"
name="new"
value="modifiedFromParent"
data-wp-on--click="actions.updateContext"
>
new
</button>
<div
data-wp-context='{ "prop2":"child","prop3":"child","obj":{"prop5":"child","prop6":"child"},"array":[4,5,6] }'
>
Expand All @@ -59,6 +68,7 @@
>
<!-- rendered during hydration -->
</pre>
<button data-testid="child replace" data-wp-on--click="actions.replaceObj">Replace obj</button>
<button
data-testid="child prop1"
name="prop1"
Expand Down Expand Up @@ -127,10 +137,15 @@
data-wp-router-region="navigation"
data-wp-context='{ "text": "first page" }'
>
<div data-wp-context='{}'>
<div data-testid="navigation inherited text" data-wp-text="context.text"></div>
<div data-testid="navigation inherited text2" data-wp-text="context.text2"></div>
</div>
<div data-testid="navigation text" data-wp-text="context.text"></div>
<div data-testid="navigation new text" data-wp-text="context.newText"></div>
<button data-testid="toggle text" data-wp-on--click="actions.toggleText">Toggle Text</button>
<button data-testid="add new text" data-wp-on--click="actions.addNewText">Add New Text</button>
<button data-testid="add text2" data-wp-on--click="actions.addText2">Add Text 2</button>
<button data-testid="navigate" data-wp-on--click="actions.navigate">Navigate</button>
<button data-testid="async navigate" data-wp-on--click="actions.asyncNavigate">Async Navigate</button>
</div>
Expand All @@ -143,3 +158,15 @@
<span data-testid="non-default suffix context" data-wp-text="context.text"></span>
<span data-testid="default suffix context" data-wp-text="context.defaultText"></span>
</div>

<div
data-wp-interactive='directive-context'
data-wp-context='{ "list": [
{ "id": 1, "text": "Text 1" },
{ "id": 2, "text": "Text 2" }
] }'
>
<button data-testid="select 1" data-wp-on--click="actions.selectItem" value=1>Select 1</button>
<button data-testid="select 2" data-wp-on--click="actions.selectItem" value=2>Select 2</button>
<div data-testid="selected" data-wp-text="state.selected"></div>
</div>
36 changes: 29 additions & 7 deletions packages/e2e-tests/plugins/interactive-blocks/directive-context/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ store( 'directive-context', {
const ctx = getContext();
return JSON.stringify( ctx, undefined, 2 );
},
get selected() {
const { list, selected } = getContext();
return list.find( ( obj ) => obj === selected )?.text;
}
},
actions: {
updateContext( event ) {
Expand All @@ -22,19 +26,33 @@ store( 'directive-context', {
const ctx = getContext();
ctx.text = ctx.text === 'Text 1' ? 'Text 2' : 'Text 1';
},
selectItem( event ) {
const ctx = getContext();
const value = parseInt( event.target.value );
ctx.selected = ctx.list.find( ( { id } ) => id === value );
},
replaceObj() {
const ctx = getContext();
ctx.obj = { overwritten: true };
}
},
} );

const html = `
<div
data-wp-interactive='{ "namespace": "directive-context-navigate" }'
data-wp-router-region="navigation"
data-wp-context='{ "text": "second page" }'
data-wp-context='{ "text": "second page", "text2": "second page" }'
>
<div data-wp-context='{}'>
<div data-testid="navigation inherited text" data-wp-text="context.text"></div>
<div data-testid="navigation inherited text2" data-wp-text="context.text2"></div>
</div>
<div data-testid="navigation text" data-wp-text="context.text"></div>
<div data-testid="navigation new text" data-wp-text="context.newText"></div>
<button data-testid="toggle text" data-wp-on--click="actions.toggleText">Toggle Text</button>
<button data-testid="add new text" data-wp-on--click="actions.addNewText">Add new text</button>
<button data-testid="add new text" data-wp-on--click="actions.addNewText">Add New Text</button>
<button data-testid="add text2" data-wp-on--click="actions.addText2">Add Text 2</button>
<button data-testid="navigate" data-wp-on--click="actions.navigate">Navigate</button>
<button data-testid="async navigate" data-wp-on--click="actions.asyncNavigate">Async Navigate</button>
</div>`;
Expand All @@ -49,13 +67,17 @@ const { actions } = store( 'directive-context-navigate', {
const ctx = getContext();
ctx.newText = 'some new text';
},
addText2() {
const ctx = getContext();
ctx.text2 = 'some new text';
},
navigate() {
return import( '@wordpress/interactivity-router' ).then(
( { actions: routerActions } ) =>
routerActions.navigate(
window.location,
{ force: true, html },
)
( { actions: routerActions } ) => {
const url = new URL( window.location.href );
url.searchParams.set( 'next_page', 'true' );
return routerActions.navigate( url, { force: true, html } );
}
);

},
Expand Down
1 change: 1 addition & 0 deletions packages/interactivity/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug Fixes

- Only add proxies to plain objects inside the store. ([#59039](https://github.com/WordPress/gutenberg/pull/59039))
- Improve context merges using proxies. ([59187](https://github.com/WordPress/gutenberg/pull/59187))

## 5.0.0 (2024-02-09)

Expand Down
158 changes: 125 additions & 33 deletions packages/interactivity/src/directives.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,118 @@ import { useWatch, useInit } from './utils';
import { directive, getScope, getEvaluate } from './hooks';
import { kebabToCamelCase } from './utils/kebab-to-camelcase';

const isObject = ( item ) =>
item && typeof item === 'object' && ! Array.isArray( item );
// Assigned objects should be ignore during proxification.
const contextAssignedObjects = new WeakMap();

const mergeDeepSignals = ( target, source, overwrite ) => {
const isPlainObject = ( item ) =>
item && typeof item === 'object' && item.constructor === Object;

const descriptor = Reflect.getOwnPropertyDescriptor;

/**
* Wrap a context object with a proxy to reproduce the context stack. The proxy
* uses the passed `inherited` context as a fallback to look up for properties
* that don't exist in the given context. Also, updated properties are modified
* where they are defined, or added to the main context when they don't exist.
*
* By default, all plain objects inside the context are wrapped, unless it is
* listed in the `ignore` option.
*
* @param {Object} current Current context.
* @param {Object} inherited Inherited context, used as fallback.
*
* @return {Object} The wrapped context object.
*/
const proxifyContext = ( current, inherited = {} ) =>
new Proxy( current, {
get: ( target, k ) => {
// Subscribe to the inherited and current props.
const inheritedProp = inherited[ k ];
const currentProp = target[ k ];

// Return the inherited prop when missing in target.
if ( ! ( k in target ) && k in inherited ) {
return inheritedProp;
}

// Proxify plain objects that are not listed in `ignore`.
if (
k in target &&
! contextAssignedObjects.get( target )?.has( k ) &&
isPlainObject( peek( target, k ) )
) {
return proxifyContext( currentProp, inheritedProp );
}

// For other cases, return the value from target.
return currentProp;
},
set: ( target, k, value ) => {
const obj =
k in target || ! ( k in inherited ) ? target : inherited;

// Values that are objects should not be proxified so they point to
// the original object and don't inherit unexpected properties.
if ( value && typeof value === 'object' ) {
if ( ! contextAssignedObjects.has( obj ) ) {
contextAssignedObjects.set( obj, new Set() );
}
contextAssignedObjects.get( obj ).add( k );
}

obj[ k ] = value;
return true;
},
ownKeys: ( target ) => [
...new Set( [
...Object.keys( inherited ),
...Object.keys( target ),
] ),
],
getOwnPropertyDescriptor: ( target, k ) =>
descriptor( target, k ) || descriptor( inherited, k ),
} );

/**
* Recursively update values within a deepSignal object.
*
* @param {Object} target A deepSignal instance.
* @param {Object} source Object with properties to update in `target`
*/
const updateSignals = ( target, source ) => {
for ( const k in source ) {
if ( isObject( peek( target, k ) ) && isObject( peek( source, k ) ) ) {
mergeDeepSignals(
target[ `$${ k }` ].peek(),
source[ `$${ k }` ].peek(),
overwrite
);
} else if ( overwrite || typeof peek( target, k ) === 'undefined' ) {
target[ `$${ k }` ] = source[ `$${ k }` ];
if (
isPlainObject( peek( target, k ) ) &&
isPlainObject( peek( source, k ) )
) {
updateSignals( target[ `$${ k }` ].peek(), source[ k ] );
} else {
target[ k ] = source[ k ];
}
}
};

/**
* Recursively clone the passed object.
*
* @param {Object} source Source object.
* @return {Object} Cloned object.
*/
const deepClone = ( source ) => {
if ( isPlainObject( source ) ) {
return Object.fromEntries(
Object.entries( source ).map( ( [ key, value ] ) => [
key,
deepClone( value ),
] )
);
}
if ( Array.isArray( source ) ) {
return source.map( ( i ) => deepClone( i ) );
}
return source;
};

const newRule =
/(?:([\u0080-\uFFFF\w-%@]+) *:? *([^{;]+?);|([^;}{]*?) *{)|(}\s*)/g;
const ruleClean = /\/\*[^]*?\*\/| +/g;
Expand Down Expand Up @@ -105,22 +200,18 @@ export default () => {
( { suffix } ) => suffix === 'default'
);

currentValue.current = useMemo( () => {
if ( ! defaultEntry ) return null;
const { namespace, value } = defaultEntry;
const newValue = deepSignal( { [ namespace ]: value } );
mergeDeepSignals( newValue, inheritedValue );
mergeDeepSignals( currentValue.current, newValue, true );
return currentValue.current;
}, [ inheritedValue, defaultEntry ] );
// No change should be made if `defaultEntry` does not exist.
const contextStack = useMemo( () => {
if ( defaultEntry ) {
const { namespace, value } = defaultEntry;
updateSignals( currentValue.current, {
[ namespace ]: deepClone( value ),
} );
}
return proxifyContext( currentValue.current, inheritedValue );
}, [ defaultEntry, inheritedValue ] );

if ( currentValue.current ) {
return (
<Provider value={ currentValue.current }>
{ children }
</Provider>
);
}
return <Provider value={ contextStack }>{ children }</Provider>;
},
{ priority: 5 }
);
Expand Down Expand Up @@ -358,15 +449,16 @@ export default () => {

const list = evaluate( entry );
return list.map( ( item ) => {
const mergedContext = deepSignal( {} );

const itemProp =
suffix === 'default' ? 'item' : kebabToCamelCase( suffix );
const newValue = deepSignal( {
[ namespace ]: { [ itemProp ]: item },
} );
mergeDeepSignals( newValue, inheritedValue );
mergeDeepSignals( mergedContext, newValue, true );
const itemContext = deepSignal( { [ namespace ]: {} } );
const mergedContext = proxifyContext(
itemContext,
inheritedValue
);

// Set the item after proxifying the context.
mergedContext[ namespace ][ itemProp ] = item;

const scope = { ...getScope(), context: mergedContext };
const key = eachKey
Expand Down
Loading

0 comments on commit 8374560

Please sign in to comment.