Repository: kyeotic/raviger (ts)

Issue:
Class on active links
If the `Link` component has the path `/login` and the user is currently navigated to `/login`, the Link component could have a class labelling it as an active link as used in the vue-router: https://router.vuejs.org/api/#linkactiveclass

The API could follow after react-router: https://reacttraining.com/react-router/web/api/NavLink

Additionally, a class for exact and non-exact matching would be appreciated. For example, if the user is at `/user/login` the `/user` Link could have an `link-active` class while the `/user/login` Link could have an `link-active-exact` class.

Pull request description:
add activeClass and exactActiveClass
closes #19 

This is a work-in-progress. I'd like to think about this API for another day, and consider how the classes stack during exact matching. It may also be necessary to split this into a separate component, so that not every `<Link>` component is listening for path change events.

Feedback welcome


*Edit*

I decided the location watcher was too much on a common component, and it should be opt-in. So I went with `<ActiveLink>`

Patch:
diff --git a/README.md b/README.md
index 4f3c2a9..5ea6ab9 100644
--- a/README.md
+++ b/README.md
@@ -163,6 +163,13 @@ The second parameter, if provided as falsy, will *merge* the provided query para
 
 This component takes all the same parameters as the built-in `<a>` tag. It's `onClick` will be extended to perform local navigation, and if it is inside a component returned from `useRoutes` it will have the provided `basePath` preprended to its `href`.
 
