Skip to content

Commit

Permalink
Interactivity API: Prevent unwanted subscriptions to inherited contex…
Browse files Browse the repository at this point in the history
…t props (#59273)

* Add failing test

* Fix subscription issue

* Fix typo

Co-authored-by: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com>

* Change multiline comment to block comment

Co-authored-by: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com>

* Update test name

Co-authored-by: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com>

---------

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: c4rl0sbr4v0 <cbravobernal@git.wordpress.org>
  • Loading branch information
3 people authored and getdave committed Feb 27, 2024
1 parent 7d1412b commit 493a97a
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,29 @@
<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>

<div
data-wp-interactive="directive-context-watch"
data-wp-context='{"counter":0}'
>
<button
data-testid="counter parent"
data-wp-on--click="actions.increment"
data-wp-text="context.counter"
></button>
<div
data-wp-context='{"counter":0, "changes":0}'
data-wp-watch="callbacks.countChanges"
>
<button
data-testid="counter child"
data-wp-on--click="actions.increment"
data-wp-text="context.counter"
>
</button>
<span
data-testid="counter changes"
data-wp-text="context.changes"
></span>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,21 @@ const { actions } = store( 'directive-context-navigate', {
},
},
} );

store( 'directive-context-watch', {
actions: {
increment: () => {
const ctx = getContext();
ctx.counter = ctx.counter + 1;
},
},
callbacks: {
countChanges: () => {
const ctx = getContext();
// Subscribe to changes in counter.
// eslint-disable-next-line no-unused-expressions
ctx.counter;
ctx.changes = ctx.changes + 1;
},
},
});
15 changes: 9 additions & 6 deletions packages/interactivity/src/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ const descriptor = Reflect.getOwnPropertyDescriptor;
const proxifyContext = ( current, inherited = {} ) =>
new Proxy( current, {
get: ( target, k ) => {
// Subscribe to the inherited and current props.
const inheritedProp = inherited[ 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 inheritedProp;
return inherited[ k ];
}

// Proxify plain objects that are not listed in `ignore`.
Expand All @@ -54,11 +53,15 @@ const proxifyContext = ( current, inherited = {} ) =>
! contextAssignedObjects.get( target )?.has( k ) &&
isPlainObject( peek( target, k ) )
) {
return proxifyContext( currentProp, inheritedProp );
return proxifyContext( currentProp, inherited[ k ] );
}

// For other cases, return the value from target.
return 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 : inherited[ k ];
},
set: ( target, k, value ) => {
const obj =
Expand Down
23 changes: 23 additions & 0 deletions test/e2e/specs/interactivity/directive-context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,4 +313,27 @@ test.describe( 'data-wp-context', () => {
await select2.click();
await expect( selected ).toHaveText( 'Text 2' );
} );

test( 'should not subscribe to parent context props if those also exist in child', async ( {
page,
} ) => {
const counterParent = page.getByTestId( 'counter parent' );
const counterChild = page.getByTestId( 'counter child' );
const changes = page.getByTestId( 'counter changes' );

await expect( counterParent ).toHaveText( '0' );
await expect( counterChild ).toHaveText( '0' );
// The first render counts, so the changes counter starts at 1.
await expect( changes ).toHaveText( '1' );

await counterParent.click();
await expect( counterParent ).toHaveText( '1' );
await expect( counterChild ).toHaveText( '0' );
await expect( changes ).toHaveText( '1' );

await counterChild.click();
await expect( counterParent ).toHaveText( '1' );
await expect( counterChild ).toHaveText( '1' );
await expect( changes ).toHaveText( '2' );
} );
} );

0 comments on commit 493a97a

Please sign in to comment.