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

Interactivity API: Improve context merges using proxies #59187

Merged
merged 18 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/block-library/src/navigation/index.php
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
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>
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
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
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 ) =>
Copy link
Contributor

@cbravobernal cbravobernal Feb 20, 2024

Choose a reason for hiding this comment

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

This will be similar as the one in store.ts
#59039

Should we move it to another file to share same code in different files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, maybe we could move it to utils.js. 🤔

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
Loading