fix(hash): allow url to contain search params before hash

Fix #378

diff --git a/__tests__/history/hash.spec.ts b/__tests__/history/hash.spec.ts
index 0c08a2b5f2060b4727f9f7b6330973f02132bd5a..ccddf04d3c58f9bd93527ee4775ef2f24c90ddea 100644 (file)
--- a/__tests__/history/hash.spec.ts
+++ b/__tests__/history/hash.spec.ts
@@ -2,6 +2,7 @@ import { JSDOM } from 'jsdom'
 import { createWebHashHistory } from '../../src/history/hash'
 import { createWebHistory } from '../../src/history/html5'
 import { createDom } from '../utils'
+import { mockWarn } from 'jest-mock-warn'
 
 jest.mock('../../src/history/html5')
 // override the value of isBrowser because the variable is created before JSDOM
@@ -15,6 +16,7 @@ describe('History Hash', () => {
   beforeAll(() => {
     dom = createDom()
   })
+  mockWarn()
 
   beforeEach(() => {
     ;(createWebHistory as jest.Mock).mockClear()
@@ -43,9 +45,13 @@ describe('History Hash', () => {
       expect(createWebHistory).toHaveBeenCalledWith('/#')
     })
 
-    it('does not append a # if the user provides one', () => {
+    it('warns if there is anything but a slash after the # in a provided base', () => {
+      createWebHashHistory('/#/')
+      createWebHashHistory('/#')
+      createWebHashHistory('/base/#')
+      expect('').not.toHaveBeenWarned()
       createWebHashHistory('/#/app')
-      expect(createWebHistory).toHaveBeenCalledWith('/#/app')
+      expect('should be "/#"').toHaveBeenWarned()
     })
 
     it('should be able to provide a base', () => {

diff --git a/__tests__/router.spec.ts b/__tests__/router.spec.ts
index 62c6626d7238cdccef7a9f57f54ef42f305e7da8..6cb73b0d830b98eb48873b9b867ae976dc9dc47f 100644 (file)
--- a/__tests__/router.spec.ts
+++ b/__tests__/router.spec.ts
@@ -172,12 +172,6 @@ describe('Router', () => {
       fullPath: '/foo?bar=baz#hey',
       href: '#/foo?bar=baz#hey',
     })
-    history = createWebHashHistory('/with/#/base/')
-    ;({ router } = await newRouter({ history }))
-    expect(router.resolve('/foo?bar=baz#hey')).toMatchObject({
-      fullPath: '/foo?bar=baz#hey',
-      href: '#/base/foo?bar=baz#hey',
-    })
   })
 
   it('can await router.go', async () => {

diff --git a/e2e/hash/index.ts b/e2e/hash/index.ts
index 68e61657cdfe0679c5c5e049117c09a4d0e977c5..190efe388fc148f66cc031c20f30dd2622312da0 100644 (file)
--- a/e2e/hash/index.ts
+++ b/e2e/hash/index.ts
@@ -20,7 +20,7 @@ const Unicode: RouteComponent = {
 const router = createRouter({
   // keep a trailing slash in this specific case because we are using a hash
   // history
-  history: createWebHashHistory('/' + __dirname + '/'),
+  history: createWebHashHistory('/' + __dirname + '/#/'),
   routes: [
     { path: '/', component: Home },
     { path: '/foo', component: Foo },

diff --git a/e2e/specs/hash.js b/e2e/specs/hash.js
index 5d381988da6b8cca71ff0940551734cec308fdcf..7d0beea547defddb27480bbae05feb8b26017f8e 100644 (file)
--- a/e2e/specs/hash.js
+++ b/e2e/specs/hash.js
@@ -48,6 +48,33 @@ module.exports = {
       .end()
   },
 
+  /** @type {import('nightwatch').NightwatchTest} */
+  'initial navigation with search': function (browser) {
+    browser
+      .url('http://localhost:8080/hash/?code=auth#')
+      .waitForElementPresent('#app > *', 1000)
+      .assert.urlEquals('http://localhost:8080/hash/?code=auth#/')
+
+      .url('http://localhost:8080/hash/?code=auth#/foo')
+      .assert.urlEquals('http://localhost:8080/hash/?code=auth#/foo')
+      // manually remove the search from the URL
+      .waitForElementPresent('#app > *', 1000)
+      .execute(function () {
+        window.history.replaceState(history.state, '', '/hash/#/foo')
+      })
+      .click('li:nth-child(3) a')
+      .assert.urlEquals(baseURL + '/bar')
+      .back()
+      .assert.urlEquals(baseURL + '/foo')
+
+      // with slash between the pathname and search
+      .url('http://localhost:8080/hash/?code=auth#')
+      .waitForElementPresent('#app > *', 1000)
+      .assert.urlEquals('http://localhost:8080/hash/?code=auth#/')
+
+      .end()
+  },
+
   /** @type {import('nightwatch').NightwatchTest} */
   'encoding on initial navigation': function (browser) {
     browser

diff --git a/src/history/hash.ts b/src/history/hash.ts
index 15bba544cf370cb1f36d18299211bd8a9baa2683..522ab7b525ef2aac3189d1037d19c0854bd71f4f 100644 (file)
--- a/src/history/hash.ts
+++ b/src/history/hash.ts
@@ -1,5 +1,6 @@
 import { RouterHistory } from './common'
 import { createWebHistory } from './html5'
+import { warn } from '../warning'
 
 /**
  * Creates a hash history.
@@ -30,5 +31,14 @@ export function createWebHashHistory(base?: string): RouterHistory {
   base = location.host ? base || location.pathname : location.pathname
   // allow the user to provide a `#` in the middle: `/base/#/app`
   if (base.indexOf('#') < 0) base += '#'
+
+  if (__DEV__ && !base.endsWith('#/') && !base.endsWith('#')) {
+    warn(
+      `A hash base must end with a "#":\n"${base}" should be "${base.replace(
+        /#.*$/,
+        '#'
+      )}".`
+    )
+  }
   return createWebHistory(base)
 }

diff --git a/src/history/html5.ts b/src/history/html5.ts
index 4a3f169e1f857344b7b8d3d23651db5c910e620f..a7fd474409275a5a52cbb45055cbba90359ad45f 100644 (file)
--- a/src/history/html5.ts
+++ b/src/history/html5.ts
@@ -54,7 +54,7 @@ function createCurrentLocation(
 function useHistoryListeners(
   base: string,
   historyState: ValueContainer<StateEntry>,
-  location: ValueContainer<HistoryLocation>,
+  currentLocation: ValueContainer<HistoryLocation>,
   replace: RouterHistory['replace']
 ) {
   let listeners: NavigationCallback[] = []
@@ -68,13 +68,13 @@ function useHistoryListeners(
   }: {
     state: StateEntry | null
   }) => {
-    const to = createCurrentLocation(base, window.location)
-    const from: HistoryLocation = location.value
+    const to = createCurrentLocation(base, location)
+    const from: HistoryLocation = currentLocation.value
     const fromState: StateEntry = historyState.value
     let delta = 0
 
     if (state) {
-      location.value = to
+      currentLocation.value = to
       historyState.value = state
 
       // ignore the popstate and reset the pauseState
@@ -94,7 +94,7 @@ function useHistoryListeners(
     // need to be passed to the listeners so the navigation can be accepted
     // call all listeners
     listeners.forEach(listener => {
-      listener(location.value, from, {
+      listener(currentLocation.value, from, {
         delta,
         type: NavigationType.pop,
         direction: delta
@@ -107,7 +107,7 @@ function useHistoryListeners(
   }
 
   function pauseListeners() {
-    pauseState = location.value
+    pauseState = currentLocation.value
   }
 
   function listen(callback: NavigationCallback) {
@@ -171,20 +171,20 @@ function buildState(
 }
 
 function useHistoryStateNavigation(base: string) {
-  const { history } = window
+  const { history, location } = window
 
   // private variables
-  let location: ValueContainer<HistoryLocation> = {
-    value: createCurrentLocation(base, window.location),
+  let currentLocation: ValueContainer<HistoryLocation> = {
+    value: createCurrentLocation(base, location),
   }
   let historyState: ValueContainer<StateEntry> = { value: history.state }
   // build current history entry as this is a fresh navigation
   if (!historyState.value) {
     changeLocation(
-      location.value,
+      currentLocation.value,
       {
         back: null,
-        current: location.value,
+        current: currentLocation.value,
         forward: null,
         // the length is off by one, we need to decrease it
         position: history.length - 1,
@@ -202,7 +202,13 @@ function useHistoryStateNavigation(base: string) {
     state: StateEntry,
     replace: boolean
   ): void {
-    const url = createBaseLocation() + base + to
+    const url =
+      createBaseLocation() +
+      // preserve any existing query when base has a hash
+      (base.indexOf('#') > -1 && location.search
+        ? location.pathname + location.search + '#'
+        : base) +
+      to
     try {
       // BROWSER QUIRK
       // NOTE: Safari throws a SecurityError when calling this function 100 times in 30 seconds
@@ -211,7 +217,7 @@ function useHistoryStateNavigation(base: string) {
     } catch (err) {
       warn('Error with push/replace State', err)
       // Force the navigation, this also resets the call count
-      window.location[replace ? 'replace' : 'assign'](url)
+      location[replace ? 'replace' : 'assign'](url)
     }
   }
 
@@ -231,7 +237,7 @@ function useHistoryStateNavigation(base: string) {
     )
 
     changeLocation(to, state, true)
-    location.value = to
+    currentLocation.value = to
   }
 
   function push(to: HistoryLocation, data?: HistoryState) {
@@ -245,7 +251,7 @@ function useHistoryStateNavigation(base: string) {
 
     const state: StateEntry = assign(
       {},
-      buildState(location.value, to, null),
+      buildState(currentLocation.value, to, null),
       {
         position: currentState.position + 1,
       },
@@ -253,11 +259,11 @@ function useHistoryStateNavigation(base: string) {
     )
 
     changeLocation(to, state, false)
-    location.value = to
+    currentLocation.value = to
   }
 
   return {
-    location,
+    location: currentLocation,
     state: historyState,
 
     push,