Interactivity API: Fix context object proxy references (#59553)

* Add failing test

* Fix returned proxy reference

* Add more reference tests

* Fix assignment of context objects

* Update comment

* Update changelog

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: c4rl0sbr4v0 <cbravobernal@git.wordpress.org>

packages/e2e-tests/plugins/interactive-blocks/directive-context/render.php

@@ -117,6 +117,24 @@
 			>
 				obj.prop6
 			</button>
+			<button
+				data-testid="child copy obj"
+				data-wp-on--click="actions.copyObj"
+			>
+				Copy obj
+			</button>
+			<div>
+				Is proxy preserved? <span
+					data-testid="is proxy preserved"
+					data-wp-text="state.isProxyPreserved"
+				></span>
+			</div>
+			<div>
+				Is proxy preserved on copy? <span
+					data-testid="is proxy preserved on copy"
+					data-wp-text="state.isProxyPreservedOnCopy"
+				></span>
+			</div>
 		</div>
 		<br />
 

packages/e2e-tests/plugins/interactive-blocks/directive-context/view.js

@@ -12,6 +12,15 @@ store( 'directive-context', {
 		get selected() {
 			const { list, selected } = getContext();
 			return list.find( ( obj ) => obj === selected )?.text;
+		},
+		get isProxyPreserved() {
+			const ctx = getContext();
+			const pointer = ctx.obj;
+			return pointer === ctx.obj;
+		},
+		get isProxyPreservedOnCopy() {
+			const { obj, obj2 } = getContext();
+			return obj === obj2;
 		}
 	},
 	actions: {
@@ -34,6 +43,10 @@ store( 'directive-context', {
 		replaceObj() {
 			const ctx = getContext();
 			ctx.obj = { overwritten: true };
+		},
+		copyObj() {
+			const ctx = getContext();
+			ctx.obj2 = ctx.obj;
 		}
 	},
 } );

packages/interactivity/CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Bug Fixes
 
 -   Prevent passing state proxies as receivers to deepSignal proxy handlers. ([#57134](https://github.com/WordPress/gutenberg/pull/57134))
+-   Keep the same references to objects defined inside the context. ([#59553](https://github.com/WordPress/gutenberg/pull/59553))
 
 ## 5.1.0 (2024-02-21)
 

packages/interactivity/src/directives.js

@@ -17,6 +17,11 @@ import { kebabToCamelCase } from './utils/kebab-to-camelcase';
 // Assigned objects should be ignore 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 isPlainObject = ( item ) =>
 	item && typeof item === 'object' && item.constructor === Object;
 
@@ -36,58 +41,87 @@ const descriptor = Reflect.getOwnPropertyDescriptor;
  *
  * @return {Object} The wrapped context object.
  */
-const proxifyContext = ( current, inherited = {} ) =>
-	new Proxy( current, {
-		get: ( target, k ) => {
-			// 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 inherited ) {
-				return inherited[ k ];
-			}
+const proxifyContext = ( current, inherited = {} ) => {
+	// Update the fallback object reference when it changes.
+	contextObjectToFallback.set( current, inherited );
+	if ( ! contextObjectToProxy.has( current ) ) {
+		const proxy = new Proxy( current, {
+			get: ( target, k ) => {
+				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 are not listed in `ignore`.
-			if (
-				k in target &&
-				! contextAssignedObjects.get( target )?.has( k ) &&
-				isPlainObject( peek( target, k ) )
-			) {
-				return proxifyContext( currentProp, inherited[ k ] );
-			}
+				// Proxify plain objects that were not directly assigned.
+				if (
+					k in target &&
+					! contextAssignedObjects.get( target )?.has( k ) &&
+					isPlainObject( peek( target, k ) )
+				) {
+					return proxifyContext( currentProp, fallback[ k ] );
+				}
 
-			/*
-			 * 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 : inherited[ k ];
-		},
-		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() );
+				// Return the stored proxy for `currentProp` when it exists.
+				if ( contextObjectToProxy.has( currentProp ) ) {
+					return contextObjectToProxy.get( currentProp );
 				}
-				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 ),
-	} );
+				/*
+				 * 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 deepSignal object.

test/e2e/specs/interactivity/directive-context.spec.ts

@@ -336,4 +336,46 @@ test.describe( 'data-wp-context', () => {
 		await expect( counterChild ).toHaveText( '1' );
 		await expect( changes ).toHaveText( '2' );
 	} );
+
+	test( 'references to the same context object should be preserved', async ( {
+		page,
+	} ) => {
+		const isProxyPreserved = page.getByTestId( 'is proxy preserved' );
+		await expect( isProxyPreserved ).toHaveText( 'true' );
+	} );
+
+	test( 'references to copied context objects should be preserved', async ( {
+		page,
+	} ) => {
+		await page.getByTestId( 'child copy obj' ).click();
+		const isProxyPreservedOnCopy = page.getByTestId(
+			'is proxy preserved on copy'
+		);
+		await expect( isProxyPreservedOnCopy ).toHaveText( 'true' );
+	} );
+
+	test( 'objects referenced from the context inherit properties where they are originally defined ', async ( {
+		page,
+	} ) => {
+		await page.getByTestId( 'child copy obj' ).click();
+
+		const childContextBefore = await parseContent(
+			page.getByTestId( 'child context' )
+		);
+
+		expect( childContextBefore.obj2.prop4 ).toBe( 'parent' );
+		expect( childContextBefore.obj2.prop5 ).toBe( 'child' );
+		expect( childContextBefore.obj2.prop6 ).toBe( 'child' );
+
+		await page.getByTestId( 'parent replace' ).click();
+
+		const childContextAfter = await parseContent(
+			page.getByTestId( 'child context' )
+		);
+
+		expect( childContextAfter.obj2.prop4 ).toBeUndefined();
+		expect( childContextAfter.obj2.prop5 ).toBe( 'child' );
+		expect( childContextAfter.obj2.prop6 ).toBe( 'child' );
+		expect( childContextAfter.obj2.overwritten ).toBe( true );
+	} );
 } );