Skip to content

Commit

Permalink
Interactivity API: Improve internal deepMerge function (WordPress#6…
Browse files Browse the repository at this point in the history
…4879)

* Move deepMerge to proxies and update implementation

* Remove Boolean casting in hasPropSignal implementation

* Test subscriptions to nested props updates

* Remove extraneous label

* Check signals are not created for nested props

* Fix getProxyFromObject types

* Add missing imports

* Add extra assertions to ensure correct signal creation

* Do not expose hasPropSignal from proxies/index

* Add missing TSDocs

* Update changelog

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: luisherranz <luisherranz@git.wordpress.org>
  • Loading branch information
3 people authored and bph committed Aug 31, 2024
1 parent 8eb6b93 commit 180af48
Show file tree
Hide file tree
Showing 8 changed files with 497 additions and 333 deletions.
4 changes: 4 additions & 0 deletions packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Enhancements

- Improve internal `deepMerge` function ([#64879](https://github.com/WordPress/gutenberg/pull/64879)).

### Bug Fixes

- Fix computeds without scope in Firefox ([#64825](https://github.com/WordPress/gutenberg/pull/64825)).
Expand Down
2 changes: 1 addition & 1 deletion packages/interactivity/src/proxies/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Internal dependencies
*/
export { proxifyState, peek } from './state';
export { proxifyState, peek, deepMerge } from './state';
export { proxifyStore } from './store';
18 changes: 16 additions & 2 deletions packages/interactivity/src/proxies/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Proxies for each object.
*/
const objToProxy = new WeakMap< object, object >();
const proxyToObj = new WeakMap< object, object >();

/**
* Namespaces for each created proxy.
Expand Down Expand Up @@ -38,6 +39,7 @@ export const createProxy = < T extends object >(
if ( ! objToProxy.has( obj ) ) {
const proxy = new Proxy( obj, handlers );
objToProxy.set( obj, proxy );
proxyToObj.set( proxy, obj );
proxyToNs.set( proxy, namespace );
}
return objToProxy.get( obj ) as T;
Expand All @@ -50,8 +52,9 @@ export const createProxy = < T extends object >(
* @param obj Object from which to know the proxy.
* @return Associated proxy or `undefined`.
*/
export const getProxyFromObject = < T extends object >( obj: T ): T =>
objToProxy.get( obj ) as T;
export const getProxyFromObject = < T extends object >(
obj: T
): T | undefined => objToProxy.get( obj ) as T;

/**
* Gets the namespace associated with the given proxy.
Expand Down Expand Up @@ -80,3 +83,14 @@ export const shouldProxy = (
! proxyToNs.has( candidate ) && supported.has( candidate.constructor )
);
};

/**
* Returns the target object for the passed proxy. If the passed object is not a registered proxy, the
* function returns `undefined`.
*
* @param proxy Proxy from which to know the target.
* @return The target object or `undefined`.
*/
export const getObjectFromProxy = < T extends object >(
proxy: T
): T | undefined => proxyToObj.get( proxy ) as T;
98 changes: 95 additions & 3 deletions packages/interactivity/src/proxies/state.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { signal, type Signal } from '@preact/signals';
import { batch, signal, type Signal } from '@preact/signals';

/**
* Internal dependencies
Expand All @@ -11,9 +11,11 @@ import {
getProxyFromObject,
getNamespaceFromProxy,
shouldProxy,
getObjectFromProxy,
} from './registry';
import { PropSignal } from './signals';
import { setNamespace, resetNamespace } from '../namespaces';
import { isPlainObject } from '../utils';

/**
* Set of built-in symbols.
Expand All @@ -33,6 +35,17 @@ const proxyToProps: WeakMap<
Map< string | symbol, PropSignal >
> = new WeakMap();

/**
* Checks wether a {@link PropSignal | `PropSignal`} instance exists for the
* given property in the passed proxy.
*
* @param proxy Proxy of a state object or array.
* @param key The property key.
* @return `true` when it exists; false otherwise.
*/
export const hasPropSignal = ( proxy: object, key: string ) =>
proxyToProps.has( proxy ) && proxyToProps.get( proxy )!.has( key );

/**
* Returns the {@link PropSignal | `PropSignal`} instance associated with the
* specified prop in the passed proxy.
Expand Down Expand Up @@ -152,7 +165,7 @@ const stateHandlers: ProxyHandler< object > = {
const result = Reflect.defineProperty( target, key, desc );

if ( result ) {
const receiver = getProxyFromObject( target );
const receiver = getProxyFromObject( target )!;
const prop = getPropSignal( receiver, key );
const { get, value } = desc;
if ( get ) {
Expand Down Expand Up @@ -189,7 +202,7 @@ const stateHandlers: ProxyHandler< object > = {
const result = Reflect.deleteProperty( target, key );

if ( result ) {
const prop = getPropSignal( getProxyFromObject( target ), key );
const prop = getPropSignal( getProxyFromObject( target )!, key );
prop.setValue( undefined );

if ( objToIterable.has( target ) ) {
Expand Down Expand Up @@ -248,3 +261,82 @@ export const peek = < T extends object, K extends keyof T >(
peeking = false;
}
};

/**
* Internal recursive implementation for {@link deepMerge | `deepMerge`}.
*
* @param target The target object.
* @param source The source object containing new values and props.
* @param override Whether existing props should be overwritten or not (`true`
* by default).
*/
const deepMergeRecursive = (
target: any,
source: any,
override: boolean = true
) => {
if ( isPlainObject( target ) && isPlainObject( source ) ) {
for ( const key in source ) {
const desc = Object.getOwnPropertyDescriptor( source, key );
if (
typeof desc?.get === 'function' ||
typeof desc?.set === 'function'
) {
if ( override || ! ( key in target ) ) {
Object.defineProperty( target, key, {
...desc,
configurable: true,
enumerable: true,
} );

const proxy = getProxyFromObject( target );
if ( desc?.get && proxy && hasPropSignal( proxy, key ) ) {
const propSignal = getPropSignal( proxy, key );
propSignal.setGetter( desc.get );
}
}
} else if ( isPlainObject( source[ key ] ) ) {
if ( ! ( key in target ) ) {
target[ key ] = {};
}

deepMergeRecursive( target[ key ], source[ key ], override );
} else if ( override || ! ( key in target ) ) {
Object.defineProperty( target, key, desc! );

const proxy = getProxyFromObject( target );
if ( desc?.value && proxy && hasPropSignal( proxy, key ) ) {
const propSignal = getPropSignal( proxy, key );
propSignal.setValue( desc.value );
}
}
}
}
};

/**
* Recursively update prop values inside the passed `target` and nested plain
* objects, using the values present in `source`. References to plain objects
* are kept, only updating props containing primitives or arrays. Arrays are
* replaced instead of merged or concatenated.
*
* If the `override` parameter is set to `false`, then all values in `target`
* are preserved, and only new properties from `source` are added.
*
* @param target The target object.
* @param source The source object containing new values and props.
* @param override Whether existing props should be overwritten or not (`true`
* by default).
*/
export const deepMerge = (
target: any,
source: any,
override: boolean = true
) =>
batch( () =>
deepMergeRecursive(
getObjectFromProxy( target ) || target,
source,
override
)
);
Loading

0 comments on commit 180af48

Please sign in to comment.