From: Eduardo San Martin Morote Date: Sat, 12 Oct 2019 14:50:50 +0000 (+0200) Subject: refactor: use new histories X-Git-Tag: v4.0.0-alpha.0~200 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e928ba1cc8303118deb27ca68ae59c09e3907d79;p=thirdparty%2Fvuejs%2Frouter.git refactor: use new histories --- diff --git a/__tests__/HistoryMock.ts b/__tests__/HistoryMock.ts deleted file mode 100644 index f70f433e..00000000 --- a/__tests__/HistoryMock.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { HistoryLocationNormalized, START } from '../src/history/base' -import { AbstractHistory } from '../src/history/abstract' - -export class HistoryMock extends AbstractHistory { - constructor(start: string | HistoryLocationNormalized = START) { - super() - const location = - typeof start === 'string' ? this.utils.normalizeLocation(start) : start - this.queue = [location] - } -} diff --git a/__tests__/history/abstract.spec.ts b/__tests__/history/abstract.spec.ts index e2fc6683..040f407d 100644 --- a/__tests__/history/abstract.spec.ts +++ b/__tests__/history/abstract.spec.ts @@ -1,6 +1,5 @@ - -import { AbstractHistory } from '../../src/history/abstract' -import { START } from '../../src/history/base' +import createMemoryHistory from '../../src/history/abstract.2' +import { START } from '../../src/history/common' /** @type {import('../../src/history/base').HistoryLocation} */ const loc = { @@ -24,21 +23,15 @@ const normaliezedLoc2 = { fullPath: '/bar', } +// TODO: figure out how to run these tests now describe('Abstract/in memory history', () => { - it('starts at /', () => { - const history = new AbstractHistory() + it('starts in nowhere', () => { + const history = createMemoryHistory() expect(history.location).toEqual(START) - expect(history.location).toEqual({ - fullPath: '/', - path: '/', - query: {}, - hash: '', - }) - expect(history.queue).toHaveLength(1) }) it('can push a location', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() // partial version history.push({ path: '/somewhere', hash: '#hey', query: { foo: 'foo' } }) expect(history.location).toEqual({ @@ -59,7 +52,7 @@ describe('Abstract/in memory history', () => { }) it('can replace a location', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() // partial version history.replace({ path: '/somewhere', hash: '#hey', query: { foo: 'foo' } }) expect(history.location).toEqual({ @@ -68,24 +61,10 @@ describe('Abstract/in memory history', () => { query: { foo: 'foo' }, hash: '#hey', }) - expect(history.position).toEqual(0) - expect(history.queue).toHaveLength(1) - history.push(loc) - - // partial version - history.replace({ path: '/path', hash: '#ho' }) - expect(history.location).toEqual({ - fullPath: '/path#ho', - path: '/path', - query: {}, - hash: '#ho', - }) - expect(history.position).toEqual(1) - expect(history.queue).toHaveLength(2) }) it('does not trigger listeners with push', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() const spy = jest.fn() history.listen(spy) history.push(loc) @@ -93,49 +72,37 @@ describe('Abstract/in memory history', () => { }) it('does not trigger listeners with replace', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() const spy = jest.fn() history.listen(spy) history.replace(loc) expect(spy).not.toHaveBeenCalled() }) - it('add entries to the queue', () => { - const history = new AbstractHistory() - history.push(loc) - expect(history.queue).toHaveLength(2) - expect(history.queue[1]).toEqual(normaliezedLoc) - history.push(loc2) - expect(history.queue).toHaveLength(3) - expect(history.queue[2]).toEqual(normaliezedLoc2) - }) - it('can go back', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() history.push(loc) history.push(loc2) history.back() - expect(history.queue).toHaveLength(3) expect(history.location).toEqual(normaliezedLoc) history.back() - expect(history.queue).toHaveLength(3) expect(history.location).toEqual(START) }) it('does nothing with back if queue contains only one element', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() history.back() expect(history.location).toEqual(START) }) it('does nothing with forward if at end of log', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() history.forward() expect(history.location).toEqual(START) }) it('can moves back and forth in history queue', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() history.push(loc) history.push(loc2) history.back() @@ -148,23 +115,21 @@ describe('Abstract/in memory history', () => { }) it('can push in the middle of the history', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() history.push(loc) history.push(loc2) history.back() history.back() expect(history.location).toEqual(START) history.push(loc2) - expect(history.queue).toHaveLength(2) expect(history.location).toEqual(normaliezedLoc2) // does nothing history.forward() - expect(history.queue).toHaveLength(2) expect(history.location).toEqual(normaliezedLoc2) }) it('can listen to navigations', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() const spy = jest.fn() history.listen(spy) history.push(loc) @@ -172,16 +137,22 @@ describe('Abstract/in memory history', () => { expect(spy).toHaveBeenCalledTimes(1) expect(spy).toHaveBeenCalledWith(START, normaliezedLoc, { direction: 'back', + distance: -1, + // TODO: should be something else + type: 'pop', }) history.forward() expect(spy).toHaveBeenCalledTimes(2) expect(spy).toHaveBeenLastCalledWith(normaliezedLoc, START, { direction: 'forward', + distance: 1, + // TODO: should be something else + type: 'pop', }) }) it('can stop listening to navigation', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() const spy = jest.fn() const spy2 = jest.fn() // remove right away @@ -198,7 +169,7 @@ describe('Abstract/in memory history', () => { }) it('removing the same listener is a noop', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() const spy = jest.fn() const spy2 = jest.fn() const rem = history.listen(spy) @@ -217,18 +188,17 @@ describe('Abstract/in memory history', () => { }) it('removes all listeners with destroy', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() + history.push('/other') const spy = jest.fn() history.listen(spy) - // @ts-ignore we need to check internals here - expect(history.listeners).toHaveLength(1) history.destroy() - // @ts-ignore we need to check internals here - expect(history.listeners).toHaveLength(0) + history.back() + expect(spy).not.toHaveBeenCalled() }) it('can avoid listeners with back and forward', () => { - const history = new AbstractHistory() + const history = createMemoryHistory() const spy = jest.fn() history.listen(spy) history.push(loc) diff --git a/__tests__/history/html5.spec.ts b/__tests__/history/html5.spec.ts index 5d291868..8cabc0b4 100644 --- a/__tests__/history/html5.spec.ts +++ b/__tests__/history/html5.spec.ts @@ -1,4 +1,4 @@ -import { HTML5History } from '../../src/history/html5' +import createHistory from '../../src/history/html5.2' import { createDom } from '../utils' // TODO: is it really worth testing this implementation on jest or is it @@ -9,7 +9,7 @@ describe.skip('History HTMl5', () => { }) it('can be instantiated', () => { - const history = new HTML5History() + const history = createHistory() expect(history.location).toEqual({ fullPath: '/', path: '/', diff --git a/__tests__/router-link.spec.ts b/__tests__/router-link.spec.ts index ee60737d..5bef966c 100644 --- a/__tests__/router-link.spec.ts +++ b/__tests__/router-link.spec.ts @@ -3,7 +3,6 @@ */ // NOTE: these tests only run when using jest `yarn jest --watch` import RouterLink from '../src/components/Link' -import { HistoryMock } from './utils' import { START_LOCATION_NORMALIZED, RouteQueryAndHash, @@ -11,6 +10,7 @@ import { RouteLocationNormalized, } from '../src/types' import { mount } from '@vue/test-utils' +import { createMemoryHistory } from '../src' const locations: Record< string, @@ -57,7 +57,7 @@ describe('RouterLink', () => { resolvedLocation: RouteLocationNormalized ) { const router = { - history: new HistoryMock(), + history: createMemoryHistory(), resolve: jest.fn(), push: jest.fn(), } diff --git a/__tests__/utils.ts b/__tests__/utils.ts index a3790994..f38bd314 100644 --- a/__tests__/utils.ts +++ b/__tests__/utils.ts @@ -1,8 +1,6 @@ import { JSDOM, ConstructorOptions } from 'jsdom' import { NavigationGuard, RouteRecord, MatchedRouteRecord } from '../src/types' -export { HistoryMock } from './HistoryMock' - export const tick = () => new Promise(resolve => process.nextTick(resolve)) export type NAVIGATION_METHOD = 'push' | 'replace' diff --git a/src/history/abstract.2.ts b/src/history/abstract.2.ts index e3603204..3aeb6f32 100644 --- a/src/history/abstract.2.ts +++ b/src/history/abstract.2.ts @@ -16,7 +16,7 @@ import { // const cs = console // const cs = consola.withTag('abstract') -export default function createAbstractHistory(): RouterHistory { +export default function createMemoryHistory(): RouterHistory { let listeners: NavigationCallback[] = [] // TODO: make sure this is right as the first location is nowhere so maybe this should be empty instead let queue: HistoryLocationNormalized[] = [START] diff --git a/src/history/abstract.ts b/src/history/abstract.ts deleted file mode 100644 index 9a50ba1a..00000000 --- a/src/history/abstract.ts +++ /dev/null @@ -1,97 +0,0 @@ -// import consola from 'consola' -import { - BaseHistory, - HistoryLocation, - HistoryLocationNormalized, - NavigationDirection, -} from './base' -import { NavigationCallback, HistoryState, START } from './base' - -// const cs = consola.withTag('abstract') - -export class AbstractHistory extends BaseHistory { - protected listeners: NavigationCallback[] = [] - public queue: HistoryLocationNormalized[] = [START] - public position: number = 0 - - constructor() { - super() - } - - // TODO: is this necessary - ensureLocation() {} - - replace(to: HistoryLocation) { - const toNormalized = this.utils.normalizeLocation(to) - // remove current entry and decrement position - this.queue.splice(this.position--, 1) - this.location = toNormalized - } - - push(to: HistoryLocation, data?: HistoryState) { - const toNormalized = this.utils.normalizeLocation(to) - this.location = toNormalized - } - - listen(callback: NavigationCallback) { - this.listeners.push(callback) - return () => { - const index = this.listeners.indexOf(callback) - if (index > -1) this.listeners.splice(index, 1) - } - } - - get location() { - return this.queue[this.position] - } - - set location(location: HistoryLocationNormalized) { - // super() call tries to push before the array is created - if (!this.queue) this.queue = [] - // move the queue cursor forward - this.position++ - if (this.position === this.queue.length) { - // we are at the end, we can simply append a new entry - this.queue.push(location) - } else { - // we are in the middle, we remove everything from here in the queue - this.queue.splice(this.position) - this.queue.push(location) - } - } - - back(triggerListeners: boolean = true) { - const from = this.location - if (this.position > 0) this.position-- - if (triggerListeners) { - this.triggerListeners(this.location, from, { - direction: NavigationDirection.back, - }) - } - } - - forward(triggerListeners: boolean = true) { - const from = this.location - if (this.position < this.queue.length - 1) this.position++ - if (triggerListeners) { - this.triggerListeners(this.location, from, { - direction: NavigationDirection.forward, - }) - } - } - - destroy() { - this.listeners = [] - } - - protected triggerListeners( - to: HistoryLocationNormalized, - from: HistoryLocationNormalized, - { direction }: { direction: NavigationDirection } - ): void { - const info = { direction } - for (let callback of this.listeners) { - callback(to, from, info) - } - } -} diff --git a/src/history/html5.ts b/src/history/html5.ts deleted file mode 100644 index b838d0ee..00000000 --- a/src/history/html5.ts +++ /dev/null @@ -1,233 +0,0 @@ -import consola from '../consola' -import { BaseHistory, HistoryLocationNormalized, HistoryLocation } from './base' -import { NavigationCallback, HistoryState, NavigationDirection } from './base' -import { computeScrollPosition, ScrollToPosition } from '../utils/scroll' - -const cs = consola.withTag('html5') - -// TODO: implement the mock instead -/* istanbul ignore next */ -// @ts-ignore otherwise fails after rollup replacement plugin -if (process.env.NODE_ENV === 'test') cs.mockTypes(() => jest.fn()) - -type PopStateListener = (this: Window, ev: PopStateEvent) => any - -interface StateEntry { - back: HistoryLocationNormalized | null - current: HistoryLocationNormalized - forward: HistoryLocationNormalized | null - replaced: boolean - scroll: ScrollToPosition | null -} - -// TODO: pretty useless right now except for typing -function buildState( - back: HistoryLocationNormalized | null, - current: HistoryLocationNormalized, - forward: HistoryLocationNormalized | null, - replaced: boolean = false, - computeScroll: boolean = false -): StateEntry { - return { - back, - current, - forward, - replaced, - scroll: computeScroll ? computeScrollPosition() : null, - } -} - -interface PauseState { - currentLocation: HistoryLocationNormalized - // location we are going to after pausing - to: HistoryLocationNormalized -} - -export class HTML5History extends BaseHistory { - private history = window.history - private _popStateHandler: PopStateListener - private _listeners: NavigationCallback[] = [] - private _teardowns: Array<() => void> = [] - - // TODO: should it be a stack? a Dict. Check if the popstate listener - // can trigger twice - private pauseState: PauseState | null = null - - constructor() { - super() - const to = this.createCurrentLocation() - // cs.log('created', to) - this.history.replaceState(buildState(null, to, null), '', to.fullPath) - this.location = to - this._popStateHandler = this.setupPopStateListener() - } - - // TODO: is this necessary - ensureLocation() {} - - private changeLocation( - state: StateEntry, - title: string, - url: string, - replace: boolean - ): void { - try { - // BROWSER QUIRK - // NOTE: Safari throws a SecurityError when calling this function 100 times in 30 seconds - this.history[replace ? 'replaceState' : 'pushState'](state, title, url) - } catch (err) { - console.log('Error with push/replace State', err) - // Force the navigation, this also resets the call count - location[replace ? 'replace' : 'assign'](url) - } - } - - replace(to: HistoryLocation) { - const normalized = this.utils.normalizeLocation(to) - if (normalized.fullPath === this.location.fullPath) return - cs.info('replace', this.location, normalized) - this.changeLocation( - // TODO: this should be user's responsibility - // _replacedState: this.history.state || null, - buildState(this.history.state.back, normalized, null, true), - '', - normalized.fullPath, - true - ) - this.location = normalized - } - - push(to: HistoryLocation, data?: HistoryState) { - // replace current entry state to add the forward value - // TODO: should be removed and let the user normalize the location? - // or make it fast so normalization on a normalized object is fast - const normalized = this.utils.normalizeLocation(to) - this.changeLocation( - buildState( - this.history.state.back, - this.history.state.current, - normalized, - this.history.state.replaced, - // TODO: this is just not enough to only save the scroll position when not pushing or replacing - true - ), - '', - this.location.fullPath, - true - ) - // TODO: compare current location to prevent navigation - // NEW NOTE: I think it shouldn't be history responsibility to check that - // if (to === this.location) return - const state = { - ...buildState(this.location, normalized, null), - ...data, - } - cs.info('push', this.location, '->', normalized, 'with state', state) - this.changeLocation(state, '', normalized.fullPath, false) - this.location = normalized - } - - back(triggerListeners: boolean = true) { - // TODO: check if we can go back - const previvousLocation = this.history.state - .back as HistoryLocationNormalized - if (!triggerListeners) this.pauseListeners(previvousLocation) - this.history.back() - } - - forward(triggerListeners: boolean = true) { - // TODO: check if we can go forward - const previvousLocation = this.history.state - .forward as HistoryLocationNormalized - if (!previvousLocation) throw new Error('Cannot go forward') - if (!triggerListeners) this.pauseListeners(previvousLocation) - this.history.forward() - } - - listen(callback: NavigationCallback) { - // settup the listener and prepare teardown callbacks - this._listeners.push(callback) - - const teardown = () => { - this._listeners.splice(this._listeners.indexOf(callback), 1) - } - - this._teardowns.push(teardown) - return teardown - } - - /** - * Remove all listeners attached to the history and cleanups the history - * instance - */ - destroy() { - for (const teardown of this._teardowns) teardown() - this._teardowns = [] - if (this._popStateHandler) - window.removeEventListener('popstate', this._popStateHandler) - } - - /** - * Setups the popstate event listener. It's important to setup only - * one to ensure the same parameters are passed to every listener - */ - private setupPopStateListener() { - const handler: PopStateListener = ({ state }: { state: StateEntry }) => { - cs.info('popstate fired', { - state, - location: this.location, - }) - - // TODO: handle go(-2) and go(2) (skipping entries) - - const from = this.location - // we have the state from the old entry, not the current one being removed - // TODO: correctly parse pathname - const to = state ? state.current : this.createCurrentLocation() - this.location = to - - if ( - this.pauseState && - this.pauseState.to && - this.pauseState.to.fullPath === to.fullPath - ) { - cs.info('Ignored beacuse paused') - // reset pauseState - this.pauseState = null - return - } - - // call all listeners - const navigationInfo = { - direction: - state.forward && from.fullPath === state.forward.fullPath - ? NavigationDirection.back - : NavigationDirection.forward, - } - this._listeners.forEach(listener => - listener(this.location, from, navigationInfo) - ) - } - - // settup the listener and prepare teardown callbacks - window.addEventListener('popstate', handler) - return handler - } - - private pauseListeners(to: HistoryLocationNormalized) { - this.pauseState = { - currentLocation: this.location, - to, - } - } - - createCurrentLocation(): HistoryLocationNormalized { - const { location } = window - return { - fullPath: location.pathname + location.search + location.hash, - path: location.pathname, - query: this.utils.parseQuery(location.search), - hash: location.hash, - } - } -} diff --git a/src/index.ts b/src/index.ts index 714b5d99..ed0526ab 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,8 +1,4 @@ import { Router, RouterOptions } from './router' -import { HTML5History } from './history/html5' -import { HashHistory } from './history/hash' -import { AbstractHistory } from './history/abstract' -import { BaseHistory } from './history/base' import { PluginFunction, VueConstructor } from 'vue' import createHistory from './history/html5.2' import createMemoryHistory from './history/abstract.2' @@ -67,14 +63,7 @@ const plugin: PluginFunction = Vue => { strats.created } -export { - Router, - HTML5History, - HashHistory, - AbstractHistory, - BaseHistory, - plugin, -} +export { Router, createHistory, createMemoryHistory, plugin } // TODO: refactor somewhere else // const inBrowser = typeof window !== 'undefined' @@ -85,8 +74,6 @@ export { // abstract: AbstractHistory // } -export { createHistory, createMemoryHistory } - export default class VueRouter extends Router { static install = plugin static version = '__VERSION__'