From b0fa1e42bc1ae2dc5048a210bda1323f0d1b8b4b Mon Sep 17 00:00:00 2001 From: David Arenas Date: Tue, 5 Sep 2023 21:58:13 +0200 Subject: [PATCH 1/6] Add failing test --- .../router-navigate/block.json | 14 ++++ .../router-navigate/render.php | 38 +++++++++ .../router-navigate/view.js | 33 ++++++++ .../interactivity/router-navigate.spec.ts | 77 +++++++++++++++++++ 4 files changed, 162 insertions(+) create mode 100644 packages/e2e-tests/plugins/interactive-blocks/router-navigate/block.json create mode 100644 packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php create mode 100644 packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js create mode 100644 test/e2e/specs/interactivity/router-navigate.spec.ts diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/block.json b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/block.json new file mode 100644 index 00000000000000..bdb5a19e030624 --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/block.json @@ -0,0 +1,14 @@ +{ + "apiVersion": 2, + "name": "test/router-navigate", + "title": "E2E Interactivity tests - router navigate", + "category": "text", + "icon": "heart", + "description": "", + "supports": { + "interactivity": true + }, + "textdomain": "e2e-interactivity", + "viewScript": "router-navigate-view", + "render": "file:./render.php" +} diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php new file mode 100644 index 00000000000000..7213dbfe0dcccf --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -0,0 +1,38 @@ + + + +
+

