refactor: remove doInitialNavigation

diff --git a/__tests__/router.spec.ts b/__tests__/router.spec.ts
index c448cbba584cfb11b20a07ec810db2eb48e0bb41..b5abfc04d214dfc643957b894cbbc71ec499a142 100644 (file)
--- a/__tests__/router.spec.ts
+++ b/__tests__/router.spec.ts
@@ -2,7 +2,12 @@ import fakePromise from 'faked-promise'
 import { createRouter, createMemoryHistory, createHistory } from '../src'
 import { NavigationCancelled } from '../src/errors'
 import { createDom, components, tick } from './utils'
-import { RouteRecord, RouteLocation } from '../src/types'
+import {
+  RouteRecord,
+  RouteLocation,
+  START_LOCATION_NORMALIZED,
+} from '../src/types'
+import { RouterHistory } from '../src/history/common'
 
 const routes: RouteRecord[] = [
   { path: '/', component: components.Home },
@@ -23,6 +28,14 @@ const routes: RouteRecord[] = [
   },
 ]
 
+async function newRouter({ history }: { history?: RouterHistory } = {}) {
+  history = history || createMemoryHistory()
+  const router = createRouter({ history, routes })
+  await router.push('/')
+
+  return { history, router }
+}
+
 describe('Router', () => {
   beforeAll(() => {
     createDom()
@@ -31,22 +44,15 @@ describe('Router', () => {
   it('can be instantiated', () => {
     const history = createMemoryHistory()
     const router = createRouter({ history, routes })
-    expect(router.currentRoute.value).toEqual({
-      name: undefined,
-      fullPath: '/',
-      hash: '',
-      params: {},
-      path: '/',
-      query: {},
-      meta: {},
-    })
+    expect(router.currentRoute.value).toEqual(START_LOCATION_NORMALIZED)
   })
 
   // TODO: should do other checks not based on history implem
-  it.skip('takes browser location', () => {
+  it.skip('takes browser location', async () => {
     const history = createMemoryHistory()
     history.replace('/search?q=dog#footer')
-    const router = createRouter({ history, routes })
+    const { router } = await newRouter({ history })
+    await router.push(history.location)
     expect(router.currentRoute).toEqual({
       fullPath: '/search?q=dog#footer',
       hash: '#footer',
@@ -57,8 +63,7 @@ describe('Router', () => {
   })
 
   it('calls history.push with router.push', async () => {
-    const history = createMemoryHistory()
-    const router = createRouter({ history, routes })
+    const { router, history } = await newRouter()
     jest.spyOn(history, 'push')
     await router.push('/foo')
     expect(history.push).toHaveBeenCalledTimes(1)
@@ -72,7 +77,7 @@ describe('Router', () => {
 
   it('calls history.replace with router.replace', async () => {
     const history = createMemoryHistory()
-    const router = createRouter({ history, routes })
+    const { router } = await newRouter({ history })
     jest.spyOn(history, 'replace')
     await router.replace('/foo')
     expect(history.replace).toHaveBeenCalledTimes(1)
@@ -85,8 +90,7 @@ describe('Router', () => {
   })
 
   it('can pass replace option to push', async () => {
-    const history = createMemoryHistory()
-    const router = createRouter({ history, routes })
+    const { router, history } = await newRouter()
     jest.spyOn(history, 'replace')
     await router.push({ path: '/foo', replace: true })
     expect(history.replace).toHaveBeenCalledTimes(1)
@@ -153,6 +157,7 @@ describe('Router', () => {
       const [p2, r2] = fakePromise()
       const history = createMemoryHistory()
       const router = createRouter({ history, routes })
+
       // navigate first to add entries to the history stack
       await router.push('/foo')
       await router.push('/p/a')

diff --git a/jest.config.js b/jest.config.js
index 74f91374226f2427d7aa63a28f1eb529898b3e9b..95862540bcdee87e693fa0cfe9639dd489538847 100644 (file)
--- a/jest.config.js
+++ b/jest.config.js
@@ -13,5 +13,6 @@ module.exports = {
     'src/consola.ts',
   ],
   testMatch: ['<rootDir>/__tests__/**/*.spec.ts?(x)'],
+  watchPathIgnorePatterns: ['<rootDir>/node_modules'],
   testEnvironment: 'node',
 }

diff --git a/src/history/html5.ts b/src/history/html5.ts
index e58b8a3f40e388d22f5dab4cd9aa600b4a777c30..8d98b1677c2c74f502c40620e594bd3f7a136a88 100644 (file)
--- a/src/history/html5.ts
+++ b/src/history/html5.ts
@@ -259,7 +259,7 @@ function useHistoryStateNavigation(base: string) {
 }
 
 export default function createHistory(base: string = ''): RouterHistory {
-  if ('scrollRestoration' in history) {
+  if ('scrollRestoration' in window.history) {
     history.scrollRestoration = 'manual'
   }
 

diff --git a/src/index.ts b/src/index.ts
index f4dfc0aecf46cff5aac21e3d6f9d8f5407a7199c..943a7a9d1fda1c09e0c5bafa4eab175be129b484 100644 (file)
--- a/src/index.ts
+++ b/src/index.ts
@@ -5,7 +5,10 @@ import createMemoryHistory from './history/memory'
 import createHashHistory from './history/hash'
 import View from './components/View'
 import Link from './components/Link'
-import { RouteLocationNormalized } from './types'
+import {
+  RouteLocationNormalized,
+  START_LOCATION_NORMALIZED as START_LOCATION,
+} from './types'
 
 declare module '@vue/runtime-core' {
   function inject(name: 'router'): Router
@@ -30,12 +33,15 @@ export function RouterPlugin(app: App, router: Router) {
   app.component('RouterView', View as any)
 
   let started = false
+  // TODO: can we use something that isn't a mixin?
   app.mixin({
     beforeCreate() {
       if (!started) {
         router.setActiveApp(this)
 
-        router.doInitialNavigation().catch(err => {
+        // TODO: this initial navigation is only necessary on client, on server it doesn't make sense
+        // because it will create an extra unecessary navigation and could lead to problems
+        router.push(router.history.location).catch(err => {
           console.error('Unhandled error', err)
         })
         started = true
@@ -56,4 +62,5 @@ export {
   createRouter,
   RouteLocationNormalized,
   Router,
+  START_LOCATION,
 }

diff --git a/src/router.ts b/src/router.ts
index 0d1d2c40992430c29235bdd72aa6d94d2570c7d7..abbf7032c33e87276a213110ae07b7f73b848bea 100644 (file)
--- a/src/router.ts
+++ b/src/router.ts
@@ -17,7 +17,6 @@ import {
   stringifyURL,
   normalizeQuery,
   HistoryLocationNormalized,
-  START,
 } from './history/common'
 import {
   ScrollToPosition,
@@ -55,6 +54,7 @@ export interface RouterOptions {
 }
 
 export interface Router {
+  history: RouterHistory
   currentRoute: Ref<RouteLocationNormalized>
 
   resolve(to: RouteLocation): RouteLocationNormalized
@@ -63,7 +63,6 @@ export interface Router {
   replace(to: RouteLocation): Promise<RouteLocationNormalized>
 
   // TODO: find a way to remove it
-  doInitialNavigation(): Promise<void>
   setActiveApp(vm: TODO): void
 
   beforeEach(guard: NavigationGuard): ListenerRemover
@@ -232,12 +231,17 @@ export function createRouter({
     )
       return currentRoute.value
 
+    const isFirstNavigation =
+      currentRoute.value === START_LOCATION_NORMALIZED &&
+      to === history.location
+
     const toLocation: RouteLocationNormalized = location
     pendingLocation = toLocation
     // trigger all guards, throw if navigation is rejected
     try {
       await navigate(toLocation, currentRoute.value)
     } catch (error) {
+      if (isFirstNavigation) markAsReady(error)
       if (NavigationGuardRedirect.is(error)) {
         // push was called while waiting in guards
         if (pendingLocation !== toLocation) {
@@ -261,21 +265,28 @@ export function createRouter({
 
     // push was called while waiting in guards
     if (pendingLocation !== toLocation) {
-      throw new NavigationCancelled(toLocation, currentRoute.value)
+      const error = new NavigationCancelled(toLocation, currentRoute.value)
+      // TODO: refactor errors to be more lightweight
+      if (isFirstNavigation) markAsReady(error)
+      throw error
     }
 
     // change URL
-    if (to.replace === true) history.replace(url)
-    else history.push(url)
+    if (!isFirstNavigation) {
+      if (to.replace === true) history.replace(url)
+      else history.push(url)
+    }
 
     const from = currentRoute.value
     currentRoute.value = markNonReactive(toLocation)
-    updateReactiveRoute()
-    handleScroll(toLocation, from).catch(err => triggerError(err, false))
+    if (!isFirstNavigation)
+      handleScroll(toLocation, from).catch(err => triggerError(err, false))
 
     // navigation is confirmed, call afterGuards
     for (const guard of afterGuards) guard(toLocation, from)
 
+    markAsReady()
+
     return currentRoute.value
   }
 
@@ -380,7 +391,6 @@ export function createRouter({
         ...to,
         ...matchedRoute,
       })
-      updateReactiveRoute()
       // TODO: refactor with a state getter
       // const { scroll } = history.state
       const { state } = window.history
@@ -451,16 +461,6 @@ export function createRouter({
     if (shouldThrow) throw error
   }
 
-  function updateReactiveRoute() {
-    if (!app) return
-    // TODO: matched should be non enumerable and the defineProperty here shouldn't be necessary
-    const route = { ...currentRoute.value }
-    Object.defineProperty(route, 'matched', { enumerable: false })
-    // @ts-ignore
-    app._route = Object.freeze(route)
-    markAsReady()
-  }
-
   function isReady(): Promise<void> {
     if (ready && currentRoute.value !== START_LOCATION_NORMALIZED)
       return Promise.resolve()
@@ -470,7 +470,7 @@ export function createRouter({
   }
 
   function markAsReady(err?: any): void {
-    if (ready || currentRoute.value === START_LOCATION_NORMALIZED) return
+    if (ready) return
     ready = true
     for (const [resolve] of onReadyCbs) {
       // TODO: is this okay?
@@ -484,63 +484,6 @@ export function createRouter({
     onReadyCbs = []
   }
 
-  async function doInitialNavigation(): Promise<void> {
-    // let the user call replace or push on SSR
-    if (history.location === START) return
-    // TODO: refactor code that was duplicated from push method
-    const toLocation: RouteLocationNormalized = resolveLocation(
-      history.location,
-      currentRoute.value
-    )
-
-    pendingLocation = toLocation
-    // trigger all guards, throw if navigation is rejected
-    try {
-      await navigate(toLocation, currentRoute.value)
-    } catch (error) {
-      markAsReady(error)
-      if (NavigationGuardRedirect.is(error)) {
-        // push was called while waiting in guards
-        if (pendingLocation !== toLocation) {
-          // TODO: trigger onError as well
-          throw new NavigationCancelled(toLocation, currentRoute.value)
-        }
-        // TODO: setup redirect stack
-        await push(error.to)
-        return
-      } else {
-        // TODO: write tests
-        // triggerError as well
-        if (pendingLocation !== toLocation) {
-          // TODO: trigger onError as well
-          throw new NavigationCancelled(toLocation, currentRoute.value)
-        }
-
-        // this throws, so nothing ahead happens
-        triggerError(error)
-      }
-    }
-
-    // push was called while waiting in guards
-    if (pendingLocation !== toLocation) {
-      const error = new NavigationCancelled(toLocation, currentRoute.value)
-      markAsReady(error)
-      throw error
-    }
-
-    // NOTE: here we removed the pushing to history part as the history
-    // already contains current location
-
-    const from = currentRoute.value
-    currentRoute.value = markNonReactive(toLocation)
-    updateReactiveRoute()
-
-    // navigation is confirmed, call afterGuards
-    for (const guard of afterGuards) guard(toLocation, from)
-
-    markAsReady()
-  }
-
   async function handleScroll(
     to: RouteLocationNormalized,
     from: RouteLocationNormalized,
@@ -556,7 +499,6 @@ export function createRouter({
 
   function setActiveApp(vm: TODO) {
     app = vm
-    updateReactiveRoute()
   }
 
   const router: Router = {
@@ -570,7 +512,7 @@ export function createRouter({
     onError,
     isReady,
 
-    doInitialNavigation,
+    history,
     setActiveApp,
   }