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 <[email protected]>
Co-authored-by: c4rl0sbr4v0 <[email protected]>
  • Loading branch information
@DAreRodz @cbravobernal
3 people committed Feb 22, 2024
1 parent 8f1466f commit 2458eb6
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="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 @@ -7,6 +7,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

1 comment on commit 2458eb6

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 2458eb6.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8003197049
📝 Reported issues:

Please sign in to comment.