Repository: kyeotic/raviger
Pull request #63 (2020-07-31 22:00:57)

Fix `replace` handling on `navigate`
This addresses the issue originally raised in #62


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5cbd457..bbd05cc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.4.5] - 2020-07-31
+### Fixed
+- `navigate` handling of `replace` in edge cases for `replaceOrQuery`
+
 ## [1.4.4] - 2020-06-26
 ### Fixed
 - `basePath` matches not using case-insensitive like route paths
diff --git a/example/src/App.js b/example/src/App.js
index ff184ca..0aa10c4 100644
--- a/example/src/App.js
+++ b/example/src/App.js
@@ -40,6 +40,7 @@ const App = () => {
         <Link href="/fallthrough">Fallthrough 1</Link>
         <Link href="/fallthrough/nested">Fallthrough Nested</Link>
         <Link href="/filter">Filters</Link>
+        <Link href="/redirect">Redirect</Link>
       </Nav>
       <div></div>
       <span style={{ display: 'block' }}>Root Path: {path}</span>
diff --git a/example/src/index.js b/example/src/index.js
index 3824a08..798fbaa 100644
--- a/example/src/index.js
+++ b/example/src/index.js
@@ -1,7 +1,7 @@
 import React from 'react'
 import ReactDOM from 'react-dom'
-// import App from './App'
-import App from './NestedApp.js'
+import App from './App'
+// import App from './NestedApp.js'
 // eslint-disable-next-line import/no-unresolved
 import whyDidYouRender from 'whyDidYouRender'
 
diff --git a/src/navigate.js b/src/navigate.js
index 67b676f..03a9f17 100644
--- a/src/navigate.js
+++ b/src/navigate.js
@@ -4,7 +4,7 @@ import { isNode } from './node.js'
 const defaultPrompt = 'Are you sure you want to leave this page?'
 const interceptors = new Set()
 
-export function navigate(url, replaceOrQuery = false, replace = false) {
+export function navigate(url, replaceOrQuery, replace) {
   if (typeof url !== 'string') {
     throw new Error(`"url" must be a string, was provided a(n) ${typeof url}`)
   }
@@ -16,8 +16,10 @@ export function navigate(url, replaceOrQuery = false, replace = false) {
   if (shouldCancelNavigation()) return
   if (replaceOrQuery !== null && typeof replaceOrQuery === 'object') {
     url += '?' + new URLSearchParams(replaceOrQuery).toString()
-  } else {
+  } else if (replace === undefined && replaceOrQuery !== undefined) {
     replace = replaceOrQuery
+  } else if (replace === undefined && replaceOrQuery === undefined) {
+    replace = false
   }
   window.history[`${replace ? 'replace' : 'push'}State`](null, null, url)
   dispatchEvent(new PopStateEvent('popstate', null))


Test patch:
diff --git a/test/navigate.spec.js b/test/navigate.spec.js
index 416ca98..01a0881 100644
--- a/test/navigate.spec.js
+++ b/test/navigate.spec.js
@@ -3,12 +3,22 @@ import { render, act } from '@testing-library/react'
 import { useNavigationPrompt, navigate } from '../src/main.js'
 
 const originalConfirm = window.confirm
+const originalReplaceState = window.history.replaceState
+const originalPushState = window.history.pushState
 
 beforeEach(() => {
   window.confirm = originalConfirm
+  window.history.replaceState = originalReplaceState
+  window.history.pushState = originalPushState
   act(() => navigate('/'))
 })
 
+afterEach(() => {
+  window.confirm = originalConfirm
+  window.history.replaceState = originalReplaceState
+  window.history.pushState = originalPushState
+})
+
 describe('useNavigationPrompt', () => {
   function Route({ block = true, prompt }) {
     useNavigationPrompt(block, prompt)
@@ -59,3 +69,47 @@ describe('useNavigationPrompt', () => {
     expect(window.confirm).toHaveBeenCalledWith('custom')
   })
 })
+
+describe('navigate', () => {
+  test('replace is correctly set in all cases', async () => {
+    window.history.replaceState = jest.fn()
+    window.history.pushState = jest.fn()
+    const url = '/foo'
+
+    navigate(url, false)
+    expect(window.history.pushState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, true)
+    expect(window.history.replaceState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, null, true)
+    expect(window.history.replaceState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, null, false)
+    expect(window.history.pushState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, undefined, true)
+    expect(window.history.replaceState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, undefined, false)
+    expect(window.history.pushState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, {})
+    expect(window.history.pushState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, {}, true)
+    expect(window.history.replaceState).toHaveBeenCalled()
+    jest.clearAllMocks()
+
+    navigate(url, {}, false)
+    expect(window.history.pushState).toHaveBeenCalled()
+    jest.clearAllMocks()
+  })
+})