+ + NaN + undefined + + $link ) { + $i = $key += 1; + echo <<link $i +HTML; + } + } + ?> +
diff --git a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js new file mode 100644 index 00000000000000..d1da34604c7b7e --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js @@ -0,0 +1,33 @@ +( ( { wp } ) => { + /** + * WordPress dependencies + */ + const { store, navigate } = wp.interactivity; + + store( { + state: { + router: { + status: 'idle', + navigations: 0, + } + }, + actions: { + router: { + navigate: async ( { state, event: e } ) => { + e.preventDefault(); + + state.router.navigations += 1; + state.router.status = 'busy'; + + await navigate( e.target.href ); + + state.router.navigations -= 1; + + if ( state.router.navigations === 0) { + state.router.status = 'idle'; + } + }, + }, + }, + } ); +} )( window ); diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts new file mode 100644 index 00000000000000..9d72b8554acb95 --- /dev/null +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -0,0 +1,77 @@ +/** + * Internal dependencies + */ +import { test, expect } from './fixtures'; + +test.describe( 'Router navigate', () => { + test.beforeAll( async ( { interactivityUtils: utils } ) => { + await utils.activatePlugins(); + const link2 = await utils.addPostWithBlock( 'test/router-navigate', { + alias: 'router navigate - link 2', + attributes: { title: 'Link 2' }, + } ); + const link1 = await utils.addPostWithBlock( 'test/router-navigate', { + alias: 'router navigate - link 1', + attributes: { title: 'Link 1' }, + } ); + await utils.addPostWithBlock( 'test/router-navigate', { + alias: 'router navigate - main', + attributes: { title: 'Main', links: [ link1, link2 ] }, + } ); + } ); + + test.beforeEach( async ( { interactivityUtils: utils, page } ) => { + await page.goto( utils.getLink( 'router navigate - main' ) ); + } ); + + test.afterAll( async ( { interactivityUtils: utils } ) => { + await utils.deactivatePlugins(); + await utils.deleteAllPosts(); + } ); + + test( 'should update the HTML only for the latest navigation', async ( { + page, + interactivityUtils: utils, + } ) => { + const link1 = utils.getLink( 'router navigate - link 1' ); + const link2 = utils.getLink( 'router navigate - link 2' ); + + const navigations = page.getByTestId( 'router navigations' ); + const status = page.getByTestId( 'router status' ); + const title = page.getByTestId( 'title' ); + + await expect( navigations ).toHaveText( '0' ); + await expect( status ).toHaveText( 'idle' ); + + let resolveLink1: Function; + let resolveLink2: Function; + + await page.route( link1, async ( route ) => { + await new Promise( ( r ) => ( resolveLink1 = r ) ); + await route.continue(); + } ); + await page.route( link2, async ( route ) => { + await new Promise( ( r ) => ( resolveLink2 = r ) ); + await route.continue(); + } ); + + await page.getByTestId( 'link 1' ).click( { delay: 60 } ); + await page.getByTestId( 'link 2' ).click( { delay: 60 } ); + + await expect( navigations ).toHaveText( '2' ); + await expect( status ).toHaveText( 'busy' ); + await expect( title ).toHaveText( 'Main' ); + + await Promise.resolve().then( () => resolveLink2() ); + + await expect( navigations ).toHaveText( '1' ); + await expect( status ).toHaveText( 'busy' ); + await expect( title ).toHaveText( 'Link 2' ); + + await Promise.resolve().then( () => resolveLink1() ); + + await expect( navigations ).toHaveText( '0' ); + await expect( status ).toHaveText( 'idle' ); + await expect( title ).toHaveText( 'Link 2' ); + } ); +} ); From 855cb75ded18023c2dd4742ef575bac800adfa65 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Tue, 5 Sep 2023 22:03:30 +0200 Subject: [PATCH 2/6] Update navigate function --- packages/interactivity/src/router.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/interactivity/src/router.js b/packages/interactivity/src/router.js index cc7925e2fc3981..e1b9b27ade72b2 100644 --- a/packages/interactivity/src/router.js +++ b/packages/interactivity/src/router.js @@ -78,11 +78,21 @@ const renderRegions = ( page ) => { } ); }; +// Variable to store the current navigation. +let navigatingTo = ''; + // Navigate to a new page. export const navigate = async ( href, options = {} ) => { const url = cleanUrl( href ); + navigatingTo = url; prefetch( url, options ); const page = await pages.get( url ); + + // Once the page is fetched, the destination URL could have changed (e.g., + // by clicking another link in the meantime). If so, bail out, and let the + // newer execution to update the HTML. + if ( navigatingTo !== url ) return; + if ( page ) { renderRegions( page ); window.history[ options.replace ? 'replaceState' : 'pushState' ]( From dabe2bf31dffa2e475a2298d783004602f490151 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Tue, 5 Sep 2023 23:10:30 +0200 Subject: [PATCH 3/6] Remove click delays --- test/e2e/specs/interactivity/router-navigate.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index 9d72b8554acb95..f3166f99a7de71 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -55,8 +55,8 @@ test.describe( 'Router navigate', () => { await route.continue(); } ); - await page.getByTestId( 'link 1' ).click( { delay: 60 } ); - await page.getByTestId( 'link 2' ).click( { delay: 60 } ); + await page.getByTestId( 'link 1' ).click(); + await page.getByTestId( 'link 2' ).click(); await expect( navigations ).toHaveText( '2' ); await expect( status ).toHaveText( 'busy' ); From 43556a085a2e8d8a2e5be460fa3217181af736c3 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Tue, 5 Sep 2023 23:33:09 +0200 Subject: [PATCH 4/6] Update changelog --- packages/interactivity/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index 636f78d3357cfc..19e82ff30471b4 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Enhancements + +- Improve `navigate()` to render only the result of the last call when multiple happen simultaneously. ([#54201](https://github.com/WordPress/gutenberg/pull/54201)) + ## 2.2.0 (2023-08-31) ### Enhancements From 83ae05f54bfc36a6ee3f05dab8152cc398542b02 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Wed, 6 Sep 2023 13:51:55 +0200 Subject: [PATCH 5/6] Add failing test for URLs with hashes --- .../router-navigate/render.php | 6 +++ .../router-navigate/view.js | 4 +- .../interactivity/router-navigate.spec.ts | 52 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) 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 7213dbfe0dcccf..683e0eaff6e894 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/render.php @@ -31,6 +31,12 @@ data-wp-on--click="actions.router.navigate" href="$link" >link $i + link $i with hash HTML; } } 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 d1da34604c7b7e..468b4a64482d3f 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js +++ b/packages/e2e-tests/plugins/interactive-blocks/router-navigate/view.js @@ -19,7 +19,9 @@ state.router.navigations += 1; state.router.status = 'busy'; - await navigate( e.target.href ); + const force = e.target.dataset.forceNavigation === 'true'; + + await navigate( e.target.href, { force } ); state.router.navigations -= 1; diff --git a/test/e2e/specs/interactivity/router-navigate.spec.ts b/test/e2e/specs/interactivity/router-navigate.spec.ts index f3166f99a7de71..308bc6fd986184 100644 --- a/test/e2e/specs/interactivity/router-navigate.spec.ts +++ b/test/e2e/specs/interactivity/router-navigate.spec.ts @@ -74,4 +74,56 @@ test.describe( 'Router navigate', () => { await expect( status ).toHaveText( 'idle' ); await expect( title ).toHaveText( 'Link 2' ); } ); + + test( 'should update the URL from the last navigation if only varies in the URL fragment', async ( { + page, + interactivityUtils: utils, + } ) => { + const link1 = utils.getLink( 'router navigate - link 1' ); + + const navigations = page.getByTestId( 'router navigations' ); + const status = page.getByTestId( 'router status' ); + const title = page.getByTestId( 'title' ); + + await expect( navigations ).toHaveText( '0' ); + await expect( status ).toHaveText( 'idle' ); + + const resolvers: Function[] = []; + + await page.route( link1, async ( route ) => { + await new Promise( ( r ) => resolvers.push( r ) ); + await route.continue(); + } ); + + await page.getByTestId( 'link 1' ).click(); + await page.getByTestId( 'link 1 with hash' ).click(); + + const href = ( await page + .getByTestId( 'link 1 with hash' ) + .getAttribute( 'href' ) ) as string; + + await expect( navigations ).toHaveText( '2' ); + await expect( status ).toHaveText( 'busy' ); + await expect( title ).toHaveText( 'Main' ); + + { + const resolver = resolvers.pop(); + if ( resolver ) resolver(); + } + + await expect( navigations ).toHaveText( '1' ); + await expect( status ).toHaveText( 'busy' ); + await expect( title ).toHaveText( 'Link 1' ); + await expect( page ).toHaveURL( href ); + + { + const resolver = resolvers.pop(); + if ( resolver ) resolver(); + } + + await expect( navigations ).toHaveText( '0' ); + await expect( status ).toHaveText( 'idle' ); + await expect( title ).toHaveText( 'Link 1' ); + await expect( page ).toHaveURL( href ); + } ); } ); From 578029c7dc17621e214ab47d955db6193152dba2 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Wed, 6 Sep 2023 13:53:21 +0200 Subject: [PATCH 6/6] Use href instead of url for the current navigation --- packages/interactivity/src/router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/src/router.js b/packages/interactivity/src/router.js index e1b9b27ade72b2..17b2b54e4d4577 100644 --- a/packages/interactivity/src/router.js +++ b/packages/interactivity/src/router.js @@ -84,14 +84,14 @@ let navigatingTo = ''; // Navigate to a new page. export const navigate = async ( href, options = {} ) => { const url = cleanUrl( href ); - navigatingTo = url; + navigatingTo = href; prefetch( url, options ); const page = await pages.get( url ); // Once the page is fetched, the destination URL could have changed (e.g., // by clicking another link in the meantime). If so, bail out, and let the // newer execution to update the HTML. - if ( navigatingTo !== url ) return; + if ( navigatingTo !== href ) return; if ( page ) { renderRegions( page );