From 2aa021cdd9a916750bd1c57b92d243bb3775d859 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sat, 16 Dec 2023 12:05:30 +0100 Subject: [PATCH 01/15] Use deepMerge and stores to update state --- packages/interactivity-router/src/index.js | 41 ++++++++++++++-------- packages/interactivity/src/index.ts | 5 +++ packages/interactivity/src/store.ts | 29 +++++++++------ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 724a2660df41d..6fb21f9a836ab 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -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(); @@ -45,21 +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 ); +const renderRegions = ( page ) => + 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; - } -}; /** * Load the given page forcing a full page reload. diff --git a/packages/interactivity/src/index.ts b/packages/interactivity/src/index.ts index 2a98900dfe379..3c91e919d91bd 100644 --- a/packages/interactivity/src/index.ts +++ b/packages/interactivity/src/index.ts @@ -2,6 +2,7 @@ * External dependencies */ import { h, cloneElement, render } from 'preact'; +import { batch } from '@preact/signals'; import { deepSignal } from 'deepsignal'; /** @@ -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'; @@ -43,6 +45,9 @@ export const privateApis = ( lock ): any => { cloneElement, render, deepSignal, + parseInitialData, + populateInitialData, + batch, }; } diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index bbdb167054e3b..b3aead19cb8e4 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -35,7 +35,7 @@ const deepMerge = ( target: any, source: any ) => { } }; -const parseInitialData = () => { +export const parseInitialData = () => { const storeTag = document.querySelector( `script[type="application/json"]#wp-interactivity-data` ); @@ -310,15 +310,22 @@ export function store( return stores.get( namespace ); } +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 ); From 1172bb0fb130baa8557bb687cd233737ef481547 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sat, 16 Dec 2023 12:05:43 +0100 Subject: [PATCH 02/15] Add test case --- .../router-navigate/render.php | 7 ++++ .../interactivity/router-navigate.spec.ts | 39 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php index 0b8e6e1012d1a..66a1a356e4a79 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -59,4 +59,11 @@ } } ?> +
+
+
+ + diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index fafa31341f463..a05b6fc3fba70 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -12,11 +12,18 @@ test.describe( 'Router navigate', () => { } ); const link1 = await utils.addPostWithBlock( 'test/router-navigate', { alias: 'router navigate - link 1', - attributes: { title: 'Link 1' }, + attributes: { + title: 'Link 1', + state: { prop1: 'link 1', prop3: 'link 1' }, + }, } ); await utils.addPostWithBlock( 'test/router-navigate', { alias: 'router navigate - main', - attributes: { title: 'Main', links: [ link1, link2 ] }, + attributes: { + title: 'Main', + links: [ link1, link2 ], + state: { prop1: 'main', prop2: 'main' }, + }, } ); await utils.addPostWithBlock( 'test/router-navigate', { alias: 'router navigate - disabled', @@ -191,4 +198,32 @@ test.describe( 'Router navigate', () => { // Check that client-navigations count has not increased. await expect( navigations ).toHaveText( '0' ); } ); + + test( 'should overwrite the state with the one serialized in the new page', async ( { + page, + } ) => { + const prop1 = page.getByTestId( 'prop1' ); + const prop2 = page.getByTestId( 'prop2' ); + const prop3 = page.getByTestId( 'prop3' ); + + await expect( prop1 ).toHaveText( 'main' ); + await expect( prop2 ).toHaveText( 'main' ); + await expect( prop3 ).toBeEmpty(); + + await page.getByTestId( 'link 1' ).click(); + + // New values for existing properties should change. + // Old values not overwritten should remain the same. + // New properties should appear. + await expect( prop1 ).toHaveText( 'link 1' ); + await expect( prop2 ).toHaveText( 'main' ); + await expect( prop3 ).toHaveText( 'link 1' ); + + await page.goBack(); + + // New added properties are preserved. + await expect( prop1 ).toHaveText( 'main' ); + await expect( prop2 ).toHaveText( 'main' ); + await expect( prop3 ).toHaveText( 'link 1' ); + } ); } ); From ece2467946cede1538a6d5ceec5c683e39632341 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sat, 16 Dec 2023 18:36:30 +0100 Subject: [PATCH 03/15] Do not render state when `data` is not present --- .../plugins/interactive-blocks/router-navigate/render.php | 6 +++++- test/e2e/specs/interactivity/router-navigate.spec.ts | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php index 66a1a356e4a79..f6102a96bf8c8 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -15,6 +15,10 @@ array( 'clientNavigationDisabled' => true ) ); } + +if ( isset( $attributes['data'] ) ) { + $initial_state = array( 'router' => array( 'data' => $attributes['data'] ) ); +} ?>
diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index a05b6fc3fba70..722bce1593cd5 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -14,7 +14,7 @@ test.describe( 'Router navigate', () => { alias: 'router navigate - link 1', attributes: { title: 'Link 1', - state: { prop1: 'link 1', prop3: 'link 1' }, + data: { prop1: 'link 1', prop3: 'link 1' }, }, } ); await utils.addPostWithBlock( 'test/router-navigate', { @@ -22,7 +22,7 @@ test.describe( 'Router navigate', () => { attributes: { title: 'Main', links: [ link1, link2 ], - state: { prop1: 'main', prop2: 'main' }, + data: { prop1: 'main', prop2: 'main' }, }, } ); await utils.addPostWithBlock( 'test/router-navigate', { From ffa8db8d746474322d7e380e58b1f8eda4e3a62c Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sat, 16 Dec 2023 18:37:14 +0100 Subject: [PATCH 04/15] Move batch call inside braces --- packages/interactivity-router/src/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 6fb21f9a836ab..2e387ed653809 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -58,7 +58,7 @@ const regionsToVdom = ( dom, { vdom } = {} ) => { }; // Render all interactive regions contained in the given page. -const renderRegions = ( page ) => +const renderRegions = ( page ) => { batch( () => { populateInitialData( page.initialData ); const attrName = `data-${ directivePrefix }-router-region`; @@ -71,6 +71,7 @@ const renderRegions = ( page ) => document.title = page.title; } } ); +}; /** * Load the given page forcing a full page reload. From 1e920bcc24e199c4ccf679c2bb25016aa9ce3357 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 22 Feb 2024 19:18:37 +0100 Subject: [PATCH 05/15] Move initial data related code together --- packages/interactivity/src/store.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index b3aead19cb8e4..9a1b73087818c 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -35,20 +35,6 @@ const deepMerge = ( target: any, source: any ) => { } }; -export 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(); @@ -310,6 +296,20 @@ 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 >; From 3b338cc3b91548277dec884d94c90aa34afde928 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Thu, 22 Feb 2024 19:25:15 +0100 Subject: [PATCH 06/15] Fix initial data tag ID --- .../plugins/interactive-blocks/router-navigate/render.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php index f6102a96bf8c8..76adfa5d0d792 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -17,7 +17,11 @@ } if ( isset( $attributes['data'] ) ) { - $initial_state = array( 'router' => array( 'data' => $attributes['data'] ) ); + $initial_state = array( + 'state' => array( + 'router' => array( 'data' => $attributes['data'] ), + ), + ); } ?> @@ -68,6 +72,6 @@
- From 648bd46b7ad79ac35c96c175f6de31427256aa68 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 23 Feb 2024 00:16:16 +0100 Subject: [PATCH 07/15] Prevent passing the state proxy as receiver --- packages/interactivity/src/store.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index 9a1b73087818c..750e4a9167f93 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -86,13 +86,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 ); } @@ -149,6 +149,9 @@ const handlers = { return result; }, + set( target: any, key: string, value: any ) { + return Reflect.set( target, key, value ); + }, }; /** From 80f28ad7e80ba831b9f14cb37d5b2b88b75ec341 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 23 Feb 2024 00:28:07 +0100 Subject: [PATCH 08/15] Add comment --- packages/interactivity/src/store.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index 750e4a9167f93..c681fd25be2f1 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -149,6 +149,7 @@ 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 ); }, From 638dcde06ee0d5fd06265fbac4750516df993a4e Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 23 Feb 2024 14:16:30 +0100 Subject: [PATCH 09/15] Move links to a nav element to prevent hydration issues --- .../router-navigate/render.php | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php index 76adfa5d0d792..f92480de77db5 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -47,26 +47,28 @@ Timeout NaN - $link ) { - $i = $key += 1; - echo <<link $i - link $i with hash +
From 333634eada32fb9fec7c552b0ff473d59e372a00 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 25 Feb 2024 19:13:34 +0100 Subject: [PATCH 10/15] Add failing test for getters merge --- .../router-navigate/render.php | 1 + .../router-navigate/view.js | 5 +++ .../interactivity/router-navigate.spec.ts | 40 ++++++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php index f92480de77db5..bab2383b387de 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -69,6 +69,7 @@ } ?> +
diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js index 1e137969936a0..800c5c9bcbbe4 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js @@ -8,6 +8,11 @@ const { state } = store( 'router', { status: 'idle', navigations: 0, timeout: 10000, + data: { + get getterProp() { + return `value from getter (${ state.data.prop1 })`; + } + } }, actions: { *navigate( e ) { diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index 722bce1593cd5..35edbf185dcb9 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -14,7 +14,11 @@ test.describe( 'Router navigate', () => { alias: 'router navigate - link 1', attributes: { title: 'Link 1', - data: { prop1: 'link 1', prop3: 'link 1' }, + data: { + getterProp: 'value from link1', + prop1: 'link 1', + prop3: 'link 1', + }, }, } ); await utils.addPostWithBlock( 'test/router-navigate', { @@ -22,7 +26,11 @@ test.describe( 'Router navigate', () => { attributes: { title: 'Main', links: [ link1, link2 ], - data: { prop1: 'main', prop2: 'main' }, + data: { + getterProp: 'value from main', + prop1: 'main', + prop2: 'main', + }, }, } ); await utils.addPostWithBlock( 'test/router-navigate', { @@ -226,4 +234,32 @@ test.describe( 'Router navigate', () => { await expect( prop2 ).toHaveText( 'main' ); await expect( prop3 ).toHaveText( 'link 1' ); } ); + + test( 'should not try to overwrite getters with values from the initial data', async ( { + page, + } ) => { + const title = page.getByTestId( 'title' ); + const getter = page.getByTestId( 'getterProp' ); + + // Title should start in 'Main' and the getter prop should be the one + // returned once hydrated. + await expect( title ).toHaveText( 'Main' ); + await expect( getter ).toHaveText( 'value from getter (main)' ); + + await page.getByTestId( 'link 1' ).click(); + + // Title should have changed. If not, that means there was an error + // during render. The getter should return the correct value. + await expect( title ).toHaveText( 'Link 1' ); + await expect( getter ).toHaveText( 'value from getter (link 1)' ); + + // Same behavior navigating back and forward. + await page.goBack(); + await expect( title ).toHaveText( 'Main' ); + await expect( getter ).toHaveText( 'value from getter (main)' ); + + await page.goForward(); + await expect( title ).toHaveText( 'Link 1' ); + await expect( getter ).toHaveText( 'value from getter (link 1)' ); + } ); } ); From ac084520c513467ba949848c3dacdd0289f9a5d5 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 25 Feb 2024 19:21:51 +0100 Subject: [PATCH 11/15] Simplify assignments --- packages/interactivity/src/store.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index c681fd25be2f1..2ed168f9eddd6 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -26,10 +26,10 @@ 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 ] } ); + target[ key ] = source[ key ]; } } } From 927db7c63ba98cb2619a0a159af06775b2a416eb Mon Sep 17 00:00:00 2001 From: David Arenas Date: Sun, 25 Feb 2024 19:25:42 +0100 Subject: [PATCH 12/15] Add a try-catch to ignore getter assignments --- packages/interactivity/src/store.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index 2ed168f9eddd6..ca65bbbc6aa18 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -29,7 +29,12 @@ const deepMerge = ( target: any, source: any ) => { if ( ! target[ key ] ) target[ key ] = {}; deepMerge( target[ key ], source[ key ] ); } else { - 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. + } } } } From b9419e2261971c3efb378447d336e811c4adf882 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 26 Feb 2024 11:06:23 +0100 Subject: [PATCH 13/15] Fix disabled navigation test --- .../router-navigate/render.php | 19 ++++++----- .../router-navigate/view.js | 12 ++++--- .../interactivity/router-navigate.spec.ts | 32 +++++++++++-------- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php index bab2383b387de..d1a7aa9211f10 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -17,10 +17,9 @@ } if ( isset( $attributes['data'] ) ) { - $initial_state = array( - 'state' => array( - 'router' => array( 'data' => $attributes['data'] ), - ), + wp_interactivity_state( + 'router', + array( 'data' => $attributes['data'] ) ); } ?> @@ -32,8 +31,12 @@

NaN + NaN
- - diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js index 800c5c9bcbbe4..b2d4ad0dc1dde 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js @@ -6,7 +6,10 @@ import { store } from '@wordpress/interactivity'; const { state } = store( 'router', { state: { status: 'idle', - navigations: 0, + navigations: { + pending: 0, + count: 0, + }, timeout: 10000, data: { get getterProp() { @@ -18,7 +21,8 @@ const { state } = store( 'router', { *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'; @@ -29,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'; } }, diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index 35edbf185dcb9..bc4c364929b3e 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -21,11 +21,23 @@ test.describe( 'Router navigate', () => { }, }, } ); + const link3 = await utils.addPostWithBlock( 'test/router-navigate', { + alias: 'router navigate - disabled', + attributes: { + title: 'Main (navigation disabled)', + links: [ link1, link2 ], + disableNavigation: true, + data: { + getterProp: 'value from main (navigation disabled)', + prop1: 'main (navigation disabled)', + }, + }, + } ); await utils.addPostWithBlock( 'test/router-navigate', { alias: 'router navigate - main', attributes: { title: 'Main', - links: [ link1, link2 ], + links: [ link1, link2, link3 ], data: { getterProp: 'value from main', prop1: 'main', @@ -33,14 +45,6 @@ test.describe( 'Router navigate', () => { }, }, } ); - await utils.addPostWithBlock( 'test/router-navigate', { - alias: 'router navigate - disabled', - attributes: { - title: 'Main (navigation disabled)', - links: [ link1, link2 ], - disableNavigation: true, - }, - } ); } ); test.beforeEach( async ( { interactivityUtils: utils, page } ) => { @@ -59,7 +63,7 @@ test.describe( 'Router navigate', () => { const link1 = utils.getLink( 'router navigate - link 1' ); const link2 = utils.getLink( 'router navigate - link 2' ); - const navigations = page.getByTestId( 'router navigations' ); + const navigations = page.getByTestId( 'router navigations pending' ); const status = page.getByTestId( 'router status' ); const title = page.getByTestId( 'title' ); @@ -104,7 +108,7 @@ test.describe( 'Router navigate', () => { } ) => { const link1 = utils.getLink( 'router navigate - link 1' ); - const navigations = page.getByTestId( 'router navigations' ); + const navigations = page.getByTestId( 'router navigations pending' ); const status = page.getByTestId( 'router status' ); const title = page.getByTestId( 'title' ); @@ -190,12 +194,12 @@ test.describe( 'Router navigate', () => { } ) => { await page.goto( utils.getLink( 'router navigate - disabled' ) ); - const navigations = page.getByTestId( 'router navigations' ); + const count = page.getByTestId( 'router navigations count' ); const status = page.getByTestId( 'router status' ); const title = page.getByTestId( 'title' ); // Check some elements to ensure the page has hydrated. - await expect( navigations ).toHaveText( '0' ); + await expect( count ).toHaveText( '0' ); await expect( status ).toHaveText( 'idle' ); await page.getByTestId( 'link 1' ).click(); @@ -204,7 +208,7 @@ test.describe( 'Router navigate', () => { await expect( title ).toHaveText( 'Link 1' ); // Check that client-navigations count has not increased. - await expect( navigations ).toHaveText( '0' ); + await expect( count ).toHaveText( '0' ); } ); test( 'should overwrite the state with the one serialized in the new page', async ( { From 12fdbd3105eaaf4552aeee602a9ef1a1dfce0012 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 26 Feb 2024 11:16:48 +0100 Subject: [PATCH 14/15] Test navigations to pages with csn disabled --- packages/interactivity-router/src/index.js | 6 +++- .../interactivity/router-navigate.spec.ts | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/interactivity-router/src/index.js b/packages/interactivity-router/src/index.js index 2e387ed653809..03d399338167c 100644 --- a/packages/interactivity-router/src/index.js +++ b/packages/interactivity-router/src/index.js @@ -188,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' diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index bc4c364929b3e..872fe9ea7ea52 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -266,4 +266,32 @@ test.describe( 'Router navigate', () => { await expect( title ).toHaveText( 'Link 1' ); await expect( getter ).toHaveText( 'value from getter (link 1)' ); } ); + + test( 'should force a page reload when navigating to a page with `clientNavigationDisabled`', async ( { + page, + } ) => { + const count = page.getByTestId( 'router navigations count' ); + const title = page.getByTestId( 'title' ); + + // Check the cound to ensure the page has hydrated. + await expect( count ).toHaveText( '0' ); + + // Navigate to a page without clientNavigationDisabled. + await page.getByTestId( 'link 1' ).click(); + + // Check the page has updated and the navigation count has increased. + await expect( title ).toHaveText( 'Link 1' ); + await expect( count ).toHaveText( '1' ); + + await page.goBack(); + await expect( title ).toHaveText( 'Main' ); + await expect( count ).toHaveText( '1' ); + + // Navigate to a page with clientNavigationDisabled. + await page.getByTestId( 'link 3' ).click(); + + // Check the page has updated and the navigation count is zero. + await expect( title ).toHaveText( 'Main (navigation disabled)' ); + await expect( count ).toHaveText( '0' ); + } ); } ); From 58f16ba8a79ae5adf4e4dd5b0bfd12b93de36f59 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 26 Feb 2024 12:00:14 +0100 Subject: [PATCH 15/15] Update changelogs --- packages/interactivity-router/CHANGELOG.md | 4 ++++ packages/interactivity/CHANGELOG.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/interactivity-router/CHANGELOG.md b/packages/interactivity-router/CHANGELOG.md index 72a9dd459a688..799425e4cd9d5 100644 --- a/packages/interactivity-router/CHANGELOG.md +++ b/packages/interactivity-router/CHANGELOG.md @@ -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) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index 8e48ead8429d3..1e81760b8d05c 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -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