Repository: kyeotic/raviger (ts)

Issue:
Bug when using a percent (%) symbol query parameter value
When a percent (%) symbol is used as part of the query parameter it will blow up on decodeURIComponent because URLSearchParams returns the value already decoded.
This was tested on:
- Chrome 68.0.3440.84 on Mac
- Microsoft Edge 80.0.361.33 on Mac
- Safari 13.0.2 (15608.2.30.1.1)

The current workaround is to encode the URI before sending to setQueryParams

Pull request:
add Link.basePath
fixes #44 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8301c92..2b6a9e5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,16 @@ 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.3.0] - 2020-01-27
+### Added
+- `<Link basePath>` prop override.
+### Fixed
+- double decoding on `useQueryParams`
+
+## [1.2.0] - 2020-01-21
+### Changed
+- Internal React-Context setup, reduces wasteful re-renders
+
 ## [1.1.1] - 2020-01-09
 ### Fixed
 - `replace: false` on `setQueryParams` replacing `location.hash`
diff --git a/docs/content/api/Link.md b/docs/content/api/Link.md
index 0b813db..aecc053 100644
--- a/docs/content/api/Link.md
+++ b/docs/content/api/Link.md
@@ -13,6 +13,7 @@ export interface LinkProps
   extends React.AnchorHTMLAttributes<HTMLAnchorElement> {
   // Unlike normal <a>, this property is required
   href: string,
+  basePath?: string,
   linkRef?: React.RefObject<HTMLAnchorElement>
 }
 export const Link: React.FC<LinkProps>
@@ -30,6 +31,33 @@ This component takes all the same parameters as the built-in `<a>` tag. It's `on
 </Link>
 {{< /highlight >}}
 
