]> git.ipfire.org Git - thirdparty/vuejs/router.git/commitdiff
refactor(history): simplify location as a string
authorEduardo San Martin Morote <posva13@gmail.com>
Fri, 3 Jul 2020 09:31:58 +0000 (11:31 +0200)
committerEduardo San Martin Morote <posva13@gmail.com>
Fri, 3 Jul 2020 09:31:58 +0000 (11:31 +0200)
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.

__tests__/history/memory.spec.ts
__tests__/initialNavigation.spec.ts
__tests__/location.spec.ts
__tests__/router.spec.ts
src/history/common.ts
src/history/html5.ts
src/history/memory.ts
src/router.ts

index 63cad39a3af05dc76e58b76013b239843ce04d1f..4afa14544e938a4e6f5cd71cb0ee0d34c18e57f9 100644 (file)
@@ -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',
index cade0b143b7137219913eec590dc341b4ee25ce2..9625e365b3f7c58fd75d391b70a25bbe25ea5258 100644 (file)
@@ -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' })
index 5399d549b903a58c34278ab1ebfe8d57f33beccf..7e798d1361551da3a8e8f589bbf8d70bb04f2122 100644 (file)
@@ -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('')
index 8fb84dc0ebff1ee6bf51a6a345701d7b54059d33..e6b8be6d7b535679e1820960a8f4c1d927254121 100644 (file)
@@ -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 () => {
index 2c7c8bc22179b3bae8e712a347b75bb199acdecf..3dc320fb9514ac9e44c766c77d011872dad24c9f 100644 (file)
@@ -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<HistoryLocation, 'fullPath'>
+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<T> = { 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
 }
index 281875619e60c55fc16a8aaf26b257f453eb4ded..4a3f169e1f857344b7b8d3d23651db5c910e620f 100644 (file)
@@ -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<StateEntry>,
-  location: ValueContainer<HistoryLocationNormalized>,
+  location: ValueContainer<HistoryLocation>,
   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<HistoryLocationNormalized> = {
+  let location: ValueContainer<HistoryLocation> = {
     value: createCurrentLocation(base, window.location),
   }
   let historyState: ValueContainer<StateEntry> = { 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),
index 86a1a3f550d23093fe04ab316c11b9a1194ec2ef..93f4b80a8a42697a0fc71042e241130bd0a2fbb0 100644 (file)
@@ -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<NavigationInformation, 'direction' | 'delta'>
   ): 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) {
index 2ef0b8bca2e843b0de91c795b997fea562f6370a..3de6d8d722ca45834123ae8f4328a7b654a584df 100644 (file)
@@ -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)
         })
       }