Skip to content

Commit

Permalink
Interactivity API: Fix non stable context reference on client side na…
Browse files Browse the repository at this point in the history
…vigation (#53876)

* Add failing test

* Fix the test
  • Loading branch information
luisherranz authored Aug 22, 2023
1 parent d7c1850 commit c57465c
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
<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="navigate" data-wp-on--click="actions.navigate">Navigate</button>
<button data-testid="async navigate" data-wp-on--click="actions.asyncNavigate">Async Navigate</button>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<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="navigate" data-wp-on--click="actions.navigate">Navigate</button>
<button data-testid="async navigate" data-wp-on--click="actions.asyncNavigate">Async Navigate</button>
</div>`;

store( {
Expand Down Expand Up @@ -41,6 +42,13 @@
force: true,
html,
} );
},
asyncNavigate: async ({ context }) => {
await navigate( window.location, {
force: true,
html,
} );
context.newText = 'changed from async action';
}
},
} );
Expand Down
18 changes: 8 additions & 10 deletions packages/interactivity/src/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,16 @@ import { directive } from './hooks';
const isObject = ( item ) =>
item && typeof item === 'object' && ! Array.isArray( item );

const mergeDeepSignals = ( target, source ) => {
const mergeDeepSignals = ( target, source, overwrite ) => {
for ( const k in source ) {
if ( typeof peek( target, k ) === 'undefined' ) {
target[ `$${ k }` ] = source[ `$${ k }` ];
} else if (
isObject( peek( target, k ) ) &&
isObject( peek( source, k ) )
) {
if ( isObject( peek( target, k ) ) && isObject( peek( source, k ) ) ) {
mergeDeepSignals(
target[ `$${ k }` ].peek(),
source[ `$${ k }` ].peek()
source[ `$${ k }` ].peek(),
overwrite
);
} else if ( overwrite || typeof peek( target, k ) === 'undefined' ) {
target[ `$${ k }` ] = source[ `$${ k }` ];
}
}
};
Expand All @@ -46,9 +44,9 @@ export default () => {
const currentValue = useRef( deepSignal( {} ) );
currentValue.current = useMemo( () => {
const newValue = deepSignal( newContext );
mergeDeepSignals( newValue, currentValue.current );
mergeDeepSignals( newValue, inheritedValue );
return newValue;
mergeDeepSignals( currentValue.current, newValue, true );
return currentValue.current;
}, [ newContext, inheritedValue ] );

return (
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/specs/interactivity/directives-context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,13 @@ test.describe( 'data-wp-context', () => {
await page.getByTestId( 'navigate' ).click();
await expect( element ).toHaveText( 'some new text' );
} );

test( 'should maintain the same context reference on async actions', async ( {
page,
} ) => {
const element = page.getByTestId( 'navigation new text' );
await expect( element ).toHaveText( '' );
await page.getByTestId( 'async navigate' ).click();
await expect( element ).toHaveText( 'changed from async action' );
} );
} );

0 comments on commit c57465c

Please sign in to comment.