Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactivity API: Fix navigate() issues related to initial state merges #57134

Merged
merged 15 commits into from
Feb 26, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@
array( 'clientNavigationDisabled' => true )
);
}

if ( isset( $attributes['data'] ) ) {
wp_interactivity_state(
'router',
array( 'data' => $attributes['data'] )
);
}
?>

<div
Expand All @@ -24,8 +31,12 @@
<h2 data-testid="title"><?php echo $attributes['title']; ?></h2>

<output
data-testid="router navigations"
data-wp-text="state.navigations"
data-testid="router navigations pending"
data-wp-text="state.navigations.pending"
>NaN</output>
<output
data-testid="router navigations count"
data-wp-text="state.navigations.count"
>NaN</output>
<output
data-testid="router status"
Expand All @@ -39,24 +50,30 @@
Timeout <span data-wp-text="state.timeout">NaN</span>
</button>

<?php
if ( isset( $attributes['links'] ) ) {
foreach ( $attributes['links'] as $key => $link ) {
$i = $key += 1;
echo <<<HTML
<a
data-testid="link $i"
data-wp-on--click="actions.navigate"
href="$link"
>link $i</a>
<a
data-testid="link $i with hash"
data-wp-on--click="actions.navigate"
data-force-navigation="true"
href="$link#link-$i-with-hash"
>link $i with hash</a>
<nav>
<?php
if ( isset( $attributes['links'] ) ) {
foreach ( $attributes['links'] as $key => $link ) {
$i = $key += 1;
echo <<<HTML
<a
data-testid="link $i"
data-wp-on--click="actions.navigate"
href="$link"
>link $i</a>
<a
data-testid="link $i with hash"
data-wp-on--click="actions.navigate"
data-force-navigation="true"
href="$link#link-$i-with-hash"
>link $i with hash</a>
HTML;
}
}
}
?>
?>
</nav>
<div data-testid="getterProp" data-wp-text="state.data.getterProp"></div>
<div data-testid="prop1" data-wp-text="state.data.prop1"></div>
<div data-testid="prop2" data-wp-text="state.data.prop2"></div>
<div data-testid="prop3" data-wp-text="state.data.prop3"></div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,23 @@ import { store } from '@wordpress/interactivity';
const { state } = store( 'router', {
state: {
status: 'idle',
navigations: 0,
navigations: {
pending: 0,
count: 0,
},
timeout: 10000,
data: {
get getterProp() {
return `value from getter (${ state.data.prop1 })`;
}
}
},
actions: {
*navigate( e ) {
e.preventDefault();

state.navigations += 1;
state.navigations.count += 1;
state.navigations.pending += 1;
state.status = 'busy';

const force = e.target.dataset.forceNavigation === 'true';
Expand All @@ -24,9 +33,9 @@ const { state } = store( 'router', {
);
yield actions.navigate( e.target.href, { force, timeout } );

state.navigations -= 1;
state.navigations.pending -= 1;

if ( state.navigations === 0 ) {
if ( state.navigations.pending === 0 ) {
state.status = 'idle';
}
},
Expand Down
4 changes: 4 additions & 0 deletions packages/interactivity-router/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Bug Fixes

- Fix navigate() issues related to initial state merges. ([#57134](https://github.com/WordPress/gutenberg/pull/57134))

## 1.2.0 (2024-02-21)

## 1.1.0 (2024-02-09)
Expand Down
44 changes: 30 additions & 14 deletions packages/interactivity-router/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@
*/
import { store, privateApis, getConfig } from '@wordpress/interactivity';

const { directivePrefix, getRegionRootFragment, initialVdom, toVdom, render } =
privateApis(
'I acknowledge that using private APIs means my theme or plugin will inevitably break in the next version of WordPress.'
);
const {
directivePrefix,
getRegionRootFragment,
initialVdom,
toVdom,
render,
parseInitialData,
populateInitialData,
batch,
} = privateApis(
'I acknowledge that using private APIs means my theme or plugin will inevitably break in the next version of WordPress.'
);

// The cache of visited and prefetched pages.
const pages = new Map();
Expand Down Expand Up @@ -45,20 +53,24 @@ const regionsToVdom = ( dom, { vdom } = {} ) => {
: toVdom( region );
} );
const title = dom.querySelector( 'title' )?.innerText;
return { regions, title };
const initialData = parseInitialData( dom );
return { regions, title, initialData };
};

// Render all interactive regions contained in the given page.
const renderRegions = ( page ) => {
const attrName = `data-${ directivePrefix }-router-region`;
document.querySelectorAll( `[${ attrName }]` ).forEach( ( region ) => {
const id = region.getAttribute( attrName );
const fragment = getRegionRootFragment( region );
render( page.regions[ id ], fragment );
batch( () => {
populateInitialData( page.initialData );
const attrName = `data-${ directivePrefix }-router-region`;
document.querySelectorAll( `[${ attrName }]` ).forEach( ( region ) => {
const id = region.getAttribute( attrName );
const fragment = getRegionRootFragment( region );
render( page.regions[ id ], fragment );
} );
if ( page.title ) {
document.title = page.title;
}
} );
if ( page.title ) {
document.title = page.title;
}
};

/**
Expand Down Expand Up @@ -176,7 +188,11 @@ export const { state, actions } = store( 'core/router', {
// out, and let the newer execution to update the HTML.
if ( navigatingTo !== href ) return;

if ( page ) {
if (
page &&
! page.initialData?.config?.[ 'core/router' ]
?.clientNavigationDisabled
) {
renderRegions( page );
window.history[
options.replace ? 'replaceState' : 'pushState'
Expand Down
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

### Bug Fixes

- Prevent passing state proxies as receivers to deepSignal proxy handlers. ([#57134](https://github.com/WordPress/gutenberg/pull/57134))

## 5.1.0 (2024-02-21)

### Bug Fixes
Expand Down
5 changes: 5 additions & 0 deletions packages/interactivity/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { h, cloneElement, render } from 'preact';
import { batch } from '@preact/signals';
import { deepSignal } from 'deepsignal';

/**
Expand All @@ -12,6 +13,7 @@ import { init, getRegionRootFragment, initialVdom } from './init';
import { directivePrefix } from './constants';
import { toVdom } from './vdom';
import { directive, getNamespace } from './hooks';
import { parseInitialData, populateInitialData } from './store';

export { store, getConfig } from './store';
export { getContext, getElement } from './hooks';
Expand Down Expand Up @@ -43,6 +45,9 @@ export const privateApis = ( lock ): any => {
cloneElement,
render,
deepSignal,
parseInitialData,
populateInitialData,
batch,
};
}

Expand Down
72 changes: 44 additions & 28 deletions packages/interactivity/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,20 @@ const deepMerge = ( target: any, source: any ) => {
if ( typeof getter === 'function' ) {
Object.defineProperty( target, key, { get: getter } );
} else if ( isObject( source[ key ] ) ) {
if ( ! target[ key ] ) Object.assign( target, { [ key ]: {} } );
if ( ! target[ key ] ) target[ key ] = {};
deepMerge( target[ key ], source[ key ] );
} else {
Object.assign( target, { [ key ]: source[ key ] } );
try {
target[ key ] = source[ key ];
} catch ( e ) {
// Assignemnts fail for properties that are only getters.
// When that's the case, the assignment is simply ignored.
}
}
}
}
};

const parseInitialData = () => {
const storeTag = document.querySelector(
`script[type="application/json"]#wp-interactivity-data`
);
if ( storeTag?.textContent ) {
try {
return JSON.parse( storeTag.textContent );
} catch ( e ) {
// Do nothing.
}
}
return {};
};

export const stores = new Map();
const rawStores = new Map();
const storeLocks = new Map();
Expand Down Expand Up @@ -100,13 +91,13 @@ const handlers = {
}
}

const result = Reflect.get( target, key, receiver );
const result = Reflect.get( target, key );

// Check if the proxy is the store root and no key with that name exist. In
// that case, return an empty object for the requested key.
if ( typeof result === 'undefined' && receiver === stores.get( ns ) ) {
const obj = {};
Reflect.set( target, key, obj, receiver );
Reflect.set( target, key, obj );
return proxify( obj, ns );
}

Expand Down Expand Up @@ -163,6 +154,10 @@ const handlers = {

return result;
},
// Prevents passing the current proxy as the receiver to the deepSignal.
set( target: any, key: string, value: any ) {
return Reflect.set( target, key, value );
},
};

/**
Expand Down Expand Up @@ -310,15 +305,36 @@ export function store(
return stores.get( namespace );
}

export const parseInitialData = ( dom = document ) => {
const storeTag = dom.querySelector(
`script[type="application/json"]#wp-interactivity-data`
);
if ( storeTag?.textContent ) {
try {
return JSON.parse( storeTag.textContent );
} catch ( e ) {
// Do nothing.
}
}
return {};
};

export const populateInitialData = ( data?: {
state?: Record< string, unknown >;
config?: Record< string, unknown >;
} ) => {
if ( isObject( data?.state ) ) {
Object.entries( data.state ).forEach( ( [ namespace, state ] ) => {
store( namespace, { state }, { lock: universalUnlock } );
} );
}
if ( isObject( data?.config ) ) {
Object.entries( data.config ).forEach( ( [ namespace, config ] ) => {
storeConfigs.set( namespace, config );
} );
}
};

// Parse and populate the initial state and config.
const data = parseInitialData();
if ( isObject( data?.state ) ) {
Object.entries( data.state ).forEach( ( [ namespace, state ] ) => {
store( namespace, { state }, { lock: universalUnlock } );
} );
}
if ( isObject( data?.config ) ) {
Object.entries( data.config ).forEach( ( [ namespace, config ] ) => {
storeConfigs.set( namespace, config );
} );
}
populateInitialData( data );
Loading
Loading