From 9706e126eafd1e6cb00f599d68b1d58b49699eca Mon Sep 17 00:00:00 2001
From: Evan Sutherland <stackoverfloweth@gmail.com>
Date: Sun, 26 Jan 2025 22:12:14 -0600
Subject: [PATCH] ensure checking for duplicate params is extended to hash

---
 src/services/combineHash.spec.ts          | 32 ++++++++++
 src/services/createExternalRoute.spec.ts  | 75 +++++++++++++++++++++++
 src/services/createExternalRoute.ts       |  2 +-
 src/services/createRoute.spec.ts          | 18 +++++-
 src/services/createRoute.ts               |  2 +-
 src/types/props.ts                        |  4 +-
 src/utilities/checkDuplicateNames.spec.ts |  2 +-
 7 files changed, 129 insertions(+), 6 deletions(-)
 create mode 100644 src/services/combineHash.spec.ts
 create mode 100644 src/services/createExternalRoute.spec.ts

diff --git a/src/services/combineHash.spec.ts b/src/services/combineHash.spec.ts
new file mode 100644
index 00000000..91e21807
--- /dev/null
+++ b/src/services/combineHash.spec.ts
@@ -0,0 +1,32 @@
+import { expect, test } from 'vitest'
+import { DuplicateParamsError } from '@/errors/duplicateParamsError'
+import { combineHash } from '@/services/combineHash'
+import { withParams } from '@/services/withParams'
+
+test('given 2 hash, returns new Hash joined together', () => {
+  const aHash = withParams('/foo', {})
+  const bHash = withParams('/bar', {})
+
+  const response = combineHash(aHash, bHash)
+
+  expect(response.value).toBe('/foo/bar')
+})
+
+test('given 2 hash with params, returns new Hash joined together with params', () => {
+  const aHash = withParams('/[foz]', { foz: String })
+  const bHash = withParams('/[?baz]', { baz: Number })
+
+  const response = combineHash(aHash, bHash)
+
+  expect(response.value).toBe('/[foz]/[?baz]')
+  expect(Object.keys(response.params)).toMatchObject(['foz', '?baz'])
+})
+
+test('given 2 hash with params that include duplicates, throws DuplicateParamsError', () => {
+  const aHash = withParams('/[foz]', { foz: String })
+  const bHash = withParams('/[foz]', { foz: String })
+
+  const action: () => void = () => combineHash(aHash, bHash)
+
+  expect(action).toThrow(DuplicateParamsError)
+})
diff --git a/src/services/createExternalRoute.spec.ts b/src/services/createExternalRoute.spec.ts
new file mode 100644
index 00000000..c9fa78e1
--- /dev/null
+++ b/src/services/createExternalRoute.spec.ts
@@ -0,0 +1,75 @@
+import { expect, test } from 'vitest'
+import { createExternalRoute } from '@/services/createExternalRoute'
+import { DuplicateParamsError } from '@/errors/duplicateParamsError'
+import { withParams } from '@/services/withParams'
+
+test('given parent, path is combined', () => {
+  const parent = createExternalRoute({
+    host: 'https://kitbag.dev',
+    path: '/parent',
+  })
+
+  const child = createExternalRoute({
+    parent: parent,
+    path: withParams('/child/[id]', { id: Number }),
+  })
+
+  expect(child.path).toMatchObject({
+    value: '/parent/child/[id]',
+    params: {
+      id: Number,
+    },
+  })
+})
+
+test('given parent, query is combined', () => {
+  const parent = createExternalRoute({
+    host: 'https://kitbag.dev',
+    query: 'static=123',
+  })
+
+  const child = createExternalRoute({
+    parent: parent,
+    query: withParams('sort=[sort]', { sort: Boolean }),
+  })
+
+  expect(child.query).toMatchObject({
+    value: 'static=123&sort=[sort]',
+    params: {
+      sort: Boolean,
+    },
+  })
+})
+
+test('given parent and child without meta, meta matches parent', () => {
+  const parent = createExternalRoute({
+    host: 'https://kitbag.dev',
+    meta: {
+      foo: 123,
+    },
+  })
+
+  const child = createExternalRoute({
+    parent: parent,
+  })
+
+  expect(child.meta).toMatchObject({
+    foo: 123,
+  })
+})
+
+test.each([
+  ['https://[foo].dev', '/[foo]', 'foo=[zoo]', '[bar]'],
+  ['https://[zoo].dev', '/[foo]', 'foo=[bar]', '[foo]'],
+  ['https://[zoo].dev', '/[bar]', 'foo=[foo]', '[foo]'],
+  ['https://[zoo].dev', '/[bar]', 'foo=[foo]', '[foo]'],
+])('given duplicate params across different parts of the route, throws DuplicateParamsError', (host, path, query, hash) => {
+  const action: () => void = () => createExternalRoute({
+    host,
+    path,
+    query,
+    hash,
+  })
+
+  expect(action).toThrow(DuplicateParamsError)
+})
diff --git a/src/services/createExternalRoute.ts b/src/services/createExternalRoute.ts
index 6c25d80c..0155c189 100644
--- a/src/services/createExternalRoute.ts
+++ b/src/services/createExternalRoute.ts
@@ -57,7 +57,7 @@ export function createExternalRoute(options: CreateRouteOptions): Route {
 
   const merged = isWithParent(options) ? combineRoutes(options.parent, route) : route
 
-  checkDuplicateParams(merged.path.params, merged.query.params, merged.host.params)
+  checkDuplicateParams(merged.path.params, merged.query.params, merged.host.params, merged.hash.params)
 
   return merged
 }
