From 3cfcda7cd25bd852c020ef80a1d5887c5e6e2019 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Wed, 21 Aug 2024 21:07:13 +0200 Subject: [PATCH 01/11] Move `proxifyContext` to proxies folder --- packages/interactivity/src/directives.tsx | 108 +---------------- packages/interactivity/src/proxies/context.ts | 113 ++++++++++++++++++ packages/interactivity/src/proxies/index.ts | 1 + 3 files changed, 115 insertions(+), 107 deletions(-) create mode 100644 packages/interactivity/src/proxies/context.ts diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index b41d50722e376..b2c7afabf30bd 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -9,7 +9,7 @@ import { useContext, useMemo, useRef } from 'preact/hooks'; /** * Internal dependencies */ -import { proxifyState, peek } from './proxies'; +import { proxifyState, proxifyContext, peek } from './proxies'; /** * Internal dependencies @@ -25,112 +25,6 @@ import { import { directive, getEvaluate, type DirectiveEntry } from './hooks'; import { getScope } from './scopes'; -// Assigned objects should be ignored during proxification. -const contextAssignedObjects = new WeakMap(); - -// Store the context proxy and fallback for each object in the context. -const contextObjectToProxy = new WeakMap(); -const contextProxyToObject = new WeakMap(); -const contextObjectToFallback = new WeakMap(); - -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 current Current context. - * @param inherited Inherited context, used as fallback. - * - * @return The wrapped context object. - */ -const proxifyContext = ( current: object, inherited: object = {} ): object => { - // Update the fallback object reference when it changes. - contextObjectToFallback.set( current, inherited ); - if ( ! contextObjectToProxy.has( current ) ) { - const proxy = new Proxy( current, { - get: ( target: object, k: string ) => { - const fallback = contextObjectToFallback.get( current ); - // Always subscribe to prop changes in the current context. - const currentProp = target[ k ]; - - // Return the inherited prop when missing in target. - if ( ! ( k in target ) && k in fallback ) { - return fallback[ k ]; - } - - // Proxify plain objects that were not directly assigned. - if ( - k in target && - ! contextAssignedObjects.get( target )?.has( k ) && - isPlainObject( currentProp ) - ) { - return proxifyContext( currentProp ); - } - - // Return the stored proxy for `currentProp` when it exists. - if ( contextObjectToProxy.has( currentProp ) ) { - return contextObjectToProxy.get( currentProp ); - } - - /* - * For other cases, return the value from target, also - * subscribing to changes in the parent context when the current - * prop is not defined. - */ - return k in target ? currentProp : fallback[ k ]; - }, - set: ( target, k, value ) => { - const fallback = contextObjectToFallback.get( current ); - const obj = - k in target || ! ( k in fallback ) ? target : fallback; - - /* - * Assigned object values 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 ); - } - - /* - * When the value is a proxy, it's because it comes from the - * context, so the inner value is assigned instead. - */ - if ( contextProxyToObject.has( value ) ) { - const innerValue = contextProxyToObject.get( value ); - obj[ k ] = innerValue; - } else { - obj[ k ] = value; - } - - return true; - }, - ownKeys: ( target ) => [ - ...new Set( [ - ...Object.keys( contextObjectToFallback.get( current ) ), - ...Object.keys( target ), - ] ), - ], - getOwnPropertyDescriptor: ( target, k ) => - descriptor( target, k ) || - descriptor( contextObjectToFallback.get( current ), k ), - } ); - contextObjectToProxy.set( current, proxy ); - contextProxyToObject.set( proxy, current ); - } - return contextObjectToProxy.get( current ); -}; - /** * Recursively update values within a context object. * diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts new file mode 100644 index 0000000000000..40e63eacdd1d3 --- /dev/null +++ b/packages/interactivity/src/proxies/context.ts @@ -0,0 +1,113 @@ +/** + * Internal dependencies + */ +import { isPlainObject } from '../utils'; + +// Assigned objects should be ignored during proxification. +const contextAssignedObjects = new WeakMap(); + +// Store the context proxy and fallback for each object in the context. +const contextObjectToProxy = new WeakMap(); +const contextProxyToObject = new WeakMap(); +const contextObjectToFallback = new WeakMap(); + +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 current Current context. + * @param inherited Inherited context, used as fallback. + * + * @return The wrapped context object. + */ +export const proxifyContext = ( + current: object, + inherited: object = {} +): object => { + // Update the fallback object reference when it changes. + contextObjectToFallback.set( current, inherited ); + if ( ! contextObjectToProxy.has( current ) ) { + const proxy = new Proxy( current, { + get: ( target: object, k: string ) => { + const fallback = contextObjectToFallback.get( current ); + // Always subscribe to prop changes in the current context. + const currentProp = target[ k ]; + + // Return the inherited prop when missing in target. + if ( ! ( k in target ) && k in fallback ) { + return fallback[ k ]; + } + + // Proxify plain objects that were not directly assigned. + if ( + k in target && + ! contextAssignedObjects.get( target )?.has( k ) && + isPlainObject( currentProp ) + ) { + return proxifyContext( currentProp ); + } + + // Return the stored proxy for `currentProp` when it exists. + if ( contextObjectToProxy.has( currentProp ) ) { + return contextObjectToProxy.get( currentProp ); + } + + /* + * For other cases, return the value from target, also + * subscribing to changes in the parent context when the current + * prop is not defined. + */ + return k in target ? currentProp : fallback[ k ]; + }, + set: ( target, k, value ) => { + const fallback = contextObjectToFallback.get( current ); + const obj = + k in target || ! ( k in fallback ) ? target : fallback; + + /* + * Assigned object values 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 ); + } + + /* + * When the value is a proxy, it's because it comes from the + * context, so the inner value is assigned instead. + */ + if ( contextProxyToObject.has( value ) ) { + const innerValue = contextProxyToObject.get( value ); + obj[ k ] = innerValue; + } else { + obj[ k ] = value; + } + + return true; + }, + ownKeys: ( target ) => [ + ...new Set( [ + ...Object.keys( contextObjectToFallback.get( current ) ), + ...Object.keys( target ), + ] ), + ], + getOwnPropertyDescriptor: ( target, k ) => + descriptor( target, k ) || + descriptor( contextObjectToFallback.get( current ), k ), + } ); + contextObjectToProxy.set( current, proxy ); + contextProxyToObject.set( proxy, current ); + } + return contextObjectToProxy.get( current ); +}; diff --git a/packages/interactivity/src/proxies/index.ts b/packages/interactivity/src/proxies/index.ts index 1a495de6b469f..168e6467fc701 100644 --- a/packages/interactivity/src/proxies/index.ts +++ b/packages/interactivity/src/proxies/index.ts @@ -3,3 +3,4 @@ */ export { proxifyState, peek, deepMerge } from './state'; export { proxifyStore } from './store'; +export { proxifyContext } from './context'; From 9853ca326538881d3792c9654b64cd1e9a1dbaff Mon Sep 17 00:00:00 2001 From: David Arenas Date: Wed, 21 Aug 2024 21:13:03 +0200 Subject: [PATCH 02/11] Move context handlers outside --- packages/interactivity/src/proxies/context.ts | 145 +++++++++--------- 1 file changed, 73 insertions(+), 72 deletions(-) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index 40e63eacdd1d3..e2d4868436189 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -13,6 +13,78 @@ const contextObjectToFallback = new WeakMap(); const descriptor = Reflect.getOwnPropertyDescriptor; +const contextHandlers: ProxyHandler< object > = { + get: ( target: object, k: string ) => { + const fallback = contextObjectToFallback.get( target ); + // Always subscribe to prop changes in the current context. + const currentProp = target[ k ]; + + // Return the inherited prop when missing in target. + if ( ! ( k in target ) && k in fallback ) { + return fallback[ k ]; + } + + // Proxify plain objects that were not directly assigned. + if ( + k in target && + ! contextAssignedObjects.get( target )?.has( k ) && + isPlainObject( currentProp ) + ) { + return proxifyContext( currentProp ); + } + + // Return the stored proxy for `currentProp` when it exists. + if ( contextObjectToProxy.has( currentProp ) ) { + return contextObjectToProxy.get( currentProp ); + } + + /* + * For other cases, return the value from target, also + * subscribing to changes in the parent context when the current + * prop is not defined. + */ + return k in target ? currentProp : fallback[ k ]; + }, + set: ( target, k, value ) => { + const fallback = contextObjectToFallback.get( target ); + const obj = k in target || ! ( k in fallback ) ? target : fallback; + + /* + * Assigned object values 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 ); + } + + /* + * When the value is a proxy, it's because it comes from the + * context, so the inner value is assigned instead. + */ + if ( contextProxyToObject.has( value ) ) { + const innerValue = contextProxyToObject.get( value ); + obj[ k ] = innerValue; + } else { + obj[ k ] = value; + } + + return true; + }, + ownKeys: ( target ) => [ + ...new Set( [ + ...Object.keys( contextObjectToFallback.get( target ) ), + ...Object.keys( target ), + ] ), + ], + getOwnPropertyDescriptor: ( target, k ) => + descriptor( target, k ) || + descriptor( contextObjectToFallback.get( target ), k ), +}; + /** * 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 @@ -34,78 +106,7 @@ export const proxifyContext = ( // Update the fallback object reference when it changes. contextObjectToFallback.set( current, inherited ); if ( ! contextObjectToProxy.has( current ) ) { - const proxy = new Proxy( current, { - get: ( target: object, k: string ) => { - const fallback = contextObjectToFallback.get( current ); - // Always subscribe to prop changes in the current context. - const currentProp = target[ k ]; - - // Return the inherited prop when missing in target. - if ( ! ( k in target ) && k in fallback ) { - return fallback[ k ]; - } - - // Proxify plain objects that were not directly assigned. - if ( - k in target && - ! contextAssignedObjects.get( target )?.has( k ) && - isPlainObject( currentProp ) - ) { - return proxifyContext( currentProp ); - } - - // Return the stored proxy for `currentProp` when it exists. - if ( contextObjectToProxy.has( currentProp ) ) { - return contextObjectToProxy.get( currentProp ); - } - - /* - * For other cases, return the value from target, also - * subscribing to changes in the parent context when the current - * prop is not defined. - */ - return k in target ? currentProp : fallback[ k ]; - }, - set: ( target, k, value ) => { - const fallback = contextObjectToFallback.get( current ); - const obj = - k in target || ! ( k in fallback ) ? target : fallback; - - /* - * Assigned object values 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 ); - } - - /* - * When the value is a proxy, it's because it comes from the - * context, so the inner value is assigned instead. - */ - if ( contextProxyToObject.has( value ) ) { - const innerValue = contextProxyToObject.get( value ); - obj[ k ] = innerValue; - } else { - obj[ k ] = value; - } - - return true; - }, - ownKeys: ( target ) => [ - ...new Set( [ - ...Object.keys( contextObjectToFallback.get( current ) ), - ...Object.keys( target ), - ] ), - ], - getOwnPropertyDescriptor: ( target, k ) => - descriptor( target, k ) || - descriptor( contextObjectToFallback.get( current ), k ), - } ); + const proxy = new Proxy( current, contextHandlers ); contextObjectToProxy.set( current, proxy ); contextProxyToObject.set( proxy, current ); } From c40acbdc2f006d8d827ddf79a484f7810a798240 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 22 Aug 2024 09:27:35 +0200 Subject: [PATCH 03/11] Remove unused code --- packages/interactivity/src/proxies/context.ts | 46 +------------------ 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index e2d4868436189..99510418157af 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -1,11 +1,3 @@ -/** - * Internal dependencies - */ -import { isPlainObject } from '../utils'; - -// Assigned objects should be ignored during proxification. -const contextAssignedObjects = new WeakMap(); - // Store the context proxy and fallback for each object in the context. const contextObjectToProxy = new WeakMap(); const contextProxyToObject = new WeakMap(); @@ -24,20 +16,6 @@ const contextHandlers: ProxyHandler< object > = { return fallback[ k ]; } - // Proxify plain objects that were not directly assigned. - if ( - k in target && - ! contextAssignedObjects.get( target )?.has( k ) && - isPlainObject( currentProp ) - ) { - return proxifyContext( currentProp ); - } - - // Return the stored proxy for `currentProp` when it exists. - if ( contextObjectToProxy.has( currentProp ) ) { - return contextObjectToProxy.get( currentProp ); - } - /* * For other cases, return the value from target, also * subscribing to changes in the parent context when the current @@ -48,29 +26,7 @@ const contextHandlers: ProxyHandler< object > = { set: ( target, k, value ) => { const fallback = contextObjectToFallback.get( target ); const obj = k in target || ! ( k in fallback ) ? target : fallback; - - /* - * Assigned object values 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 ); - } - - /* - * When the value is a proxy, it's because it comes from the - * context, so the inner value is assigned instead. - */ - if ( contextProxyToObject.has( value ) ) { - const innerValue = contextProxyToObject.get( value ); - obj[ k ] = innerValue; - } else { - obj[ k ] = value; - } + obj[ k ] = value; return true; }, From 0f123997caac778d33667fa47acd32b19dc37730 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 22 Aug 2024 10:51:27 +0200 Subject: [PATCH 04/11] Update comment --- packages/interactivity/src/proxies/context.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index 99510418157af..358981331d2e8 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -47,9 +47,6 @@ const contextHandlers: ProxyHandler< object > = { * 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 current Current context. * @param inherited Inherited context, used as fallback. * From 8859cd5b63b006f24121d1361743c5429d6e6ebe Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 22 Aug 2024 10:51:38 +0200 Subject: [PATCH 05/11] Add TODO comment --- packages/interactivity/src/proxies/context.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index 358981331d2e8..ec61132db7dd2 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -1,3 +1,5 @@ +// TODO: Use the proxy registry to avoid multiple proxies on the same object. + // Store the context proxy and fallback for each object in the context. const contextObjectToProxy = new WeakMap(); const contextProxyToObject = new WeakMap(); From dc16445080949eb53b90f07535faa3de742135c1 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 26 Aug 2024 14:08:39 +0200 Subject: [PATCH 06/11] Remove unused code and rename `k` arg to `key` --- packages/interactivity/src/proxies/context.ts | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index ec61132db7dd2..d72151d6794ae 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -1,34 +1,27 @@ -// TODO: Use the proxy registry to avoid multiple proxies on the same object. - -// Store the context proxy and fallback for each object in the context. const contextObjectToProxy = new WeakMap(); -const contextProxyToObject = new WeakMap(); const contextObjectToFallback = new WeakMap(); const descriptor = Reflect.getOwnPropertyDescriptor; +// TODO: Use the proxy registry to avoid multiple proxies on the same object. const contextHandlers: ProxyHandler< object > = { - get: ( target: object, k: string ) => { + get: ( target, key ) => { const fallback = contextObjectToFallback.get( target ); // Always subscribe to prop changes in the current context. - const currentProp = target[ k ]; - - // Return the inherited prop when missing in target. - if ( ! ( k in target ) && k in fallback ) { - return fallback[ k ]; - } + const currentProp = target[ key ]; /* - * For other cases, return the value from target, also - * subscribing to changes in the parent context when the current - * prop is not defined. + * Return the value from `target` if it exists, or from `fallback` + * otherwise. This way, in the case the property doesn't exist either in + * `target` or `fallback`, it also subscribes to changes in the parent + * context. */ - return k in target ? currentProp : fallback[ k ]; + return key in target ? currentProp : fallback[ key ]; }, - set: ( target, k, value ) => { + set: ( target, key, value ) => { const fallback = contextObjectToFallback.get( target ); - const obj = k in target || ! ( k in fallback ) ? target : fallback; - obj[ k ] = value; + const obj = key in target || ! ( key in fallback ) ? target : fallback; + obj[ key ] = value; return true; }, @@ -38,9 +31,9 @@ const contextHandlers: ProxyHandler< object > = { ...Object.keys( target ), ] ), ], - getOwnPropertyDescriptor: ( target, k ) => - descriptor( target, k ) || - descriptor( contextObjectToFallback.get( target ), k ), + getOwnPropertyDescriptor: ( target, key ) => + descriptor( target, key ) || + descriptor( contextObjectToFallback.get( target ), key ), }; /** @@ -63,7 +56,6 @@ export const proxifyContext = ( if ( ! contextObjectToProxy.has( current ) ) { const proxy = new Proxy( current, contextHandlers ); contextObjectToProxy.set( current, proxy ); - contextProxyToObject.set( proxy, current ); } return contextObjectToProxy.get( current ); }; From ad6a9749b223870022d94228cc8dca4a0f5d513e Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 8 Sep 2024 18:18:21 +0200 Subject: [PATCH 07/11] Replace `updateContext` with `deepMerge` --- packages/interactivity/src/directives.tsx | 29 ++++------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/packages/interactivity/src/directives.tsx b/packages/interactivity/src/directives.tsx index b2c7afabf30bd..cde39d830499a 100644 --- a/packages/interactivity/src/directives.tsx +++ b/packages/interactivity/src/directives.tsx @@ -6,10 +6,6 @@ */ import { h as createElement, type RefObject } from 'preact'; import { useContext, useMemo, useRef } from 'preact/hooks'; -/** - * Internal dependencies - */ -import { proxifyState, proxifyContext, peek } from './proxies'; /** * Internal dependencies @@ -24,25 +20,7 @@ import { } from './utils'; import { directive, getEvaluate, type DirectiveEntry } from './hooks'; import { getScope } from './scopes'; - -/** - * Recursively update values within a context object. - * - * @param target A context instance. - * @param source Object with properties to update in `target`. - */ -const updateContext = ( target: any, source: any ) => { - for ( const k in source ) { - if ( - isPlainObject( peek( target, k ) ) && - isPlainObject( source[ k ] ) - ) { - updateContext( peek( target, k ) as object, source[ k ] ); - } else if ( ! ( k in target ) ) { - target[ k ] = source[ k ]; - } - } -}; +import { proxifyState, proxifyContext, deepMerge } from './proxies'; /** * Recursively clone the passed object. @@ -180,9 +158,10 @@ export default () => { `The value of data-wp-context in "${ namespace }" store must be a valid stringified JSON object.` ); } - updateContext( + deepMerge( currentValue.current, - deepClone( value ) as object + deepClone( value ) as object, + false ); result[ namespace ] = proxifyContext( currentValue.current, From d09316d8c355dd2ca09f817f23b0e83728e9c3ef Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 8 Sep 2024 19:05:41 +0200 Subject: [PATCH 08/11] Throw when calling `proxifyContext` on a context proxy --- packages/interactivity/src/proxies/context.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index d72151d6794ae..0773d3886da6e 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -1,5 +1,6 @@ const contextObjectToProxy = new WeakMap(); const contextObjectToFallback = new WeakMap(); +const contextProxies = new WeakSet(); const descriptor = Reflect.getOwnPropertyDescriptor; @@ -51,11 +52,15 @@ export const proxifyContext = ( current: object, inherited: object = {} ): object => { + if ( contextProxies.has( current ) ) { + throw Error( 'This object cannot be proxified.' ); + } // Update the fallback object reference when it changes. contextObjectToFallback.set( current, inherited ); if ( ! contextObjectToProxy.has( current ) ) { const proxy = new Proxy( current, contextHandlers ); contextObjectToProxy.set( current, proxy ); + contextProxies.add( proxy ); } return contextObjectToProxy.get( current ); }; From 2c17bf3371deba3b239c0833a86e3426ed5670a0 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 8 Sep 2024 19:29:24 +0200 Subject: [PATCH 09/11] Add unit tests --- .../src/proxies/test/context-proxy.ts | 270 ++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 packages/interactivity/src/proxies/test/context-proxy.ts diff --git a/packages/interactivity/src/proxies/test/context-proxy.ts b/packages/interactivity/src/proxies/test/context-proxy.ts new file mode 100644 index 0000000000000..d165666b57355 --- /dev/null +++ b/packages/interactivity/src/proxies/test/context-proxy.ts @@ -0,0 +1,270 @@ +/** + * External dependencies + */ +import { effect } from '@preact/signals'; + +/** + * Internal dependencies + */ +import { proxifyContext, proxifyState } from '../'; + +describe( 'Interactivity API', () => { + describe( 'context proxy', () => { + describe( 'get', () => { + it( 'should inherit props from its fallback', () => { + const fallback: any = proxifyContext( { a: 1 }, {} ); + const context: any = proxifyContext( { b: 2 }, fallback ); + + expect( context.a ).toBe( 1 ); + expect( context.b ).toBe( 2 ); + } ); + + it( "should inherit props from its fallback's fallback", () => { + const fallback2: any = proxifyContext( { a: 1 }, {} ); + const fallback1: any = proxifyContext( { b: 2 }, fallback2 ); + const context: any = proxifyContext( { c: 3 }, fallback1 ); + + expect( context.a ).toBe( 1 ); + expect( context.b ).toBe( 2 ); + expect( context.c ).toBe( 3 ); + } ); + + it( 'should list all inherited props', () => { + const fallback2: any = proxifyContext( { a: 1 }, {} ); + const fallback1: any = proxifyContext( { b: 2 }, fallback2 ); + const context: any = proxifyContext( { c: 3 }, fallback1 ); + + expect( Object.entries( context ) ).toEqual( [ + [ 'a', 1 ], + [ 'b', 2 ], + [ 'c', 3 ], + ] ); + } ); + + it( 'should shadow properties defined in its fallback', () => { + const fallback: any = proxifyContext( + { prop: 'fallback' }, + {} + ); + const context: any = proxifyContext( + { prop: 'context' }, + fallback + ); + + expect( context.prop ).toBe( 'context' ); + } ); + + it( 'should not inherit properties from nested objects', () => { + const fallback: any = proxifyContext( { obj: { a: 1 } }, {} ); + const context: any = proxifyContext( + { obj: { b: 2 } }, + fallback + ); + + expect( 'a' in context.obj ).toBe( false ); + expect( context.obj.b ).toBe( 2 ); + } ); + } ); + + describe( 'set', () => { + it( 'should modify props defined in it', () => { + const fallback: any = proxifyContext( + { prop: 'fallback' }, + {} + ); + const context: any = proxifyContext( + { prop: 'context' }, + fallback + ); + + context.prop = 'modified'; + + expect( context.prop ).toBe( 'modified' ); + expect( fallback.prop ).toBe( 'fallback' ); + } ); + + it( 'should modify props inherited from its fallback', () => { + const fallback: any = proxifyContext( + { prop: 'fallback' }, + {} + ); + const context: any = proxifyContext( {}, fallback ); + + context.prop = 'modified'; + + expect( context.prop ).toBe( 'modified' ); + expect( fallback.prop ).toBe( 'modified' ); + } ); + + it( 'should see changes in inherited props', () => { + const fallback: any = proxifyContext( + { prop: 'fallback' }, + {} + ); + const context: any = proxifyContext( {}, fallback ); + + fallback.prop = 'modified'; + + expect( context.prop ).toBe( 'modified' ); + expect( fallback.prop ).toBe( 'modified' ); + } ); + + it( 'should create non-inherited props in itself', () => { + const fallback: any = proxifyContext( {}, {} ); + const context: any = proxifyContext( {}, fallback ); + + context.prop = 'modified'; + + expect( context.prop ).toBe( 'modified' ); + expect( fallback.prop ).toBeUndefined(); + } ); + } ); + + describe( 'computations', () => { + it( 'should subscribe to changes in the current context', () => { + const fallback: any = proxifyContext( + proxifyState( 'test', { fromFallback: 'fallback' } ), + {} + ); + const context: any = proxifyContext( + proxifyState( 'test', { fromContext: 'context' } ), + fallback + ); + + const spy = jest.fn( () => context.fromContext ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( context.fromContext ).toBe( 'context' ); + + context.fromContext = 'modified'; + + expect( spy ).toHaveBeenCalledTimes( 2 ); + expect( context.fromContext ).toBe( 'modified' ); + } ); + + it( 'should subscribe to changes in inherited values', () => { + const fallback: any = proxifyContext( + proxifyState( 'test', { fromFallback: 'fallback' } ), + {} + ); + const context: any = proxifyContext( + proxifyState( 'test', { fromContext: 'context' } ), + fallback + ); + + const spy = jest.fn( () => context.fromFallback ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( context.fromFallback ).toBe( 'fallback' ); + + fallback.fromFallback = 'modified'; + + expect( spy ).toHaveBeenCalledTimes( 2 ); + expect( context.fromFallback ).toBe( 'modified' ); + } ); + + it( 'should subscribe to undefined props added to the context', () => { + const fallback: any = proxifyContext( + proxifyState( 'test', {} ), + {} + ); + const context: any = proxifyContext( + proxifyState( 'test', {} ), + fallback + ); + + const spy = jest.fn( () => context.fromContext ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( context.fromContext ).toBeUndefined(); + + context.fromContext = 'added'; + + expect( spy ).toHaveBeenCalledTimes( 2 ); + expect( context.fromContext ).toBe( 'added' ); + } ); + + it( 'should subscribe to undefined props added to the fallback', () => { + const fallback: any = proxifyContext( + proxifyState( 'test', {} ), + {} + ); + const context: any = proxifyContext( + proxifyState( 'test', {} ), + fallback + ); + + const spy = jest.fn( () => context.fromFallback ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( context.fromFallback ).toBeUndefined(); + + fallback.fromFallback = 'added'; + + expect( spy ).toHaveBeenCalledTimes( 2 ); + expect( context.fromFallback ).toBe( 'added' ); + } ); + + it( 'should subscribe to shadowed props', () => { + const fallbackState: any = proxifyState( 'test', {} ); + const fallback: any = proxifyContext( fallbackState, {} ); + + const contextState: any = proxifyState( 'test', {} ); + const context: any = proxifyContext( contextState, fallback ); + + const spy = jest.fn( () => context.prop ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( context.prop ).toBeUndefined(); + + fallbackState.prop = 'fromFallback'; + + expect( spy ).toHaveBeenCalledTimes( 2 ); + expect( context.prop ).toBe( 'fromFallback' ); + + contextState.prop = 'fromContext'; + + expect( spy ).toHaveBeenCalledTimes( 3 ); + expect( context.prop ).toBe( 'fromContext' ); + } ); + + it( 'should subscribe to any changes in listed props', () => { + const fallback: any = proxifyContext( + proxifyState( 'test', {} ), + {} + ); + const context: any = proxifyContext( + proxifyState( 'test', {} ), + fallback + ); + + const spy = jest.fn( () => Object.keys( context ) ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( Object.keys( context ) ).toEqual( [] ); + + context.fromContext = 'added'; + fallback.fromFallback = 'added'; + + expect( spy ).toHaveBeenCalledTimes( 3 ); + expect( Object.keys( context ).sort() ).toEqual( [ + 'fromContext', + 'fromFallback', + ] ); + } ); + } ); + + describe( 'proxifyContext', () => { + it( 'should throw when trying to re-proxify a proxy object', () => { + const context = proxifyContext( {}, {} ); + expect( () => proxifyContext( context, {} ) ).toThrow(); + } ); + } ); + } ); +} ); From 23927423d40731a195256b4d11f7b9bff20e9230 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 8 Sep 2024 20:29:12 +0200 Subject: [PATCH 10/11] Update changelog --- packages/interactivity/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index 9ea269fa269ad..6e8fbc6b74e08 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Enhancements + +- Refactor internal context proxies implementation ([#64713](https://github.com/WordPress/gutenberg/pull/64713)). + ### Bug Fixes - Prevent calling `proxifyContext` over an already-proxified context inside `wp-context` ([#65090](https://github.com/WordPress/gutenberg/pull/65090)). From d11f743fbbb65d8e3c2d52d75de157da975e19ae Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 17 Sep 2024 17:45:43 +0100 Subject: [PATCH 11/11] Added new tests to validate behavior with proxified state. Also included more descriptive error messages for re-proxification attempts. --- packages/interactivity/src/proxies/context.ts | 3 +++ .../src/proxies/test/context-proxy.ts | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/interactivity/src/proxies/context.ts b/packages/interactivity/src/proxies/context.ts index 0773d3886da6e..64517c91a6940 100644 --- a/packages/interactivity/src/proxies/context.ts +++ b/packages/interactivity/src/proxies/context.ts @@ -21,6 +21,9 @@ const contextHandlers: ProxyHandler< object > = { }, set: ( target, key, value ) => { const fallback = contextObjectToFallback.get( target ); + + // If the property exists in the current context, modify it. Otherwise, + // add it to the current context. const obj = key in target || ! ( key in fallback ) ? target : fallback; obj[ key ] = value; diff --git a/packages/interactivity/src/proxies/test/context-proxy.ts b/packages/interactivity/src/proxies/test/context-proxy.ts index d165666b57355..306b3e4a8aa94 100644 --- a/packages/interactivity/src/proxies/test/context-proxy.ts +++ b/packages/interactivity/src/proxies/test/context-proxy.ts @@ -64,6 +64,14 @@ describe( 'Interactivity API', () => { expect( 'a' in context.obj ).toBe( false ); expect( context.obj.b ).toBe( 2 ); } ); + + it( 'should work with the proxified state', () => { + const state = proxifyState( 'test', { a: 1 } ); + const fallback: any = proxifyContext( state, {} ); + const context: any = proxifyContext( state, fallback ); + + expect( context.a ).toBe( 1 ); + } ); } ); describe( 'set', () => { @@ -118,6 +126,17 @@ describe( 'Interactivity API', () => { expect( context.prop ).toBe( 'modified' ); expect( fallback.prop ).toBeUndefined(); } ); + + it( 'should work with the proxified state', () => { + const state = proxifyState( 'test', { a: 1 } ); + const fallback: any = proxifyContext( state, {} ); + const context: any = proxifyContext( {}, fallback ); + + context.a = 2; + + expect( context.a ).toBe( 2 ); + expect( state.a ).toBe( 2 ); + } ); } ); describe( 'computations', () => { @@ -263,7 +282,9 @@ describe( 'Interactivity API', () => { describe( 'proxifyContext', () => { it( 'should throw when trying to re-proxify a proxy object', () => { const context = proxifyContext( {}, {} ); - expect( () => proxifyContext( context, {} ) ).toThrow(); + expect( () => proxifyContext( context, {} ) ).toThrow( + 'This object cannot be proxified.' + ); } ); } ); } );