diff --git a/src/focus/focus.js b/src/focus/focus.js index a5f9d29a..bf2e0f77 100644 --- a/src/focus/focus.js +++ b/src/focus/focus.js @@ -41,6 +41,8 @@ export default { return focusedComponent }, set(component, event) { + if (component === undefined || component.eol === true) return + clearTimeout(setFocusTimeout) // early return if already focused @@ -75,7 +77,7 @@ export default { // and finally set focus to the leaf component setFocusTimeout = setTimeout( - () => setFocus(component, event), + () => component.eol !== true && setFocus(component, event), this.hold === true ? Settings.get('holdTimeout', DEFAULT_HOLD_TIMEOUT_MS) : 0 ) }, diff --git a/src/focus/focus.test.js b/src/focus/focus.test.js index 27949adb..177a9390 100644 --- a/src/focus/focus.test.js +++ b/src/focus/focus.test.js @@ -214,6 +214,48 @@ test('Unfocus partial focus chain', (assert) => { }) }) +test('Ignore focus requests for destroyed components', (assert) => { + const focusedComponent = Focus.get() + const destroyedComponent = { + eol: true, + [symbols.lifecycle]: { + state: 'destroy', + }, + } + + Focus.set(destroyedComponent) + + assert.equal(Focus.get(), focusedComponent, 'Destroyed component should not receive focus') + assert.end() +}) + +test('Ignore delayed focus callback when component is destroyed', (assert) => { + const focusedComponent = Focus.get() + const component = { + eol: false, + [symbols.lifecycle]: { + state: 'init', + }, + } + + Focus.set(component) + component.eol = true + + setTimeout(() => { + assert.equal( + Focus.get(), + focusedComponent, + 'Destroyed component should not receive delayed focus' + ) + assert.equal( + component[symbols.lifecycle].state, + 'init', + 'Destroyed component lifecycle should not be changed by delayed focus' + ) + assert.end() + }) +}) + // @todo // this test case needs some changes to the codebase in order to work // these changes will help testablity in general diff --git a/src/router/router.js b/src/router/router.js index 1f349fb1..cf0697b9 100644 --- a/src/router/router.js +++ b/src/router/router.js @@ -406,33 +406,32 @@ const executeBeforeHook = async function ( try { result = await hooks[hookName].call(parent, route, previousRoute) if (isString(result)) { - currentRoute = previousRoute - to(result) + this.currentRoute = previousRoute + to(result, {}, {}, this.name) return false } } catch (error) { Log.error(`Error or Rejected Promise in "${hookName}" Hook`, error) + this.currentRoute = previousRoute if (this.history.length > 0) { preventHashChangeNavigation = true - currentRoute = previousRoute - /// hmmmm window.history.back() navigatingBack = false state.navigating = false - return false } + return false } // If the resolved result is an object, redirect if the path in the object was changed if (isObject(result) === true && result.path !== currentPath) { - currentRoute = previousRoute - to(result.path, result.data, result.options) + this.currentRoute = previousRoute + to(result.path, result.data, result.options, this.name) return false } // If the resolved result is false, cancel navigation if (result === false) { + this.currentRoute = previousRoute if (this.history.length > 0) { preventHashChangeNavigation = true - currentRoute = previousRoute window.history.back() navigatingBack = false state.navigating = false diff --git a/src/router/router.test.js b/src/router/router.test.js index 62338ada..f9906a7e 100644 --- a/src/router/router.test.js +++ b/src/router/router.test.js @@ -17,7 +17,7 @@ import test from 'tape' import { initLog } from '../lib/log.js' -import { to, navigate, back, state } from './router.js' +import { back, navigate, state, to } from './router.js' import { matchHash, getHash, setHash } from './utils.js' import { stage } from '../launch.js' import Component from '../component.js' @@ -863,6 +863,153 @@ test('BeforeEach hook route object redirect', async (assert) => { stage.element = originalElement }) +test('Cold redirect does not leave a phantom previous route', async (assert) => { + const originalElement = stage.element + stage.element = ({ parent }) => ({ + populate() {}, + set() {}, + destroy() {}, + parent, + }) + + const TestComponent = Component('ColdRedirectComponent', { + template: '', + code: { render: () => ({ elms: [], cleanup: () => {} }), effects: [] }, + }) + + const host = { + [symbols.parent]: { + [symbols.routes]: [ + { + path: '/cold-original', + component: TestComponent, + hooks: { + before() { + return '/cold-redirected' + }, + }, + }, + { path: '/cold-redirected', component: TestComponent, options: { passFocus: false } }, + ], + }, + [symbols.children]: [{}], + [symbols.props]: {}, + name: '', + } + + to('/cold-original') + await navigate.call(host) + assert.equal( + host.currentRoute, + undefined, + 'Redirect should restore the previous RouterView route' + ) + + await navigate.call(host) + assert.equal(host.currentRoute.path, '/cold-redirected', 'Redirected route should become current') + assert.equal(host[symbols.children].length, 2, 'Redirected view should remain attached') + assert.notEqual(host.activeView.eol, true, 'Redirected view should not be destroyed') + + location.hash = '#/' + stage.element = originalElement +}) + +test('Named RouterView redirect retains named hash context', async (assert) => { + const originalElement = stage.element + stage.element = ({ parent }) => ({ + populate() {}, + set() {}, + destroy() {}, + parent, + }) + + const TestComponent = Component('NamedRedirectComponent', { + template: '', + code: { render: () => ({ elms: [], cleanup: () => {} }), effects: [] }, + }) + + const host = { + [symbols.parent]: { + [symbols.routes]: [ + { + path: '/named-original', + component: TestComponent, + hooks: { + before() { + return { path: '/named-redirected' } + }, + }, + }, + { path: '/named-redirected', component: TestComponent, options: { passFocus: false } }, + ], + }, + [symbols.children]: [{}], + [symbols.props]: {}, + name: 'modal', + } + + location.hash = '#/' + to('/named-original', {}, {}, host.name) + await navigate.call(host) + + assert.equal( + getHash(location.hash, 'modal').path, + '/named-redirected', + 'Redirect should update the named RouterView hash segment' + ) + assert.equal(getHash(location.hash).path, '/', 'Redirect should not replace the main route') + + await navigate.call(host) + assert.equal(host.currentRoute.path, '/named-redirected', 'Named redirected route should render') + assert.equal(host[symbols.children].length, 2, 'Named redirected view should remain attached') + + location.hash = '#/' + stage.element = originalElement +}) + +test('Rejected hook with empty history cancels navigation', async (assert) => { + const originalElement = stage.element + stage.element = ({ parent }) => ({ + populate() {}, + set() {}, + destroy() {}, + parent, + }) + + const TestComponent = Component('RejectedHookComponent', { + template: '', + code: { render: () => ({ elms: [], cleanup: () => {} }), effects: [] }, + }) + + const host = { + [symbols.parent]: { + [symbols.routes]: [{ path: '/rejected-hook', component: TestComponent }], + [symbols.routerHooks]: { + beforeEach() { + throw new Error('Rejected hook') + }, + }, + }, + [symbols.children]: [{}], + [symbols.props]: {}, + name: '', + } + + to('/rejected-hook') + await navigate.call(host) + + assert.equal( + host.currentRoute, + undefined, + 'Rejected navigation should restore the previous route' + ) + assert.equal(host[symbols.children].length, 1, 'Rejected navigation should not render a view') + assert.equal(state.navigating, false, 'Rejected navigation should clear navigating state') + + location.hash = '#/' + stage.element = originalElement +}) + test('Route meta data is accessible in route object', (assert) => { const route = { path: '/test', meta: { auth: true, role: 'admin' } } assert.deepEqual( @@ -1564,6 +1711,11 @@ test('beforeEach returning false leaves state.path on previous route', async (as '/guard-each-1', 'Navigation should be cancelled when beforeEach returns false' ) + assert.equal( + host.currentRoute.path, + '/guard-each-1', + 'RouterView should restore its previous route' + ) stage.element = originalElement }) @@ -1647,6 +1799,11 @@ test('route before returning false leaves state.path on previous route', async ( '/guard-before-1', 'Navigation should be cancelled when route before returns false' ) + assert.equal( + host.currentRoute.path, + '/guard-before-1', + 'RouterView should restore its previous route' + ) stage.element = originalElement })