feat(router): remove partial Promise from router.go

posva · posva · commit 6ed6eee38b59 · 2020-09-11T18:23:02.000+02:00
BREAKING CHANGE: The `router.go()` methods doesn't return anything
(like in Vue 3) anymore. The existing implementation was wrong as it
would resolve the promise for the following navigation if `router.go()`
was called with something that wasn't possible e.g. `router.go(-20)`
right after entering the application would not do anything. Even worse,
the promise returned by that call would resolve **after the next
navigation**. There is no proper native API to implement this
promise-based api properly, but one can write a version that should work
in most scenarios by setting up multiple hooks right before calling
`router.go()`:
```js
export function go(delta) {
  return new Promise((resolve, reject) =&gt; {
    function popStateListener() {
      clearTimeout(timeout)
    }
    window.addEventListener('popstate', popStateListener)

    function clearHooks() {
      removeAfterEach()
      removeOnError()
      window.removeEventListener('popstate', popStateListener)
    }

    // if the popstate event is not called, consider this a failure
    const timeout = setTimeout(() =&gt; {
      clearHooks()
      reject(new Error('Failed to use router.go()'))
      // It's unclear of what value would always work here
    }, 10)

    setImmediate

    const removeAfterEach = router.afterEach((_to, _from, failure) =&gt; {
      clearHooks()
      resolve(failure)
    })
    const removeOnError = router.onError(err =&gt; {
      clearHooks()
      reject(err)
    })

    router.go(delta)
  })
}
```
diff --git a/__tests__/router.spec.ts b/__tests__/router.spec.ts
@@ -175,23 +175,6 @@ describe('Router', () => {
     })
   })
 
-  it('can await router.go', async () => {
-    const { router } = await newRouter()
-    await router.push('/foo')
-    let currentRoute = router.currentRoute.value
-    const [p1, r1] = fakePromise()
-    router.beforeEach(async (to, from, next) => {
-      await p1
-      next()
-    })
-    let p = router.go(-1)
-    expect(router.currentRoute.value).toBe(currentRoute)
-    r1()
-    // resolves to undefined as a working navigation
-    await expect(p).resolves.toBe(undefined)
-    expect(router.currentRoute.value).not.toBe(currentRoute)
-  })
-
   it('can pass replace option to push', async () => {
     const { router, history } = await newRouter()
     jest.spyOn(history, 'replace')
diff --git a/src/router.ts b/src/router.ts
@@ -233,25 +233,22 @@ export interface Router {
   replace(to: RouteLocationRaw): Promise<NavigationFailure | void | undefined>
   /**
    * Go back in history if possible by calling `history.back()`. Equivalent to
-   * `router.go(-1)`.  Returns a Promise. See the limitations at
-   * {@link Router.go}.
+   * `router.go(-1)`.
    */
-  back(): Promise<NavigationFailure | void | undefined>
+  back(): ReturnType<Router['go']>
   /**
    * Go forward in history if possible by calling `history.forward()`.
-   * Equivalent to `router.go(1)`. Returns a Promise. See the limitations at
-   * {@link Router.go}.
+   * Equivalent to `router.go(1)`.
    */
-  forward(): Promise<NavigationFailure | void | undefined>
+  forward(): ReturnType<Router['go']>
   /**
-   * Allows you to move forward or backward through the history. Returns a
-   * Promise that resolves when the navigation finishes. If it wasn't possible
-   * to go back, the promise never resolves or rejects
+   * Allows you to move forward or backward through the history. Calls
+   * `history.go()`.
    *
    * @param delta - The position in the history to which you want to move,
    * relative to the current page
    */
-  go(delta: number): Promise<NavigationFailure | void | undefined>
+  go(delta: number): void
 
   /**
    * Add a navigation guard that executes before any navigation. Returns a
@@ -1025,24 +1022,7 @@ export function createRouter(options: RouterOptions): Router {
       .catch(triggerError)
   }
 
-  function go(delta: number) {
-    return new Promise<NavigationFailure | void | undefined>(
-      (resolve, reject) => {
-        let removeError = errorHandlers.add(err => {
-          removeError()
-          removeAfterEach()
-          reject(err)
-        })
-        let removeAfterEach = afterGuards.add((_to, _from, failure) => {
-          removeError()
-          removeAfterEach()
-          resolve(failure)
-        })
-
-        routerHistory.go(delta)
-      }
-    )
-  }
+  const go = (delta: number) => routerHistory.go(delta)
 
   let started: boolean | undefined
   const installedApps = new Set<App>()
