From: Eduardo San Martin Morote Date: Fri, 3 Jul 2020 09:31:58 +0000 (+0200) Subject: refactor(history): simplify location as a string X-Git-Tag: v4.0.0-beta.1~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=10a071c85c62b6674929162aa36220bd8c167f27;p=thirdparty%2Fvuejs%2Frouter.git refactor(history): simplify location as a string BREAKING CHANGE: HistoryLocation is just a string now. It was pretty much an internal property but it could be used inside `history.state`. It used to be an object `{ fullPath: '/the-url' }`. And it's now just the `fullPath` property. --- diff --git a/__tests__/history/memory.spec.ts b/__tests__/history/memory.spec.ts index 63cad39a..4afa1454 100644 --- a/__tests__/history/memory.spec.ts +++ b/__tests__/history/memory.spec.ts @@ -1,21 +1,9 @@ import { createMemoryHistory } from '../../src/history/memory' -import { - START, - HistoryLocationNormalized, - RawHistoryLocation, -} from '../../src/history/common' +import { START, HistoryLocation } from '../../src/history/common' -const loc: RawHistoryLocation = '/foo' +const loc: HistoryLocation = '/foo' -const loc2: RawHistoryLocation = '/bar' - -const normalizedLoc: HistoryLocationNormalized = { - fullPath: '/foo', -} - -const normalizedLoc2: HistoryLocationNormalized = { - fullPath: '/bar', -} +const loc2: HistoryLocation = '/bar' describe('Memory history', () => { it('starts in nowhere', () => { @@ -26,18 +14,14 @@ describe('Memory history', () => { it('can push a location', () => { const history = createMemoryHistory() history.push('/somewhere?foo=foo#hey') - expect(history.location).toEqual({ - fullPath: '/somewhere?foo=foo#hey', - }) + expect(history.location).toEqual('/somewhere?foo=foo#hey') }) it('can replace a location', () => { const history = createMemoryHistory() // partial version history.replace('/somewhere?foo=foo#hey') - expect(history.location).toEqual({ - fullPath: '/somewhere?foo=foo#hey', - }) + expect(history.location).toEqual('/somewhere?foo=foo#hey') }) it('does not trigger listeners with push', () => { @@ -61,7 +45,7 @@ describe('Memory history', () => { history.push(loc) history.push(loc2) history.go(-1) - expect(history.location).toEqual(normalizedLoc) + expect(history.location).toEqual(loc) history.go(-1) expect(history.location).toEqual(START) }) @@ -86,9 +70,9 @@ describe('Memory history', () => { history.go(-1) expect(history.location).toEqual(START) history.go(1) - expect(history.location).toEqual(normalizedLoc) + expect(history.location).toEqual(loc) history.go(1) - expect(history.location).toEqual(normalizedLoc2) + expect(history.location).toEqual(loc2) }) it('can push in the middle of the history', () => { @@ -99,10 +83,10 @@ describe('Memory history', () => { history.go(-1) expect(history.location).toEqual(START) history.push(loc2) - expect(history.location).toEqual(normalizedLoc2) + expect(history.location).toEqual(loc2) // does nothing history.go(1) - expect(history.location).toEqual(normalizedLoc2) + expect(history.location).toEqual(loc2) }) it('can listen to navigations', () => { @@ -112,14 +96,14 @@ describe('Memory history', () => { history.push(loc) history.go(-1) expect(spy).toHaveBeenCalledTimes(1) - expect(spy).toHaveBeenCalledWith(START, normalizedLoc, { + expect(spy).toHaveBeenCalledWith(START, loc, { direction: 'back', delta: -1, type: 'pop', }) history.go(1) expect(spy).toHaveBeenCalledTimes(2) - expect(spy).toHaveBeenLastCalledWith(normalizedLoc, START, { + expect(spy).toHaveBeenLastCalledWith(loc, START, { direction: 'forward', delta: 1, type: 'pop', diff --git a/__tests__/initialNavigation.spec.ts b/__tests__/initialNavigation.spec.ts index cade0b14..9625e365 100644 --- a/__tests__/initialNavigation.spec.ts +++ b/__tests__/initialNavigation.spec.ts @@ -50,9 +50,9 @@ describe('Initial Navigation', () => { it('handles initial navigation with redirect', async () => { const { history, router } = newRouter('/home') - expect(history.location.fullPath).toBe('/home') + expect(history.location).toBe('/home') // this is done automatically on install but there is none here - await router.push(history.location.fullPath) + await router.push(history.location) expect(router.currentRoute.value).toMatchObject({ path: '/' }) await router.push('/foo') expect(router.currentRoute.value).toMatchObject({ path: '/foo' }) @@ -63,9 +63,9 @@ describe('Initial Navigation', () => { it('handles initial navigation with beforEnter', async () => { const { history, router } = newRouter('/home-before') - expect(history.location.fullPath).toBe('/home-before') + expect(history.location).toBe('/home-before') // this is done automatically on mount but there is no mount here - await router.push(history.location.fullPath) + await router.push(history.location) expect(router.currentRoute.value).toMatchObject({ path: '/' }) await router.push('/foo') expect(router.currentRoute.value).toMatchObject({ path: '/foo' }) diff --git a/__tests__/location.spec.ts b/__tests__/location.spec.ts index 5399d549..7e798d13 100644 --- a/__tests__/location.spec.ts +++ b/__tests__/location.spec.ts @@ -1,4 +1,3 @@ -import { normalizeHistoryLocation as normalizeLocation } from '../src/history/common' import { parseQuery, stringifyQuery } from '../src/query' import { parseURL as originalParseURL, @@ -208,20 +207,6 @@ describe('stringifyURL', () => { }) }) -describe('normalizeLocation', () => { - it('works with string', () => { - expect(normalizeLocation('/foo')).toEqual({ fullPath: '/foo' }) - }) - - it('works with objects', () => { - expect( - normalizeLocation({ - fullPath: '/foo', - }) - ).toEqual({ fullPath: '/foo' }) - }) -}) - describe('stripBase', () => { it('returns the pathname if no base', () => { expect(stripBase('', '')).toBe('') diff --git a/__tests__/router.spec.ts b/__tests__/router.spec.ts index 8fb84dc0..e6b8be6d 100644 --- a/__tests__/router.spec.ts +++ b/__tests__/router.spec.ts @@ -102,15 +102,7 @@ describe('Router', () => { jest.spyOn(history, 'push') await router.push('/foo') expect(history.push).toHaveBeenCalledTimes(1) - expect(history.push).toHaveBeenCalledWith( - expect.objectContaining({ - fullPath: '/foo', - path: '/foo', - query: {}, - hash: '', - }), - undefined - ) + expect(history.push).toHaveBeenCalledWith('/foo', undefined) }) it('calls history.replace with router.replace', async () => { @@ -119,15 +111,7 @@ describe('Router', () => { jest.spyOn(history, 'replace') await router.replace('/foo') expect(history.replace).toHaveBeenCalledTimes(1) - expect(history.replace).toHaveBeenCalledWith( - expect.objectContaining({ - fullPath: '/foo', - path: '/foo', - query: {}, - hash: '', - }), - expect.anything() - ) + expect(history.replace).toHaveBeenCalledWith('/foo', expect.anything()) }) it('replaces if a guard redirects', async () => { @@ -138,15 +122,7 @@ describe('Router', () => { jest.spyOn(history, 'replace') await router.replace('/home-before') expect(history.replace).toHaveBeenCalledTimes(1) - expect(history.replace).toHaveBeenCalledWith( - expect.objectContaining({ - fullPath: '/', - path: '/', - query: {}, - hash: '', - }), - expect.anything() - ) + expect(history.replace).toHaveBeenCalledWith('/', expect.anything()) }) it('allows to customize parseQuery', async () => { @@ -226,15 +202,7 @@ describe('Router', () => { jest.spyOn(history, 'replace') await router.push({ path: '/foo', replace: true }) expect(history.replace).toHaveBeenCalledTimes(1) - expect(history.replace).toHaveBeenCalledWith( - expect.objectContaining({ - fullPath: '/foo', - path: '/foo', - query: {}, - hash: '', - }), - expect.anything() - ) + expect(history.replace).toHaveBeenCalledWith('/foo', expect.anything()) }) it('can replaces current location with a string location', async () => { @@ -242,15 +210,7 @@ describe('Router', () => { jest.spyOn(history, 'replace') await router.replace('/foo') expect(history.replace).toHaveBeenCalledTimes(1) - expect(history.replace).toHaveBeenCalledWith( - expect.objectContaining({ - fullPath: '/foo', - path: '/foo', - query: {}, - hash: '', - }), - expect.anything() - ) + expect(history.replace).toHaveBeenCalledWith('/foo', expect.anything()) }) it('can replaces current location with an object location', async () => { @@ -258,15 +218,7 @@ describe('Router', () => { jest.spyOn(history, 'replace') await router.replace({ path: '/foo' }) expect(history.replace).toHaveBeenCalledTimes(1) - expect(history.replace).toHaveBeenCalledWith( - expect.objectContaining({ - fullPath: '/foo', - path: '/foo', - query: {}, - hash: '', - }), - expect.anything() - ) + expect(history.replace).toHaveBeenCalledWith('/foo', expect.anything()) }) it('navigates if the location does not exist', async () => { diff --git a/src/history/common.ts b/src/history/common.ts index 2c7c8bc2..3dc320fb 100644 --- a/src/history/common.ts +++ b/src/history/common.ts @@ -1,13 +1,7 @@ import { isBrowser } from '../utils' import { removeTrailingSlash } from '../location' -interface HistoryLocation { - fullPath: string - state?: HistoryState -} - -export type RawHistoryLocation = HistoryLocation | string -export type HistoryLocationNormalized = Pick +export type HistoryLocation = string // pushState clones the state passed and do not accept everything // it doesn't accept symbols, nor functions as values. It also ignores Symbols as keys type HistoryStateValue = @@ -44,8 +38,8 @@ export interface NavigationInformation { export interface NavigationCallback { ( - to: HistoryLocationNormalized, - from: HistoryLocationNormalized, + to: HistoryLocation, + from: HistoryLocation, information: NavigationInformation ): void } @@ -53,10 +47,7 @@ export interface NavigationCallback { /** * Starting location for Histories */ -const START_PATH = '' -export const START: HistoryLocationNormalized = { - fullPath: START_PATH, -} +export const START: HistoryLocation = '' export type ValueContainer = { value: T } @@ -76,7 +67,7 @@ export interface RouterHistory { /** * Current History location */ - readonly location: HistoryLocationNormalized + readonly location: HistoryLocation /** * Current History state */ @@ -91,7 +82,7 @@ export interface RouterHistory { * @param data - optional {@link HistoryState} to be associated with the * navigation entry */ - push(to: RawHistoryLocation, data?: HistoryState): void + push(to: HistoryLocation, data?: HistoryState): void /** * Same as {@link RouterHistory.push} but performs a `history.replaceState` * instead of `history.pushState` @@ -100,7 +91,7 @@ export interface RouterHistory { * @param data - optional {@link HistoryState} to be associated with the * navigation entry */ - replace(to: RawHistoryLocation, data?: HistoryState): void + replace(to: HistoryLocation, data?: HistoryState): void /** * Traverses history in a given direction. @@ -134,7 +125,7 @@ export interface RouterHistory { * * @param location - history location that should create an href */ - createHref(location: HistoryLocationNormalized): string + createHref(location: HistoryLocation): string /** * Clears any event listener attached by the history implementation. @@ -144,15 +135,6 @@ export interface RouterHistory { // Generic utils -export function normalizeHistoryLocation( - location: RawHistoryLocation -): HistoryLocationNormalized { - return { - // to avoid doing a typeof or in that is quite long - fullPath: (location as HistoryLocation).fullPath || (location as string), - } -} - /** * Normalizes a base by removing any trailing slash and reading the base tag if * present. @@ -184,9 +166,6 @@ export function normalizeBase(base?: string): string { // remove any character before the hash const BEFORE_HASH_RE = /^[^#]+#/ -export function createHref( - base: string, - location: HistoryLocationNormalized -): string { - return base.replace(BEFORE_HASH_RE, '#') + location.fullPath +export function createHref(base: string, location: HistoryLocation): string { + return base.replace(BEFORE_HASH_RE, '#') + location } diff --git a/src/history/html5.ts b/src/history/html5.ts index 28187561..4a3f169e 100644 --- a/src/history/html5.ts +++ b/src/history/html5.ts @@ -3,13 +3,11 @@ import { NavigationCallback, NavigationType, NavigationDirection, - HistoryLocationNormalized, - normalizeHistoryLocation, HistoryState, - RawHistoryLocation, ValueContainer, normalizeBase, createHref, + HistoryLocation, } from './common' import { computeScrollPosition, @@ -24,9 +22,9 @@ type PopStateListener = (this: Window, ev: PopStateEvent) => any let createBaseLocation = () => location.protocol + '//' + location.host interface StateEntry extends HistoryState { - back: HistoryLocationNormalized | null - current: HistoryLocationNormalized - forward: HistoryLocationNormalized | null + back: HistoryLocation | null + current: HistoryLocation + forward: HistoryLocation | null position: number replaced: boolean scroll: _ScrollPositionNormalized | null | false @@ -39,7 +37,7 @@ interface StateEntry extends HistoryState { function createCurrentLocation( base: string, location: Location -): HistoryLocationNormalized { +): HistoryLocation { const { pathname, search, hash } = location // allows hash based url const hashPos = base.indexOf('#') @@ -47,23 +45,23 @@ function createCurrentLocation( // prepend the starting slash to hash so the url starts with /# let pathFromHash = hash.slice(1) if (pathFromHash[0] !== '/') pathFromHash = '/' + pathFromHash - return normalizeHistoryLocation(stripBase(pathFromHash, '')) + return stripBase(pathFromHash, '') } const path = stripBase(pathname, base) - return normalizeHistoryLocation(path + search + hash) + return path + search + hash } function useHistoryListeners( base: string, historyState: ValueContainer, - location: ValueContainer, + location: ValueContainer, replace: RouterHistory['replace'] ) { let listeners: NavigationCallback[] = [] let teardowns: Array<() => void> = [] // TODO: should it be a stack? a Dict. Check if the popstate listener // can trigger twice - let pauseState: HistoryLocationNormalized | null = null + let pauseState: HistoryLocation | null = null const popStateHandler: PopStateListener = ({ state, @@ -71,7 +69,7 @@ function useHistoryListeners( state: StateEntry | null }) => { const to = createCurrentLocation(base, window.location) - const from: HistoryLocationNormalized = location.value + const from: HistoryLocation = location.value const fromState: StateEntry = historyState.value let delta = 0 @@ -80,13 +78,13 @@ function useHistoryListeners( historyState.value = state // ignore the popstate and reset the pauseState - if (pauseState && pauseState.fullPath === from.fullPath) { + if (pauseState && pauseState === from) { pauseState = null return } delta = fromState ? state.position - fromState.position : 0 } else { - replace(to.fullPath) + replace(to) } // console.log({ deltaFromCurrent }) @@ -156,9 +154,9 @@ function useHistoryListeners( * Creates a state object */ function buildState( - back: HistoryLocationNormalized | null, - current: HistoryLocationNormalized, - forward: HistoryLocationNormalized | null, + back: HistoryLocation | null, + current: HistoryLocation, + forward: HistoryLocation | null, replaced: boolean = false, computeScroll: boolean = false ): StateEntry { @@ -176,7 +174,7 @@ function useHistoryStateNavigation(base: string) { const { history } = window // private variables - let location: ValueContainer = { + let location: ValueContainer = { value: createCurrentLocation(base, window.location), } let historyState: ValueContainer = { value: history.state } @@ -200,11 +198,11 @@ function useHistoryStateNavigation(base: string) { } function changeLocation( - to: HistoryLocationNormalized, + to: HistoryLocation, state: StateEntry, replace: boolean ): void { - const url = createBaseLocation() + base + to.fullPath + const url = createBaseLocation() + base + to try { // BROWSER QUIRK // NOTE: Safari throws a SecurityError when calling this function 100 times in 30 seconds @@ -217,16 +215,14 @@ function useHistoryStateNavigation(base: string) { } } - function replace(to: RawHistoryLocation, data?: HistoryState) { - const normalized = normalizeHistoryLocation(to) - + function replace(to: HistoryLocation, data?: HistoryState) { const state: StateEntry = assign( {}, history.state, buildState( historyState.value.back, // keep back and forward entries but override current position - normalized, + to, historyState.value.forward, true ), @@ -234,32 +230,30 @@ function useHistoryStateNavigation(base: string) { { position: historyState.value.position } ) - changeLocation(normalized, state, true) - location.value = normalized + changeLocation(to, state, true) + location.value = to } - function push(to: RawHistoryLocation, data?: HistoryState) { - const normalized = normalizeHistoryLocation(to) - + function push(to: HistoryLocation, data?: HistoryState) { // Add to current entry the information of where we are going // as well as saving the current position const currentState: StateEntry = assign({}, history.state, { - forward: normalized, + forward: to, scroll: computeScrollPosition(), }) changeLocation(currentState.current, currentState, true) const state: StateEntry = assign( {}, - buildState(location.value, normalized, null), + buildState(location.value, to, null), { position: currentState.position + 1, }, data ) - changeLocation(normalized, state, false) - location.value = normalized + changeLocation(to, state, false) + location.value = to } return { @@ -289,7 +283,7 @@ export function createWebHistory(base?: string): RouterHistory { const routerHistory: RouterHistory = assign( { // it's overridden right after - location: ('' as unknown) as HistoryLocationNormalized, + location: '', base, go, createHref: createHref.bind(null, base), diff --git a/src/history/memory.ts b/src/history/memory.ts index 86a1a3f5..93f4b80a 100644 --- a/src/history/memory.ts +++ b/src/history/memory.ts @@ -2,13 +2,12 @@ import { RouterHistory, NavigationCallback, START, - normalizeHistoryLocation, - HistoryLocationNormalized, HistoryState, NavigationType, NavigationDirection, NavigationInformation, createHref, + HistoryLocation, } from './common' // TODO: verify base is working for SSR @@ -21,10 +20,10 @@ import { */ export function createMemoryHistory(base: string = ''): RouterHistory { let listeners: NavigationCallback[] = [] - let queue: HistoryLocationNormalized[] = [START] + let queue: HistoryLocation[] = [START] let position: number = 0 - function setLocation(location: HistoryLocationNormalized) { + function setLocation(location: HistoryLocation) { position++ if (position === queue.length) { // we are at the end, we can simply append a new entry @@ -37,8 +36,8 @@ export function createMemoryHistory(base: string = ''): RouterHistory { } function triggerListeners( - to: HistoryLocationNormalized, - from: HistoryLocationNormalized, + to: HistoryLocation, + from: HistoryLocation, { direction, delta }: Pick ): void { const info: NavigationInformation = { @@ -59,14 +58,13 @@ export function createMemoryHistory(base: string = ''): RouterHistory { createHref: createHref.bind(null, base), replace(to) { - const toNormalized = normalizeHistoryLocation(to) // remove current entry and decrement position queue.splice(position--, 1) - setLocation(toNormalized) + setLocation(to) }, push(to, data?: HistoryState) { - setLocation(normalizeHistoryLocation(to)) + setLocation(to) }, listen(callback) { diff --git a/src/router.ts b/src/router.ts index 2ef0b8bc..3de6d8d7 100644 --- a/src/router.ts +++ b/src/router.ts @@ -254,7 +254,7 @@ export function createRouter(options: RouterOptions): Router { currentLocation ) - let href = routerHistory.createHref(locationNormalized) + let href = routerHistory.createHref(locationNormalized.fullPath) if (__DEV__) { if (href.startsWith('//')) warn( @@ -321,7 +321,7 @@ export function createRouter(options: RouterOptions): Router { }) ) - let href = routerHistory.createHref({ fullPath }) + let href = routerHistory.createHref(fullPath) if (__DEV__) { if (href.startsWith('//')) warn( @@ -706,7 +706,7 @@ export function createRouter(options: RouterOptions): Router { // history state if it exists if (replace || isFirstNavigation) routerHistory.replace( - toLocation, + toLocation.fullPath, assign( { scroll: isFirstNavigation && state && state.scroll, @@ -714,7 +714,7 @@ export function createRouter(options: RouterOptions): Router { data ) ) - else routerHistory.push(toLocation, data) + else routerHistory.push(toLocation.fullPath, data) } // accept current navigation @@ -729,7 +729,7 @@ export function createRouter(options: RouterOptions): Router { function setupListeners() { removeHistoryListener = routerHistory.listen((to, _from, info) => { // cannot be a redirect route because it was in history - const toLocation = resolve(to.fullPath) as RouteLocationNormalized + const toLocation = resolve(to) as RouteLocationNormalized pendingLocation = toLocation const from = currentRoute.value @@ -933,7 +933,7 @@ export function createRouter(options: RouterOptions): Router { ) { // see above started = true - push(routerHistory.location.fullPath).catch(err => { + push(routerHistory.location).catch(err => { if (__DEV__) warn('Unexpected error when starting the router:', err) }) }