fix(hash): force navigation restore on manual navigation (#921)

Fix #916

diff --git a/__tests__/hash-manual-navigation.spec.ts b/__tests__/hash-manual-navigation.spec.ts
new file mode 100644 (file)
index 0000000..a465bb3
--- /dev/null
+++ b/__tests__/hash-manual-navigation.spec.ts
@@ -0,0 +1,83 @@
+import { createMemoryHistory, createRouter, RouterHistory } from '../src'
+import { tick } from './utils'
+
+const component = {}
+
+interface RouterHistory_Test extends RouterHistory {
+  changeURL(url: string): void
+}
+
+describe('hash history edge cases', () => {
+  it('correctly sets the url when it is manually changed but aborted with a redirect in guard', async () => {
+    const history = createMemoryHistory() as RouterHistory_Test
+    const router = createRouter({
+      history,
+      routes: [
+        { path: '/', component },
+        { path: '/foo', component },
+      ],
+    })
+
+    await router.push('/foo?step=1')
+    await router.push('/foo?step=2')
+    await router.push('/foo?step=3')
+    router.back()
+    await tick() // wait for router listener on history
+    expect(router.currentRoute.value.fullPath).toBe('/foo?step=2')
+
+    // force a redirect next time
+    const removeListener = router.beforeEach(to => {
+      if (to.path === '/') {
+        removeListener()
+        return '/foo?step=2'
+      }
+      return
+    })
+
+    // const spy = jest.spyOn(history, 'go')
+
+    history.changeURL('/')
+    await tick()
+    expect(router.currentRoute.value.fullPath).toBe('/foo?step=2')
+    expect(history.location).toBe('/foo?step=2')
+    // expect(spy).toHaveBeenCalledTimes(1)
+    // expect(spy).toHaveBeenCalledWith(-1)
+  })
+
+  it('correctly sets the url when it is manually changed but aborted with guard', async () => {
+    const history = createMemoryHistory() as RouterHistory_Test
+    const router = createRouter({
+      history,
+      routes: [
+        { path: '/', component },
+        { path: '/foo', component },
+      ],
+    })
+
+    await router.push('/foo?step=1')
+    await router.push('/foo?step=2')
+    await router.push('/foo?step=3')
+    router.back()
+    await tick() // wait for router listener on history
+    expect(router.currentRoute.value.fullPath).toBe('/foo?step=2')
+
+    // force a redirect next time
+    const removeListener = router.beforeEach(to => {
+      if (to.path === '/') {
+        removeListener()
+        return false
+      }
+
+      return
+    })
+
+    // const spy = jest.spyOn(history, 'go')
+
+    history.changeURL('/')
+    await tick()
+    expect(router.currentRoute.value.fullPath).toBe('/foo?step=2')
+    expect(history.location).toBe('/foo?step=2')
+    // expect(spy).toHaveBeenCalledTimes(1)
+    // expect(spy).toHaveBeenCalledWith(-1)
+  })
+})

diff --git a/src/router.ts b/src/router.ts
index 5cc118fd6696cd9b00ddf30574ed4383ae1aa9d1..4895b884c98195520a940e9ff61f2e91ac50b05c 100644 (file)
--- a/src/router.ts
+++ b/src/router.ts
@@ -13,7 +13,7 @@ import {
   RouteLocationOptions,
   MatcherLocationRaw,
 } from './types'
-import { RouterHistory, HistoryState } from './history/common'
+import { RouterHistory, HistoryState, NavigationType } from './history/common'
 import {
   ScrollPosition,
   getSavedScrollPosition,
@@ -689,7 +689,7 @@ export function createRouter(options: RouterOptions): Router {
               ) &&
               // and we have done it a couple of times
               redirectedFrom &&
-              // @ts-expect-error
+              // @ts-expect-error: added only in dev
               (redirectedFrom._count = redirectedFrom._count
                 ? // @ts-expect-error
                   redirectedFrom._count + 1
@@ -980,7 +980,24 @@ export function createRouter(options: RouterOptions): Router {
               (error as NavigationRedirectError).to,
               toLocation
               // avoid an uncaught rejection, let push call triggerError
-            ).catch(noop)
+            )
+              .then(failure => {
+                // manual change in hash history #916 ending up in the URL not
+                // changing but it was changed by the manual url change, so we
+                // need to manually change it ourselves
+                if (
+                  isNavigationFailure(
+                    failure,
+                    ErrorTypes.NAVIGATION_ABORTED |
+                      ErrorTypes.NAVIGATION_DUPLICATED
+                  ) &&
+                  !info.delta &&
+                  info.type === NavigationType.pop
+                ) {
+                  routerHistory.go(-1, false)
+                }
+              })
+              .catch(noop)
             // avoid the then branch
             return Promise.reject()
           }
@@ -1000,7 +1017,21 @@ export function createRouter(options: RouterOptions): Router {
             )
 
           // revert the navigation
-          if (failure && info.delta) routerHistory.go(-info.delta, false)
+          if (failure) {
+            if (info.delta) {
+              routerHistory.go(-info.delta, false)
+            } else if (
+              info.type === NavigationType.pop &&
+              isNavigationFailure(
+                failure,
+                ErrorTypes.NAVIGATION_ABORTED | ErrorTypes.NAVIGATION_DUPLICATED
+              )
+            ) {
+              // manual change in hash history #916
+              // it's like a push but lacks the information of the direction
+              routerHistory.go(-1, false)
+            }
+          }
 
           triggerAfterEach(
             toLocation as RouteLocationNormalizedLoaded,