diff --git a/package-lock.json b/package-lock.json index b3ed775..9f90a52 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,7 @@ "chai": "^5.1.1", "htm": "^3.1.1", "kleur": "^4.1.5", + "navigation-api-types": "^0.6.1", "preact": "^10.26.5", "preact-render-to-string": "^6.5.11", "sinon": "^18.0.0", @@ -3028,6 +3029,13 @@ "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" } }, + "node_modules/navigation-api-types": { + "version": "0.6.1", + "resolved": "https://registry.npmjs.org/navigation-api-types/-/navigation-api-types-0.6.1.tgz", + "integrity": "sha512-e1BbABfPRKLkBbZfAVuRFR2CLFWOtSt8e0ryJivjvLdw8yxD7ASPgyfl+klcGYvrPcP4zhOtZ4KpmQcEo1FgQw==", + "dev": true, + "license": "MIT" + }, "node_modules/negotiator": { "version": "0.6.3", "dev": true, diff --git a/package.json b/package.json index e1e61fa..e2f8847 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "chai": "^5.1.1", "htm": "^3.1.1", "kleur": "^4.1.5", + "navigation-api-types": "^0.6.1", "preact": "^10.26.5", "preact-render-to-string": "^6.5.11", "sinon": "^18.0.0", diff --git a/src/internal.d.ts b/src/internal.d.ts index eb1ca3a..b120caf 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -1,3 +1,4 @@ +/// import { Component } from 'preact'; export interface AugmentedComponent extends Component { diff --git a/src/router.d.ts b/src/router.d.ts index 1140aad..1d51195 100644 --- a/src/router.d.ts +++ b/src/router.d.ts @@ -43,7 +43,6 @@ interface LocationHook { path: string; pathParams: Record; searchParams: Record; - route: (url: string, replace?: boolean) => void; } export const useLocation: () => LocationHook; diff --git a/src/router.js b/src/router.js index 3161f63..7439330 100644 --- a/src/router.js +++ b/src/router.js @@ -7,61 +7,57 @@ import { useContext, useMemo, useReducer, useLayoutEffect, useRef } from 'preact * @typedef {import('./internal.d.ts').VNode} VNode */ -/** @type {boolean} */ -let push; +/** + * @param {NavigateEvent} e + */ +function isSameWindow(e) { + const sourceElement = /** @type {HTMLAnchorElement | null} */ (e.sourceElement); + return ( + !sourceElement || + !sourceElement.target || + /^(_self)?$/i.test(sourceElement.target) + ); +} + /** @type {string | RegExp | undefined} */ let scope; /** - * @param {string} href + * @param {URL} url * @returns {boolean} */ -function isInScope(href) { +function isInScope(url) { return !scope || (typeof scope == 'string' - ? href.startsWith(scope) - : scope.test(href) + ? url.pathname.startsWith(scope) + : scope.test(url.pathname) ); } /** * @param {string} state - * @param {MouseEvent | PopStateEvent | { url: string, replace?: boolean }} action + * @param {NavigateEvent} e */ -function handleNav(state, action) { - let url = ''; - push = undefined; - if (action && action.type === 'click') { - // ignore events the browser takes care of already: - if (action.ctrlKey || action.metaKey || action.altKey || action.shiftKey || action.button !== 0) { - return state; - } - - const link = action.composedPath().find(el => el.nodeName == 'A' && el.href), - href = link && link.getAttribute('href'); - if ( - !link || - link.origin != location.origin || - /^#/.test(href) || - !/^(_?self)?$/i.test(link.target) || - !isInScope(href) - ) { - return state; - } - - push = true; - action.preventDefault(); - url = link.href.replace(location.origin, ''); - } else if (action && action.url) { - push = !action.replace; - url = action.url; - } else { - url = location.pathname + location.search; +function handleNav(state, e) { + // TODO: Double-check this can't fail to parse. + // `.destination` is read-only, so I'm hoping it guarantees a valid URL. + const url = new URL(e.destination.url); + + if ( + !e.canIntercept || + e.hashChange || + e.downloadRequest !== null || + !isSameWindow(e) || + !isInScope(url) + ) { + // This is set purely for our test suite so that we can check + // if the event was ignored in another `navigate` handler. + e['preact-iso-ignored'] = true; + return state; } - if (push === true) history.pushState(null, '', url); - else if (push === false) history.replaceState(null, '', url); - return url; -}; + e.intercept(); + return url.href.replace(url.origin, ''); +} export const exec = (url, route, matches = {}) => { url = url.split('/').filter(Boolean); @@ -99,7 +95,6 @@ export const exec = (url, route, matches = {}) => { export function LocationProvider(props) { const [url, route] = useReducer(handleNav, location.pathname + location.search); if (props.scope) scope = props.scope; - const wasPush = push === true; const value = useMemo(() => { const u = new URL(url, location.origin); @@ -110,18 +105,14 @@ export function LocationProvider(props) { path, pathParams: {}, searchParams: Object.fromEntries(u.searchParams), - route: (url, replace) => route({ url, replace }), - wasPush }; }, [url]); useLayoutEffect(() => { - addEventListener('click', route); - addEventListener('popstate', route); + navigation.addEventListener('navigate', route); return () => { - removeEventListener('click', route); - removeEventListener('popstate', route); + navigation.removeEventListener('navigate', route); }; }, []); @@ -133,7 +124,7 @@ const RESOLVED = Promise.resolve(); export function Router(props) { const [c, update] = useReducer(c => c + 1, 0); - const { url, path, pathParams, searchParams, wasPush } = useLocation(); + const { url, path, pathParams, searchParams } = useLocation(); if (!url) { throw new Error(`preact-iso's must be used within a , see: https://github.com/preactjs/preact-iso#locationprovider`); } @@ -257,7 +248,7 @@ export function Router(props) { // The route is loaded and rendered. if (prevRoute.current !== path) { - if (wasPush) scrollTo(0, 0); + scrollTo(0, 0); if (props.onRouteChange) props.onRouteChange(url); prevRoute.current = path; @@ -265,7 +256,7 @@ export function Router(props) { if (props.onLoadEnd && isLoading.current) props.onLoadEnd(url); isLoading.current = false; - }, [path, wasPush, c]); + }, [path, c]); // Note: cur MUST render first in order to set didSuspend & prev. return routeChanged @@ -282,7 +273,7 @@ const RenderRef = ({ r }) => r.current; Router.Provider = LocationProvider; LocationProvider.ctx = createContext( - /** @type {import('./router.d.ts').LocationHook & { wasPush: boolean }} */ ({}) + /** @type {import('./router.d.ts').LocationHook}} */ ({}) ); const RouterContext = createContext( /** @type {{ rest: string }} */ ({}) diff --git a/test/router.test.js b/test/router.test.js index 8885731..b188c41 100644 --- a/test/router.test.js +++ b/test/router.test.js @@ -1,5 +1,5 @@ import { h, Fragment, render, Component, hydrate, options } from 'preact'; -import { useState } from 'preact/hooks'; +import { useEffect, useState } from 'preact/hooks'; import * as chai from 'chai'; import * as sinon from 'sinon'; import sinonChai from 'sinon-chai'; @@ -66,7 +66,8 @@ describe('Router', () => { scratch ); - loc.route('/a/'); + navigation.navigate('/a/'); + await sleep(1); expect(loc).to.deep.include({ @@ -195,7 +196,7 @@ describe('Router', () => { }); Home.resetHistory(); - loc.route('/profiles'); + navigation.navigate('/profiles'); await sleep(1); expect(scratch).to.have.property('textContent', 'Profiles'); @@ -211,7 +212,7 @@ describe('Router', () => { }); Profiles.resetHistory(); - loc.route('/profiles/bob'); + navigation.navigate('/profiles/bob'); await sleep(1); expect(scratch).to.have.property('textContent', 'Profile: bob'); @@ -229,7 +230,7 @@ describe('Router', () => { }); Profile.resetHistory(); - loc.route('/other?a=b&c=d'); + navigation.navigate('/other?a=b&c=d'); await sleep(1); expect(scratch).to.have.property('textContent', 'Fallback'); @@ -283,7 +284,7 @@ describe('Router', () => { expect(A).to.have.been.calledWith({ path: '/', searchParams: {}, pathParams: {} }); A.resetHistory(); - loc.route('/b'); + navigation.navigate('/b'); expect(scratch).to.have.property('innerHTML', '

A

hello

'); expect(A).not.to.have.been.called; @@ -303,18 +304,18 @@ describe('Router', () => { expect(B).to.have.been.calledWith({ path: '/b', searchParams: {}, pathParams: {} }); B.resetHistory(); - loc.route('/c'); - loc.route('/c?1'); - loc.route('/c'); + navigation.navigate('/c'); + navigation.navigate('/c?1'); + navigation.navigate('/c'); expect(scratch).to.have.property('innerHTML', '

B

hello

'); expect(B).not.to.have.been.called; await sleep(1); - loc.route('/c'); - loc.route('/c?2'); - loc.route('/c'); + navigation.navigate('/c'); + navigation.navigate('/c?2'); + navigation.navigate('/c'); expect(scratch).to.have.property('innerHTML', '

B

hello

'); // We should never re-invoke while loading (that would be a remount of the old route): @@ -332,7 +333,7 @@ describe('Router', () => { C.resetHistory(); B.resetHistory(); - loc.route('/b'); + navigation.navigate('/b'); await sleep(1); expect(scratch).to.have.property('innerHTML', '

B

hello

'); @@ -342,7 +343,7 @@ describe('Router', () => { A.resetHistory(); B.resetHistory(); - loc.route('/'); + navigation.navigate('/'); await sleep(1); expect(scratch).to.have.property('innerHTML', '

A

hello

'); @@ -386,21 +387,21 @@ describe('Router', () => { expect(renderRefCount).to.equal(2); renderRefCount = 0; - loc.route('/b/a'); + navigation.navigate('/b/a'); await sleep(10); expect(scratch).to.have.property('innerHTML', '

b/a

'); expect(renderRefCount).to.equal(4); renderRefCount = 0; - loc.route('/b/b'); + navigation.navigate('/b/b'); await sleep(10); expect(scratch).to.have.property('innerHTML', '

b/b

'); expect(renderRefCount).to.equal(1); renderRefCount = 0; - loc.route('/'); + navigation.navigate('/'); await sleep(10); expect(scratch).to.have.property('innerHTML', '

a

'); @@ -490,7 +491,8 @@ describe('Router', () => { loadEnd.resetHistory(); routeChange.resetHistory(); - loc.route('/b'); + navigation.navigate('/b'); + await sleep(1); expect(loadStart).to.have.been.calledWith('/b'); @@ -548,77 +550,82 @@ describe('Router', () => { }); describe('intercepted VS external links', () => { - const shouldIntercept = [null, '', '_self', 'self', '_SELF']; - const shouldNavigate = ['_top', '_parent', '_blank', 'custom', '_BLANK']; - - const clickHandler = sinon.fake(e => e.preventDefault()); - - const Route = sinon.fake( - () =>
- {[...shouldIntercept, ...shouldNavigate].map((target, i) => { - const url = '/' + i + '/' + target; - if (target === null) return target = {target + ''}; - return target = {target}; - })} -
- ); - - let pushState; + const shouldIntercept = [null, '', '_self', '_SELF']; + const shouldNavigate = ['_top', '_parent', '_blank', '_BLANK', 'custom']; + + // Create an ID as Chrome & Safari don't support the + // case-sensitivity flag for attribute selectors, meaning + // we can't distinguish between `_self` and `_SELF`. + const createId = (target) => { + if (target === null) return 'null'; + if (target === '') return 'emptyString'; + return target; + } - before(() => { - pushState = sinon.spy(history, 'pushState'); - addEventListener('click', clickHandler); - }); + const Links = () =>
+ {[...shouldIntercept, ...shouldNavigate].map((target, i) => { + const url = '/' + i + '/' + target; + return target = {target}; + })} +
; + + let triedToNavigate = false; + const handler = (e) => { + e.intercept(); + if (e['preact-iso-ignored']) { + triedToNavigate = true; + } + } - after(() => { - pushState.restore(); - removeEventListener('click', clickHandler); - }); + const App = () => { + useEffect(() => { + navigation.addEventListener('navigate', handler); + return () => navigation.removeEventListener('navigate', handler); + }, []); - beforeEach(async () => { - render( + return ( - - - + - , - scratch + ); - Route.resetHistory(); - clickHandler.resetHistory(); - pushState.resetHistory(); + } + + beforeEach(async () => { + render(, scratch); + await sleep(10); }); const getName = target => (target == null ? 'no target attribute' : `target="${target}"`); - // these should all be intercepted by the router. for (const target of shouldIntercept) { it(`should intercept clicks on links with ${getName(target)}`, async () => { - const sel = target == null ? `a:not([target])` : `a[target="${target}"]`; - const el = scratch.querySelector(sel); - if (!el) throw Error(`Unable to find link: ${sel}`); + const el = scratch.querySelector(`#${createId(target)}`); const url = el.getAttribute('href'); el.click(); await sleep(1); expect(loc).to.deep.include({ url }); - expect(Route).to.have.been.calledOnce; - expect(pushState).to.have.been.calledWith(null, '', url); - expect(clickHandler).to.have.been.called; + expect(triedToNavigate).to.be.false; + + triedToNavigate = false; }); } - // these should all navigate. + // TODO: These tests are rather flakey, for some reason the additional handler + // occasionally isn't running prior browser actually navigates away. for (const target of shouldNavigate) { it(`should allow default browser navigation for links with ${getName(target)}`, async () => { - const sel = target == null ? `a:not([target])` : `a[target="${target}"]`; - const el = scratch.querySelector(sel); - if (!el) throw Error(`Unable to find link: ${sel}`); - el.click(); + // Currently cross-window navigations, (e.g., `target="_blank"`), do not trigger a + // `navigate` event, which makes this difficult to observe. Per the spec, however, this + // might be a bug in Chrome's implementation: + // https://github.com/WICG/navigation-api?tab=readme-ov-file#restrictions-on-firing-canceling-and-responding + if (target === '_blank' || target === '_BLANK' || target === 'custom') return; + + scratch.querySelector(`#${createId(target)}`).click(); await sleep(1); - expect(Route).not.to.have.been.called; - expect(pushState).not.to.have.been.called; - expect(clickHandler).to.have.been.called; + expect(triedToNavigate).to.be.true; + + triedToNavigate = false; }); } }); @@ -627,8 +634,6 @@ describe('Router', () => { const shouldIntercept = ['/app', '/app/deeper']; const shouldNavigate = ['/site', '/site/deeper']; - const clickHandler = sinon.fake(e => e.preventDefault()); - const Links = () => ( <> Internal Link @@ -638,102 +643,81 @@ describe('Router', () => { ); - let pushState; - - before(() => { - pushState = sinon.spy(history, 'pushState'); - addEventListener('click', clickHandler); - }); - - after(() => { - pushState.restore(); - removeEventListener('click', clickHandler); - }); - - beforeEach(async () => { - clickHandler.resetHistory(); - pushState.resetHistory(); - }); + let triedToNavigate = false; + const handler = (e) => { + e.intercept(); + if (e['preact-iso-ignored']) { + triedToNavigate = true; + } + } - it('should intercept clicks on links matching the `scope` props (string)', async () => { - render( - - - - , - scratch - ); + it('should support the `scope` prop (string)', async () => { + const App = () => { + useEffect(() => { + navigation.addEventListener('navigate', handler); + return () => navigation.removeEventListener('navigate', handler); + }, []); + + return ( + + + + + ); + } + render(, scratch); + await sleep(10); for (const url of shouldIntercept) { scratch.querySelector(`a[href="${url}"]`).click(); await sleep(1); expect(loc).to.deep.include({ url }); - expect(pushState).to.have.been.calledWith(null, '', url); - expect(clickHandler).to.have.been.called; + expect(triedToNavigate).to.be.false; - pushState.resetHistory(); - clickHandler.resetHistory(); + triedToNavigate = false; } - }); - - it('should allow default browser navigation for links not matching the `scope` props (string)', async () => { - render( - - - - , - scratch - ); for (const url of shouldNavigate) { scratch.querySelector(`a[href="${url}"]`).click(); await sleep(1); - expect(pushState).not.to.have.been.called; - expect(clickHandler).to.have.been.called; + expect(triedToNavigate).to.be.true; - pushState.resetHistory(); - clickHandler.resetHistory(); + triedToNavigate = false; } }); - it('should intercept clicks on links matching the `scope` props (regex)', async () => { - render( - - - - , - scratch - ); + it('should support the `scope` prop (regex)', async () => { + const App = () => { + useEffect(() => { + navigation.addEventListener('navigate', handler); + return () => navigation.removeEventListener('navigate', handler); + }, []); + + return ( + + + + + ); + } + render(, scratch); + await sleep(10); for (const url of shouldIntercept) { scratch.querySelector(`a[href="${url}"]`).click(); await sleep(1); expect(loc).to.deep.include({ url }); - expect(pushState).to.have.been.calledWith(null, '', url); - expect(clickHandler).to.have.been.called; + expect(triedToNavigate).to.be.false; - pushState.resetHistory(); - clickHandler.resetHistory(); + triedToNavigate = false; } - }); - - it('should allow default browser navigation for links not matching the `scope` props (regex)', async () => { - render( - - - - , - scratch - ); for (const url of shouldNavigate) { scratch.querySelector(`a[href="${url}"]`).click(); await sleep(1); - expect(pushState).not.to.have.been.called; - expect(clickHandler).to.have.been.called; + expect(triedToNavigate).to.be.true; - pushState.resetHistory(); - clickHandler.resetHistory(); + triedToNavigate = false; } }); }); @@ -741,7 +725,14 @@ describe('Router', () => { it('should scroll to top when navigating forward', async () => { const scrollTo = sinon.spy(window, 'scrollTo'); - const Route = sinon.fake(() => ); + const Route = sinon.fake( + () => ( +
+ link +
+ ) + ); + render( @@ -756,7 +747,7 @@ describe('Router', () => { expect(Route).to.have.been.calledOnce; Route.resetHistory(); - loc.route('/programmatic'); + navigation.navigate('/programmatic'); await sleep(1); expect(loc).to.deep.include({ url: '/programmatic' }); @@ -779,14 +770,13 @@ describe('Router', () => { }); it('should ignore clicks on document fragment links', async () => { - const pushState = sinon.spy(history, 'pushState'); - const Route = sinon.fake( () => ); + render( @@ -799,7 +789,6 @@ describe('Router', () => { scratch ); - expect(Route).to.have.been.calledOnce; Route.resetHistory(); scratch.querySelector('a[href="#foo"]').click(); @@ -808,7 +797,6 @@ describe('Router', () => { // NOTE: we don't (currently) propagate in-page anchor navigations into context, to avoid useless renders. expect(loc).to.deep.include({ url: '/' }); expect(Route).not.to.have.been.called; - expect(pushState).not.to.have.been.called; expect(location.hash).to.equal('#foo'); scratch.querySelector('a[href="/other#bar"]').click(); @@ -816,14 +804,10 @@ describe('Router', () => { expect(Route).to.have.been.calledOnce; expect(loc).to.deep.include({ url: '/other#bar', path: '/other' }); - expect(pushState).to.have.been.called; expect(location.hash).to.equal('#bar'); - - pushState.restore(); }); it('should normalize children', async () => { - const pushState = sinon.spy(history, 'pushState'); const Route = sinon.fake(() => foo); const routes = ['/foo', '/bar']; @@ -846,9 +830,6 @@ describe('Router', () => { expect(Route).to.have.been.calledOnce; expect(loc).to.deep.include({ url: '/foo#foo', path: '/foo' }); - expect(pushState).to.have.been.called; - - pushState.restore(); }); it('should match nested routes', async () => { @@ -905,25 +886,30 @@ describe('Router', () => { }); it('should replace the current URL', async () => { - const pushState = sinon.spy(history, 'pushState'); - const replaceState = sinon.spy(history, 'replaceState'); - render( + null} /> null} /> + null} /> , scratch ); - loc.route("/foo", true); - expect(pushState).not.to.have.been.called; - expect(replaceState).to.have.been.calledWith(null, "", "/foo"); + navigation.navigate('/foo'); + navigation.navigate('/bar', { history: 'replace' }); + + const entries = navigation.entries(); - pushState.restore(); - replaceState.restore(); + // Top of the stack + const last = new URL(entries[entries.length - 1].url); + expect(last.pathname).to.equal('/bar'); + + // Entry before + const secondLast = new URL(entries[entries.length - 2].url); + expect(secondLast.pathname).to.equal('/'); }); it('should support using `Router` as an implicit suspense boundary', async () => { @@ -965,11 +951,38 @@ describe('Router', () => { expect(scratch).to.have.property('textContent', 'data'); }); + it('should support navigating backwards and forwards', async () => { + render( + + + null} /> + null} /> + + + , + scratch + ); + + navigation.navigate('/foo'); + await sleep(10); + + expect(loc).to.deep.include({ url: '/foo', path: '/foo', searchParams: {} }); + + navigation.back(); + await sleep(10); + + expect(loc).to.deep.include({ url: '/', path: '/', searchParams: {} }); + + navigation.forward(); + await sleep(10); + + expect(loc).to.deep.include({ url: '/foo', path: '/foo', searchParams: {} }); + }); + it('should intercept clicks on links inside open shadow DOM', async () => { const shadowlink = document.createElement('a'); shadowlink.href = '/shadow'; shadowlink.textContent = 'Shadow Link'; - shadowlink.addEventListener('click', e => e.preventDefault()); const attachShadow = (el) => { if (!el || el.shadowRoot) return;