+## BasePath
+
+If a `<Link>` component is inside a router context (there is a `useRoutes` in its parent heirarchy) the `basePath` will be inherited. You can also provide a `basePath` as a `<Link>` prop, which will override an inherited one.
+
+{{< highlight jsx >}}
+import { useRoutes, Link } from 'raviger'
+
+function Home () {
+  return (
+    <div>
+      <Link href="/foo" /> {/* href = /app/foo */}
+      <Link href="/foo" basePath="/bar" /> {/* href = /bar/foo */}
+    </div>
+  )
+}
+
+const routes = {
+  '/': () => <Home />
+}
+
+export default function App() {
+  return useRoutes(routes, { basePath: 'app' })
+  )
+}
+{{< /highlight >}}
+
+
 ## Ref Passing
 
 To pass a [React ref](https://reactjs.org/docs/refs-and-the-dom.html) to the `<Link>` DOM Node use the `linkRef` property. This will assign the ref to the internal `<a>` node. In future version this will become `ref` and use the standard [forwardRef](https://reactjs.org/docs/forwarding-refs.html#forwarding-refs-to-dom-components) API.
\ No newline at end of file
diff --git a/src/Link.js b/src/Link.js
index a3a441f..eb3cf62 100644
--- a/src/Link.js
+++ b/src/Link.js
@@ -2,8 +2,9 @@ import React, { useCallback } from 'react'
 import { navigate } from './navigate'
 import { usePath, useBasePath } from './path.js'
 
-export default function Link({ href, linkRef, ...props }) {
-  const basePath = useBasePath()
+export default function Link({ href, basePath, linkRef, ...props }) {
+  const contextBasePath = useBasePath()
+  basePath = basePath || contextBasePath
   href = getLinkHref(href, basePath)
   const onClick = useCallback(
     e => {
diff --git a/src/querystring.js b/src/querystring.js
index a6626a2..75c0050 100644
--- a/src/querystring.js
+++ b/src/querystring.js
@@ -26,7 +26,7 @@ export function useQueryParams(
 function parseQuery(querystring) {
   return [...new URLSearchParams(querystring)].reduce(
     (result, [key, value]) => {
-      result[key] = decodeURIComponent(value)
+      result[key] = value
       return result
     },
     {}
diff --git a/types/raviger.d.ts b/types/raviger.d.ts
index cc07a5f..4efa1f0 100644
--- a/types/raviger.d.ts
+++ b/types/raviger.d.ts
@@ -20,6 +20,7 @@ export function useRedirect(
 export interface LinkProps
   extends React.AnchorHTMLAttributes<HTMLAnchorElement> {
   href: string
+  basePath?: string
   linkRef?: React.RefObject<HTMLAnchorElement>
 }
 export const Link: React.FC<LinkProps>


Test patch:
diff --git a/test/Link.spec.js b/test/Link.spec.js
index a49ac1f..6346991 100644
--- a/test/Link.spec.js
+++ b/test/Link.spec.js
@@ -27,6 +27,26 @@ describe('Link', () => {
     act(() => void fireEvent.click(getByTestId('link')))
     expect(document.location.pathname).toEqual('/foo')
   })
+  test('navigates to href with basePath', async () => {
+    act(() => navigate('/'))
+    const { getByTestId } = render(
+      <Link href="/foo" basePath="/bar" className="base" data-testid="link">
+        go to foo
+      </Link>
+    )
+    act(() => void fireEvent.click(getByTestId('link')))
+    expect(document.location.pathname).toEqual('/bar/foo')
+  })
+  test('navigates to href with basePath without root slash', async () => {
+    act(() => navigate('/'))
+    const { getByTestId } = render(
+      <Link href="/foo" basePath="bar" className="base" data-testid="link">
+        go to foo
+      </Link>
+    )
+    act(() => void fireEvent.click(getByTestId('link')))
+    expect(document.location.pathname).toEqual('/bar/foo')
+  })
   test('fires onClick', async () => {
     let spy = jest.fn()
     act(() => navigate('/'))
diff --git a/test/querystring.spec.js b/test/querystring.spec.js
index 7e4af05..942eb33 100644
--- a/test/querystring.spec.js
+++ b/test/querystring.spec.js
@@ -31,49 +31,60 @@ describe('useQueryParams', () => {
     act(() => navigate('/about', q1))
     expect(getByTestId('label')).toHaveTextContent(JSON.stringify(q1))
   })
-  describe('setQueryParams', () => {
-    function Route({ replace, foo = 'bar' }) {
-      let [, setQuery] = useQueryParams()
-      return (
-        <button data-testid="update" onClick={() => setQuery({ foo }, replace)}>
-          Set Query
-        </button>
-      )
-    }
-    test('updates query', async () => {
-      act(() => navigate('/about', { bar: 'foo' }))
-      const { getByTestId } = render(<Route replace />)
+})
 
-      act(() => void fireEvent.click(getByTestId('update')))
-      expect(document.location.search).toEqual('?foo=bar')
-    })
-    test('merges query', async () => {
-      act(() => navigate('/about', { bar: 'foo' }))
-      const { getByTestId } = render(<Route replace={false} />)
+describe('setQueryParams', () => {
+  function Route({ replace, foo = 'bar' }) {
+    let [query, setQuery] = useQueryParams()
+    return (
+      <button data-testid="update" onClick={() => setQuery({ foo }, replace)}>
+        Set Query: {query.foo}
+      </button>
+    )
+  }
+  test('updates query', async () => {
+    act(() => navigate('/about', { bar: 'foo' }))
+    const { getByTestId } = render(<Route replace />)
 
-      act(() => void fireEvent.click(getByTestId('update')))
-      expect(document.location.search).toContain('bar=foo')
-      expect(document.location.search).toContain('foo=bar')
-    })
-    test('merges query without null', async () => {
-      act(() => navigate('/about', { bar: 'foo' }))
-      const { getByTestId } = render(<Route replace={false} foo={null} />)
+    act(() => void fireEvent.click(getByTestId('update')))
+    expect(document.location.search).toEqual('?foo=bar')
+  })
+  test('handles encoded values', async () => {
+    act(() => navigate('/about', { foo: 'foo' }))
+    const { getByTestId } = render(<Route replace={false} foo={'%100'} />)
 
-      act(() => void fireEvent.click(getByTestId('update')))
-      expect(document.location.search).toContain('bar=foo')
-      expect(document.location.search).not.toContain('foo=')
-    })
-    test('removes has when replace is true', async () => {
-      act(() => navigate('/about#test'))
-      const { getByTestId } = render(<Route replace />)
-      act(() => void fireEvent.click(getByTestId('update')))
-      expect(document.location.hash).toEqual('')
-    })
-    test('retains has when replace is false', async () => {
-      act(() => navigate('/about#test'))
-      const { getByTestId } = render(<Route replace={false} />)
-      act(() => void fireEvent.click(getByTestId('update')))
-      expect(document.location.hash).toEqual('#test')
-    })
+    act(() => void fireEvent.click(getByTestId('update')))
+    expect(document.location.search).toContain(
+      `foo=${encodeURIComponent('%100')}`
+    )
+    expect(getByTestId('update')).toHaveTextContent('Set Query: %100')
+  })
+  test('merges query', async () => {
+    act(() => navigate('/about', { bar: 'foo' }))
+    const { getByTestId } = render(<Route replace={false} />)
+
+    act(() => void fireEvent.click(getByTestId('update')))
+    expect(document.location.search).toContain('bar=foo')
+    expect(document.location.search).toContain('foo=bar')
+  })
+  test('merges query without null', async () => {
+    act(() => navigate('/about', { bar: 'foo' }))
+    const { getByTestId } = render(<Route replace={false} foo={null} />)
+
+    act(() => void fireEvent.click(getByTestId('update')))
+    expect(document.location.search).toContain('bar=foo')
+    expect(document.location.search).not.toContain('foo=')
+  })
+  test('removes has when replace is true', async () => {
+    act(() => navigate('/about#test'))
+    const { getByTestId } = render(<Route replace />)
+    act(() => void fireEvent.click(getByTestId('update')))
+    expect(document.location.hash).toEqual('')
+  })
+  test('retains has when replace is false', async () => {
+    act(() => navigate('/about#test'))
+    const { getByTestId } = render(<Route replace={false} />)
+    act(() => void fireEvent.click(getByTestId('update')))
+    expect(document.location.hash).toEqual('#test')
   })
 })