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.
vuejs · Jul 3, 2020 · 10a071c · 10a071c
1 parent c6274ae
commit 10a071c
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 181 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,47 +202,23 @@ 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 () => {
     const { router, history } = await newRouter()
     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 () => {
     const { router, history } = await newRouter()
     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
@@ -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,19 +38,16 @@ export interface NavigationInformation {
 
 export interface NavigationCallback {
   (
-    to: HistoryLocationNormalized,
-    from: HistoryLocationNormalized,
+    to: HistoryLocation,
+    from: HistoryLocation,
     information: NavigationInformation
   ): void
 }
 
 /**
  * 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
 }