code review suggestions

src/utilities/routerNavigation.browser.spec.ts

@@ -1,102 +1,48 @@
-import { beforeEach, afterEach, describe, expect, test, vi } from 'vitest'
-import * as isBrowser from '@/utilities/isBrowser'
+import { describe, expect, test, vi } from 'vitest'
 import { createRouterNavigation } from '@/utilities/routerNavigation'
 import { random } from '@/utilities/testHelpers'
 import * as utilities from '@/utilities/updateBrowserUrl'
 
 describe('createRouterNavigation', () => {
-  describe('when isBrowser is true', () => {
-    test('when go is called, forwards call to window history', () => {
-      vi.spyOn(window.history, 'go')
+  test('when go is called, forwards call to window history', () => {
+    vi.spyOn(window.history, 'go')
 
-      const delta = random.number({ min: 0, max: 100 })
-      const history = createRouterNavigation()
+    const delta = random.number({ min: 0, max: 100 })
+    const history = createRouterNavigation()
 
-      history.go(delta)
+    history.go(delta)
 
-      expect(window.history.go).toHaveBeenCalledWith(delta)
-    })
-
-    test('when back is called, forwards call to window history', () => {
-      vi.spyOn(window.history, 'back')
-
-      const history = createRouterNavigation()
-
-      history.back()
-
-      expect(window.history.back).toHaveBeenCalledOnce()
-    })
-
-    test('when forward is called, forwards call to window history', () => {
-      vi.spyOn(window.history, 'forward')
-
-      const history = createRouterNavigation()
-
-      history.forward()
-
-      expect(window.history.forward).toHaveBeenCalledOnce()
-    })
-
-    test('when update is called, calls updateBrowserUrl', () => {
-      vi.spyOn(utilities, 'updateBrowserUrl')
-
-      const url = random.number().toString()
-      const history = createRouterNavigation()
-
-      history.update(url)
-
-      expect(utilities.updateBrowserUrl).toHaveBeenCalledWith(url)
-    })
+    expect(window.history.go).toHaveBeenCalledWith(delta)
   })
 
-  describe('when isBrowser is false', () => {
-    beforeEach(() => {
-      vi.spyOn(isBrowser, 'isBrowser').mockImplementation(() => false)
-    })
-    afterEach(() => {
-      vi.restoreAllMocks()
-    })
-
-    test('when go is called, does nothing', () => {
-      vi.spyOn(window.history, 'go')
-
-      const delta = random.number({ min: 0, max: 100 })
-      const history = createRouterNavigation()
-
-      history.go(delta)
+  test('when back is called, forwards call to window history', () => {
+    vi.spyOn(window.history, 'back')
 
-      expect(window.history.go).toHaveBeenCalledTimes(0)
-    })
+    const history = createRouterNavigation()
 
-    test('when back is called, does nothing', () => {
-      vi.spyOn(window.history, 'back')
+    history.back()
 
-      const history = createRouterNavigation()
-
-      history.back()
-
-      expect(window.history.back).toHaveBeenCalledTimes(0)
-    })
+    expect(window.history.back).toHaveBeenCalledOnce()
+  })
 
-    test('when forward is called, does nothing', () => {
-      vi.spyOn(window.history, 'forward')
+  test('when forward is called, forwards call to window history', () => {
+    vi.spyOn(window.history, 'forward')
 
-      const history = createRouterNavigation()
+    const history = createRouterNavigation()
 
-      history.forward()
+    history.forward()
 
-      expect(window.history.forward).toHaveBeenCalledTimes(0)
-    })
+    expect(window.history.forward).toHaveBeenCalledOnce()
+  })
 
-    test('when update is called, calls updateBrowserUrl', () => {
-      vi.spyOn(utilities, 'updateBrowserUrl')
+  test('when update is called, calls updateBrowserUrl', () => {
+    vi.spyOn(utilities, 'updateBrowserUrl')
 
-      const url = random.number().toString()
-      const history = createRouterNavigation()
+    const url = random.number().toString()
+    const history = createRouterNavigation()
 
-      history.update(url)
+    history.update(url)
 
-      expect(utilities.updateBrowserUrl).toHaveBeenCalledWith(url)
-    })
+    expect(utilities.updateBrowserUrl).toHaveBeenCalledWith(url)
   })
 })

src/utilities/routerNavigation.spec.ts

@@ -0,0 +1,8 @@
+import { describe, expect, test } from 'vitest'
+import { createRouterNavigation } from '@/utilities/routerNavigation'
+
+describe('createRouterNavigation', () => {
+  test('is not implemented, and throws exception', () => {
+    expect(() => createRouterNavigation()).toThrowError('not implemented')
+  })
+})

src/utilities/routerNavigation.ts

@@ -1,11 +1,15 @@
 import { isBrowser } from '@/utilities/isBrowser'
 import { updateBrowserUrl } from '@/utilities/updateBrowserUrl'
 
+type RouterNavigationUpdateOptions = {
+  replace?: boolean,
+}
+
 type RouterNavigation = {
   forward: () => void,
   back: () => void,
   go: (delta: number) => void,
-  update: (url: string) => void,
+  update: (url: string, options?: RouterNavigationUpdateOptions) => void,
 }
 
 export function createRouterNavigation(): RouterNavigation {
@@ -22,10 +26,5 @@ function createWebNavigation(): RouterNavigation {
 }
 
 function createMemoryNavigation(): RouterNavigation {
-  return {
-    go: () => {},
-    back: () => {},
-    forward: () => {},
-    update: updateBrowserUrl,
-  }
+  throw 'not implemented'
 }