diff --git a/src/services/createRoute.spec.ts b/src/services/createRoute.spec.ts
index 63a704bc..9ce80379 100644
--- a/src/services/createRoute.spec.ts
+++ b/src/services/createRoute.spec.ts
@@ -1,7 +1,9 @@
 import { expect, test, vi } from 'vitest'
 import { createRoute } from '@/services/createRoute'
-import { createRouter, withParams } from '@/main'
 import { component } from '@/utilities/testHelpers'
+import { createRouter } from '@/services/createRouter'
+import { DuplicateParamsError } from '@/errors/duplicateParamsError'
+import { withParams } from '@/services/withParams'
 
 test('given parent, path is combined', () => {
   const parent = createRoute({
@@ -263,3 +265,17 @@ test('async parent props with multiple views are passed to child props', async (
 
   expect(spy).toHaveBeenCalledWith({ value1: 123, value2: 456 })
 })
+
+test.each([
+  ['/[foo]', 'foo=[foo]', '[bar]'],
+  ['/[foo]', 'foo=[bar]', '[foo]'],
+  ['/[bar]', 'foo=[foo]', '[foo]'],
+])('given duplicate params across different parts of the route, throws DuplicateParamsError', (path, query, hash) => {
+  const action: () => void = () => createRoute({
+    path,
+    query,
+    hash,
+  })
+
+  expect(action).toThrow(DuplicateParamsError)
+})
diff --git a/src/services/createRoute.ts b/src/services/createRoute.ts
index bd382c5f..28111b71 100644
--- a/src/services/createRoute.ts
+++ b/src/services/createRoute.ts
@@ -49,7 +49,7 @@ export function createRoute(options: CreateRouteOptions, props?: CreateRouteProp
 
   const merged = isWithParent(options) ? combineRoutes(options.parent, route) : route
 
-  checkDuplicateParams(merged.path.params, merged.query.params)
+  checkDuplicateParams(merged.path.params, merged.query.params, merged.hash.params)
 
   return merged
 }
diff --git a/src/types/props.ts b/src/types/props.ts
index ad141834..0cd47863 100644
--- a/src/types/props.ts
+++ b/src/types/props.ts
@@ -1,6 +1,6 @@
-import { Route } from '@/main'
 import { CallbackContext } from '@/services/createCallbackContext'
-import { PropsGetter } from './createRouteOptions'
+import { PropsGetter } from '@/types/createRouteOptions'
+import { Route } from '@/types/route'
 
 /**
  * Context provided to props callback functions
diff --git a/src/utilities/checkDuplicateNames.spec.ts b/src/utilities/checkDuplicateNames.spec.ts
index 1b171a83..dc9545b7 100644
--- a/src/utilities/checkDuplicateNames.spec.ts
+++ b/src/utilities/checkDuplicateNames.spec.ts
@@ -1,7 +1,7 @@
 import { expect, test } from 'vitest'
 import { DuplicateNamesError } from '@/errors/duplicateNamesError'
-import { createRoute } from '@/main'
 import { checkDuplicateNames } from '@/utilities/checkDuplicateNames'
+import { createRoute } from '@/services/createRoute'
 
 test('given a single array without duplicates, does nothing', () => {
   const routes = [