+### **ActiveLink**
+
+Just like `<Link>`, but with two additional properties for modifying the `className`
+
+* *activeClass: string* If the `href` matches the start of the current path this will be appended to the `<a> `className`.
+* *exactActiveClass: string* If the `href` matches the cirrent path exactly this will be appended to the `<a> `className`. Stacks with `activeClass*
+
 ## **useRedirect**
 
 A redirect hook.
diff --git a/jest.config.js b/jest.config.js
index 1b35aa3..d1a834b 100644
--- a/jest.config.js
+++ b/jest.config.js
@@ -21,7 +21,7 @@ module.exports = {
   // collectCoverage: false,
 
   // An array of glob patterns indicating a set of files for which coverage information should be collected
-  // collectCoverageFrom: null,
+  collectCoverageFrom: ['src/**/*.js', '!src/main.js'],
 
   // The directory where Jest should output its coverage files
   coverageDirectory: 'coverage',
@@ -40,7 +40,14 @@ module.exports = {
   // ],
 
   // An object that configures minimum threshold enforcement for coverage results
-  // coverageThreshold: null,
+  coverageThreshold: {
+    global: {
+      branches: 80,
+      functions: 50,
+      lines: 50,
+      statements: 50
+    }
+  },
 
   // A path to a custom dependency extractor
   // dependencyExtractor: null,
diff --git a/package.json b/package.json
index 44dcdaa..74cc25b 100644
--- a/package.json
+++ b/package.json
@@ -37,7 +37,7 @@
     "test:watch": "jest --watch",
     "test:coverage": "jest --coverage",
     "test:coverage:open": "run-s test:coverage coverage:report",
-    "test:unit:ci": "jest --ci --reporters=default --reporters=jest-junit",
+    "test:unit:ci": "jest --ci --coverage --reporters=default --reporters=jest-junit",
     "coverage:report": "open coverage/lcov-report/index.html",
     "test:ci": "run-s check test:unit:ci size",
     "size": "run-s build size:check",
diff --git a/src/Link.js b/src/Link.js
index 59e8197..4230688 100644
--- a/src/Link.js
+++ b/src/Link.js
@@ -1,11 +1,10 @@
 import React, { useCallback } from 'react'
 import { navigate } from './navigate'
-import { useRouter } from './context.js'
+import { usePath, useBasePath } from './path.js'
 
 export default function Link(props) {
-  const { basePath } = useRouter()
-  const href =
-    props.href.substr(0, 1) === '/' ? basePath + props.href : props.href
+  const basePath = useBasePath()
+  const href = getLinkHref(props.href, basePath)
   const onClick = useCallback(
     e => {
       try {
@@ -19,15 +18,30 @@ export default function Link(props) {
         navigate(e.currentTarget.href)
       }
     },
-    [basePath, href]
+    [basePath, href, props.onClick]
   )
   return <a {...props} href={href} onClick={onClick} />
 }
 
+export function ActiveLink(props) {
+  const basePath = useBasePath()
+  const path = usePath(basePath)
+  const href = getLinkHref(props.href, basePath)
+  let { className, activeClass, exactActiveClass, ...rest } = props
+  if (!className) className = ''
+  if (exactActiveClass && path === href) className += ` ${exactActiveClass}`
+  if (activeClass && path.startsWith(href)) className += ` ${activeClass}`
+  return <Link {...rest} className={className} />
+}
+
+function getLinkHref(href, basePath = '') {
+  return href.substring(0, 1) === '/' ? basePath + href : href
+}
+
 function shouldTrap(e) {
   return (
     !e.defaultPrevented && // onClick prevented default
     e.button === 0 && // ignore everything but left clicks
-    !(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey)
+    !(e.metaKey || e.altKey || e.ctrlKey || e.shiftKey)
   )
 }
diff --git a/src/main.js b/src/main.js
index 28d0b86..e352f70 100644
--- a/src/main.js
+++ b/src/main.js
@@ -1,6 +1,6 @@
 export { useRoutes } from './router.js'
 export { useRedirect } from './redirect.js'
-export { default as Link } from './Link.js'
+export { default as Link, ActiveLink } from './Link.js'
 export { navigate } from './navigate.js'
 export { usePath, useBasePath, usePopState } from './path.js'
 export { useQueryParams } from './querystring.js'
diff --git a/src/path.js b/src/path.js
index ea08f5c..6844927 100644
--- a/src/path.js
+++ b/src/path.js
@@ -10,8 +10,7 @@ export function usePath(basePath) {
 }
 
 export function useBasePath() {
-  let context = useRouter()
-  return context.basePath
+  return useRouter().basePath
 }
 
 export function getCurrentPath(basePath = '') {
diff --git a/types/raviger.d.ts b/types/raviger.d.ts
index 056c178..34f732f 100644
--- a/types/raviger.d.ts
+++ b/types/raviger.d.ts
@@ -22,6 +22,11 @@ export interface LinkProps
   href: string
 }
 export const Link: React.FC<LinkProps>
+export interface ActiveLinkProps extends LinkProps {
+  activeClass?: string
+  exactActiveClass?: string
+}
+export const ActiveLink: React.FC<ActiveLinkProps>
 
 export function navigate(url: string, replace?: boolean): void
 export function navigate(


Test patch:
diff --git a/test/Link.spec.js b/test/Link.spec.js
new file mode 100644
index 0000000..4858d5d
--- /dev/null
+++ b/test/Link.spec.js
@@ -0,0 +1,69 @@
+import React from 'react'
+import { render, act, fireEvent } from '@testing-library/react'
+import { Link, ActiveLink, navigate } from '../src/main.js'
+
+// this is just a little hack to silence a warning that we'll get until we
+// upgrade to 16.9: https://github.com/facebook/react/pull/14853
+const originalError = console.error
+beforeAll(() => {
+  console.error = (...args) => {
+    if (/Warning.*not wrapped in act/.test(args[0])) {
+      return
+    }
+    originalError.call(console, ...args)
+  }
+})
+
+afterAll(() => {
+  act(() => navigate('/'))
+  console.error = originalError
+})
+
+describe('Link', () => {
+  test('renders', async () => {
+    const { getByTestId } = render(
+      <Link href="/foo" className="base" data-testid="link">
+        go to foo
+      </Link>
+    )
+
+    act(() => navigate('/'))
+    expect(getByTestId('link')).toHaveTextContent('go to foo')
+  })
+  test('navigates to href', async () => {
+    act(() => navigate('/'))
+    const { getByTestId } = render(
+      <Link href="/foo" className="base" data-testid="link">
+        go to foo
+      </Link>
+    )
+    act(() => void fireEvent.click(getByTestId('link')))
+    expect(document.location.pathname).toEqual('/foo')
+  })
+})
+describe('ActiveLink', () => {
+  test('adds active class when active', async () => {
+    const { getByTestId } = render(
+      <ActiveLink
+        href="/foo"
+        className="base"
+        activeClass="extra"
+        exactActiveClass="double"
+        data-testid="link"
+      >
+        go to foo
+      </ActiveLink>
+    )
+
+    act(() => navigate('/'))
+    expect(getByTestId('link')).toHaveClass('base')
+
+    act(() => navigate('/foo'))
+    expect(getByTestId('link')).toHaveClass('extra')
+    expect(getByTestId('link')).toHaveClass('double')
+
+    act(() => navigate('/foo/bar'))
+    expect(getByTestId('link')).toHaveClass('extra')
+    expect(getByTestId('link')).not.toHaveClass('double')
+  })
+})
diff --git a/test/querystring.spec.js b/test/querystring.spec.js
new file mode 100644
index 0000000..3af620f
--- /dev/null
+++ b/test/querystring.spec.js
@@ -0,0 +1,17 @@
+import React from 'react'
+import { render, act } from '@testing-library/react'
+import { navigate, useQueryParams } from '../src/main.js'
+
+describe('useQueryParams', () => {
+  test('parses query', async () => {
+    function Route() {
+      let [query] = useQueryParams()
+      return <span data-testid="label">{JSON.stringify(query)}</span>
+    }
+    act(() => navigate('/about', { foo: 'bar' }))
+    const { getByTestId } = render(<Route />)
+    expect(getByTestId('label')).toHaveTextContent(
+      JSON.stringify({ foo: 'bar' })
+    )
+  })
+})