From c62447dc6c516d5ea689bde1861fac7fed69b5ef Mon Sep 17 00:00:00 2001 From: Evan Sutherland Date: Sun, 10 Nov 2024 09:58:20 -0600 Subject: [PATCH 1/5] simplified approach to ensuring Url type is returned from urlAssembly --- docs/api/types/RouterResolve.md | 2 +- src/components/routerLink.vue | 2 +- src/services/createRouterResolve.ts | 6 ++--- src/services/urlAssembly.spec.ts | 40 ++++++++++++++--------------- src/services/urlAssembly.ts | 12 ++++++--- src/services/withQuery.ts | 4 +++ 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/docs/api/types/RouterResolve.md b/docs/api/types/RouterResolve.md index 1f427814..4e324a8e 100644 --- a/docs/api/types/RouterResolve.md +++ b/docs/api/types/RouterResolve.md @@ -40,4 +40,4 @@ If source is `Url`, expected type for params is `never`. Else when source is `TS ## Returns -`string` +`Url` diff --git a/src/components/routerLink.vue b/src/components/routerLink.vue index 1410330c..27bb8446 100644 --- a/src/components/routerLink.vue +++ b/src/components/routerLink.vue @@ -12,7 +12,7 @@ import { RouterPushOptions } from '@/types/routerPush' import { Url, isUrl } from '@/types/url' - type ToCallback = (resolve: RegisteredRouter['resolve']) => string + type ToCallback = (resolve: RegisteredRouter['resolve']) => Url type RouterLinkProps = { /** diff --git a/src/services/createRouterResolve.ts b/src/services/createRouterResolve.ts index 2fc3f4ca..065fd95d 100644 --- a/src/services/createRouterResolve.ts +++ b/src/services/createRouterResolve.ts @@ -24,8 +24,8 @@ type RouterResolveArgs< export type RouterResolve< TRoutes extends Routes > = { - >(name: TSource, ...args: RouterResolveArgs): string, - (url: Url, options?: RouterResolveOptions): string, + >(name: TSource, ...args: RouterResolveArgs): Url, + (url: Url, options?: RouterResolveOptions): Url, } export function createRouterResolve(routes: TRoutes): RouterResolve { @@ -33,7 +33,7 @@ export function createRouterResolve(routes: TRoute source: Url | RoutesName, paramsOrOptions?: Record, maybeOptions?: RouterResolveOptions, - ): string => { + ): Url => { if (isUrl(source)) { const options: RouterPushOptions = paramsOrOptions ?? {} diff --git a/src/services/urlAssembly.spec.ts b/src/services/urlAssembly.spec.ts index 2920835b..b5387eaa 100644 --- a/src/services/urlAssembly.spec.ts +++ b/src/services/urlAssembly.spec.ts @@ -75,7 +75,7 @@ describe('path params', () => { test.each([ ['/simple/[simple]'], [path('/simple/[simple]', { simple: String })], - ])('given route with required string param NOT provided, throws error', (path) => { + ])('given route with required string param NOT provided, throws InvalidRouteParamValueError', (path) => { const route = createRoute({ name: 'simple', path, @@ -202,7 +202,7 @@ describe('query params', () => { test.each([ ['simple=[simple]'], [query('simple=[simple]', { simple: String })], - ])('given route with required string param NOT provided, throws error', (query) => { + ])('given route with required string param NOT provided, throws InvalidRouteParamValueError', (query) => { const route = createRoute({ name: 'simple', path: '/', @@ -265,8 +265,8 @@ describe('static query', () => { describe('host params', () => { test.each([ - ['kitbag.dev'], - [host('kitbag.dev', {})], + ['https://kitbag.dev'], + [host('https://kitbag.dev', {})], ])('given simple route with string host and without params, returns route host', (host) => { const route = createExternalRoute({ name: 'simple', @@ -276,13 +276,13 @@ describe('host params', () => { const url = assembleUrl(route) - expect(url).toBe('kitbag.dev/') + expect(url).toBe('https://kitbag.dev/') }) test.each([ - ['[?subdomain]kitbag.dev'], - [host('[?subdomain]kitbag.dev', { subdomain: String })], - [host('[?subdomain]kitbag.dev', { subdomain: withDefault(String, 'abc') })], + ['https://[?subdomain]kitbag.dev'], + [host('https://[?subdomain]kitbag.dev', { subdomain: String })], + [host('https://[?subdomain]kitbag.dev', { subdomain: withDefault(String, 'abc') })], ])('given route with optional param NOT provided, leaves entire key off', (host) => { const route = createExternalRoute({ name: 'simple', @@ -292,12 +292,12 @@ describe('host params', () => { const url = assembleUrl(route) - expect(url).toBe('kitbag.dev/') + expect(url).toBe('https://kitbag.dev/') }) test.each([ - ['[?subdomain]kitbag.dev'], - [host('[?subdomain]kitbag.dev', { subdomain: String })], + ['https://[?subdomain]kitbag.dev'], + [host('https://[?subdomain]kitbag.dev', { subdomain: String })], ])('given route with optional string param provided, returns route Host with string with values interpolated', (host) => { const route = createExternalRoute({ name: 'simple', @@ -309,27 +309,27 @@ describe('host params', () => { params: { subdomain: 'ABC.' }, }) - expect(url).toBe('ABC.kitbag.dev/') + expect(url).toBe('https://ABC.kitbag.dev/') }) test('given route with default string param provided, returns route Host with string with values interpolated', () => { const route = createExternalRoute({ name: 'simple', path: '/', - host: host('[?subdomain]kitbag.dev', { subdomain: withDefault(String, 'abc.') }), + host: host('https://[?subdomain]kitbag.dev', { subdomain: withDefault(String, 'abc.') }), }) const url = assembleUrl(route, { params: { subdomain: 'DEF.' }, }) - expect(url).toBe('DEF.kitbag.dev/') + expect(url).toBe('https://DEF.kitbag.dev/') }) test.each([ - ['[subdomain]kitbag.dev'], - [host('[subdomain]kitbag.dev', { subdomain: String })], - ])('given route with required string param NOT provided, throws error', (host) => { + ['https://[subdomain]kitbag.dev'], + [host('https://[subdomain]kitbag.dev', { subdomain: String })], + ])('given route with required string param NOT provided, throws InvalidRouteParamValueError', (host) => { const route = createExternalRoute({ name: 'simple', path: '/', @@ -340,8 +340,8 @@ describe('host params', () => { }) test.each([ - ['[subdomain]kitbag.dev'], - [host('[subdomain]kitbag.dev', { subdomain: String })], + ['https://[subdomain]kitbag.dev'], + [host('https://[subdomain]kitbag.dev', { subdomain: String })], ])('given route with required string param provided, returns route Host with string with values interpolated', (host) => { const route = createExternalRoute({ name: 'simple', @@ -353,7 +353,7 @@ describe('host params', () => { params: { subdomain: 'ABC.' }, }) - expect(url).toBe('ABC.kitbag.dev/') + expect(url).toBe('https://ABC.kitbag.dev/') }) }) diff --git a/src/services/urlAssembly.ts b/src/services/urlAssembly.ts index d7f4670f..acf4aca0 100644 --- a/src/services/urlAssembly.ts +++ b/src/services/urlAssembly.ts @@ -8,6 +8,7 @@ import { paramEnd, paramStart } from '@/types/params' import { Path } from '@/types/path' import { Query } from '@/types/query' import { Route } from '@/types/route' +import { isUrl, Url } from '@/types/url' type AssembleUrlOptions = { params?: Record, @@ -15,17 +16,22 @@ type AssembleUrlOptions = { hash?: string, } -export function assembleUrl(route: Route, options: AssembleUrlOptions = {}): string { +export function assembleUrl(route: Route, options: AssembleUrlOptions = {}): Url { const { params: paramValues = {}, query: queryValues } = options const hostWithParamsSet = assembleHostParamValues(route.host, paramValues) const pathWithParamsSet = assemblePathParamValues(route.path, paramValues) const queryWithParamsSet = assembleQueryParamValues(route.query, paramValues) - const url = withQuery(`${hostWithParamsSet}${pathWithParamsSet}`, queryWithParamsSet, queryValues) + const hostPathAndQuery = withQuery(`${hostWithParamsSet}${pathWithParamsSet}`, queryWithParamsSet, queryValues) const hash = createHash(route.hash.value ?? options.hash).toString() + const url = `${hostPathAndQuery}${hash}` - return `${url}${hash}` + if (isUrl(url)) { + return url + } + + return `/${url}` } function assembleHostParamValues(host: Host, paramValues: Record): string { diff --git a/src/services/withQuery.ts b/src/services/withQuery.ts index c996500c..3559bc18 100644 --- a/src/services/withQuery.ts +++ b/src/services/withQuery.ts @@ -1,5 +1,9 @@ +import { Url } from '@/types/url' + export type QueryRecord = Record +export function withQuery(url: Url, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url +export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): string export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): string { return queries.reduce((value, query) => { if (!query) { From 7a2e45b459e360e45336683a26c4346f2b8d29af Mon Sep 17 00:00:00 2001 From: Evan Sutherland Date: Sun, 10 Nov 2024 14:33:09 -0600 Subject: [PATCH 2/5] simplify url parts, better logic for joining url parts in urlAssembly --- src/services/combinePath.spec.ts | 4 +- src/services/combinePath.ts | 1 - src/services/combineQuery.spec.ts | 4 +- src/services/combineQuery.ts | 1 - src/services/createRouterResolve.spec.ts | 4 +- src/services/hash.ts | 16 +------- src/services/host.ts | 1 - src/services/insertBaseRoute.spec.ts | 2 +- src/services/paramValidation.ts | 2 +- src/services/path.ts | 1 - src/services/query.ts | 1 - src/services/routeMatchRules.ts | 4 +- src/services/routeMatchScore.ts | 10 ++--- src/services/routeRegex.spec.ts | 2 +- src/services/routeRegex.ts | 4 +- src/services/urlAssembly.spec.ts | 47 ++++++++++++++++++++++++ src/services/urlAssembly.ts | 29 +++++++++------ src/services/withHash.spec.ts | 25 +++++++++++++ src/services/withHash.ts | 12 ++++++ src/services/withHost.spec.ts | 26 +++++++++++++ src/services/withHost.ts | 19 ++++++++++ src/services/withPath.spec.ts | 25 +++++++++++++ src/services/withPath.ts | 13 +++++++ src/types/hash.ts | 2 - src/types/host.ts | 1 - src/types/path.ts | 1 - src/types/query.ts | 1 - 27 files changed, 204 insertions(+), 54 deletions(-) create mode 100644 src/services/withHash.spec.ts create mode 100644 src/services/withHash.ts create mode 100644 src/services/withHost.spec.ts create mode 100644 src/services/withHost.ts create mode 100644 src/services/withPath.spec.ts create mode 100644 src/services/withPath.ts diff --git a/src/services/combinePath.spec.ts b/src/services/combinePath.spec.ts index 955f8b61..99e858e2 100644 --- a/src/services/combinePath.spec.ts +++ b/src/services/combinePath.spec.ts @@ -9,7 +9,7 @@ test('given 2 paths, returns new Path joined together', () => { const response = combinePath(aPath, bPath) - expect(response.toString()).toBe('/foo/bar') + expect(response.value).toBe('/foo/bar') }) test('given 2 paths with params, returns new Path joined together with params', () => { @@ -18,7 +18,7 @@ test('given 2 paths with params, returns new Path joined together with params', const response = combinePath(aPath, bPath) - expect(response.toString()).toBe('/[foz]/[?baz]') + expect(response.value).toBe('/[foz]/[?baz]') expect(Object.keys(response.params)).toMatchObject(['foz', '?baz']) }) diff --git a/src/services/combinePath.ts b/src/services/combinePath.ts index a43e5461..a7c7f63f 100644 --- a/src/services/combinePath.ts +++ b/src/services/combinePath.ts @@ -22,6 +22,5 @@ export function combinePath(parentPath: Path, childPath: Path): Path { return { value: newPathString, params: { ...parentPath.params, ...childPath.params }, - toString: () => newPathString, } } diff --git a/src/services/combineQuery.spec.ts b/src/services/combineQuery.spec.ts index d4b65361..baf01f04 100644 --- a/src/services/combineQuery.spec.ts +++ b/src/services/combineQuery.spec.ts @@ -9,7 +9,7 @@ test('given 2 queries, returns new Query joined together', () => { const response = combineQuery(aQuery, bQuery) - expect(response.toString()).toBe('foo=ABC&bar=123') + expect(response.value).toBe('foo=ABC&bar=123') }) test('given 2 queries with params, returns new Query joined together with params', () => { @@ -18,7 +18,7 @@ test('given 2 queries with params, returns new Query joined together with params const response = combineQuery(aQuery, bQuery) - expect(response.toString()).toBe('foo=[foz]&bar=[?baz]') + expect(response.value).toBe('foo=[foz]&bar=[?baz]') expect(Object.keys(response.params)).toMatchObject(['foz', '?baz']) }) diff --git a/src/services/combineQuery.ts b/src/services/combineQuery.ts index bac84f62..ee29f6e7 100644 --- a/src/services/combineQuery.ts +++ b/src/services/combineQuery.ts @@ -31,6 +31,5 @@ export function combineQuery(parentQuery: Query, childQuery: Query): Query { return { value: newQueryString, params: { ...parentQuery.params, ...childQuery.params }, - toString: () => newQueryString, } } diff --git a/src/services/createRouterResolve.spec.ts b/src/services/createRouterResolve.spec.ts index d736ea3c..1405487c 100644 --- a/src/services/createRouterResolve.spec.ts +++ b/src/services/createRouterResolve.spec.ts @@ -86,5 +86,7 @@ test('when given an external route with params in host, interpolates param value test('given a route with hash, interpolates hash value', () => { const resolve = createRouterResolve(routes) - expect(resolve('parentA', { paramA: 'bar' }, { hash: 'foo' })).toBe('/parentA/bar#foo') + const url = resolve('parentA', { paramA: 'bar' }, { hash: 'foo' }) + + expect(url).toBe('/parentA/bar#foo') }) diff --git a/src/services/hash.ts b/src/services/hash.ts index 4f1dfa1e..d6838e3f 100644 --- a/src/services/hash.ts +++ b/src/services/hash.ts @@ -3,23 +3,9 @@ import { stringHasValue } from '@/utilities/guards' export function hash(hash?: THash): Hash export function hash(hash?: string): Hash { - const value = !stringHasValue(hash) ? undefined : hash.replace(/^#/, '') - - function hasValue(): boolean { - return value !== undefined - } - - function toString(): string { - if (hasValue()) { - return `#${hash}` - } - - return '' - } + const value = !stringHasValue(hash) ? undefined : hash.replace(/^#*/, '') return { value, - hasValue, - toString, } } diff --git a/src/services/host.ts b/src/services/host.ts index 1cb43837..6266ced7 100644 --- a/src/services/host.ts +++ b/src/services/host.ts @@ -29,6 +29,5 @@ export function host(value: string, params: Record): return { value, params: getParamsForString(value, params), - toString: () => value, } } diff --git a/src/services/insertBaseRoute.spec.ts b/src/services/insertBaseRoute.spec.ts index b786cdb4..3d450ea3 100644 --- a/src/services/insertBaseRoute.spec.ts +++ b/src/services/insertBaseRoute.spec.ts @@ -36,5 +36,5 @@ test('given value for base, returns routes with base prefixed', () => { const response = insertBaseRoute(routes, base) - expect(response.every((route) => route.path.toString().startsWith('/kitbag'))).toBe(true) + expect(response.every((route) => route.path.value.startsWith('/kitbag'))).toBe(true) }) diff --git a/src/services/paramValidation.ts b/src/services/paramValidation.ts index 7e7cf8f4..bc0905ca 100644 --- a/src/services/paramValidation.ts +++ b/src/services/paramValidation.ts @@ -32,7 +32,7 @@ function getPathParams(path: Path, url: string): Record { for (const [key, param] of Object.entries(path.params)) { const isOptional = key.startsWith('?') const paramName = isOptional ? key.slice(1) : key - const stringValue = getParamValueFromUrl(decodedValueFromUrl, path.toString(), key) + const stringValue = getParamValueFromUrl(decodedValueFromUrl, path.value, key) const paramValue = getParamValue(stringValue, param, isOptional) values[paramName] = paramValue diff --git a/src/services/path.ts b/src/services/path.ts index aaca7104..4253336f 100644 --- a/src/services/path.ts +++ b/src/services/path.ts @@ -29,6 +29,5 @@ export function path(value: string, params: Record): return { value, params: getParamsForString(value, params), - toString: () => value, } } diff --git a/src/services/query.ts b/src/services/query.ts index 1ffaca18..24ab51f8 100644 --- a/src/services/query.ts +++ b/src/services/query.ts @@ -29,6 +29,5 @@ export function query(value: string, params: Record): return { value, params: getParamsForString(value, params), - toString: () => value, } } diff --git a/src/services/routeMatchRules.ts b/src/services/routeMatchRules.ts index a605ef5e..5acd5610 100644 --- a/src/services/routeMatchRules.ts +++ b/src/services/routeMatchRules.ts @@ -22,7 +22,7 @@ export const routeQueryMatches: RouteMatchRule = (route, url) => { export const routeHashMatches: RouteMatchRule = (route, url) => { const { hash } = createMaybeRelativeUrl(url) - const value = route.hash.toString() + const { value } = route.hash - return !route.hash.hasValue() || value.toLowerCase() === hash.toLowerCase() + return value === undefined || `#${value.toLowerCase()}` === hash.toLowerCase() } diff --git a/src/services/routeMatchScore.ts b/src/services/routeMatchScore.ts index 0f677d92..f866f6a4 100644 --- a/src/services/routeMatchScore.ts +++ b/src/services/routeMatchScore.ts @@ -1,6 +1,7 @@ import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' import { getParamValueFromUrl } from '@/services/paramsFinder' import { Route } from '@/types/route' +import { routeHashMatches } from './routeMatchRules' type RouteSortMethod = (aRoute: Route, bRoute: Route) => number @@ -29,11 +30,10 @@ export function getRouteScoreSortMethod(url: string): RouteSortMethod { return sortAfter } - const { hash } = createMaybeRelativeUrl(url) - if (aRoute.hash.toString() === hash) { + if (routeHashMatches(aRoute, url)) { return sortBefore } - if (bRoute.hash.toString() === hash) { + if (routeHashMatches(bRoute, url)) { return sortAfter } @@ -46,13 +46,13 @@ export function countExpectedPathParams(route: Route, actualPath: string): numbe .filter((key) => key.startsWith('?')) .map((key) => key) - const missing = optionalParams.filter((expected) => getParamValueFromUrl(actualPath, route.path.toString(), expected) === undefined) + const missing = optionalParams.filter((expected) => getParamValueFromUrl(actualPath, route.path.value, expected) === undefined) return optionalParams.length - missing.length } export function countExpectedQueryParams(route: Route, actualQuery: URLSearchParams): number { - const expectedQuery = new URLSearchParams(route.query.toString()) + const expectedQuery = new URLSearchParams(route.query.value) const expectedQueryKeys = Array.from(expectedQuery.keys()) const missing = expectedQueryKeys.filter((expected) => !actualQuery.has(expected)) diff --git a/src/services/routeRegex.spec.ts b/src/services/routeRegex.spec.ts index 13205df4..41741d4f 100644 --- a/src/services/routeRegex.spec.ts +++ b/src/services/routeRegex.spec.ts @@ -13,7 +13,7 @@ describe('generateRoutePathRegexPattern', () => { const result = generateRoutePathRegexPattern(route) - const expected = new RegExp(`^${route.path}$`, 'i') + const expected = new RegExp(`^${route.path.value}$`, 'i') expect(result.toString()).toBe(expected.toString()) }) diff --git a/src/services/routeRegex.ts b/src/services/routeRegex.ts index 05544573..aa7c633c 100644 --- a/src/services/routeRegex.ts +++ b/src/services/routeRegex.ts @@ -37,13 +37,13 @@ export function splitByMatches(string: string, regexp: RegExp): string[] { } export function generateRoutePathRegexPattern(route: Route): RegExp { - const pathRegex = replaceParamSyntaxWithCatchAllsAndEscapeRest(route.path.toString()) + const pathRegex = replaceParamSyntaxWithCatchAllsAndEscapeRest(route.path.value) return new RegExp(`^${pathRegex}$`, 'i') } export function generateRouteQueryRegexPatterns(route: Route): RegExp[] { - const queryParams = new URLSearchParams(route.query.toString()) + const queryParams = new URLSearchParams(route.query.value) return Array .from(queryParams.entries()) diff --git a/src/services/urlAssembly.spec.ts b/src/services/urlAssembly.spec.ts index b5387eaa..71a14f32 100644 --- a/src/services/urlAssembly.spec.ts +++ b/src/services/urlAssembly.spec.ts @@ -367,3 +367,50 @@ test('given route with hash, returns url with hash value interpolated', () => { expect(url).toBe('/#foo') }) + +test('given route without host that does not start with a forward slash, returns url with forward slash', () => { + const route = createRoute({ + name: 'invalid-relative-path', + path: 'foo', + }) + + const url = assembleUrl(route) + + expect(url).toBe('/foo') +}) + +test('given route with host that does not start with a protocol, returns url with protocol', () => { + const route = createExternalRoute({ + name: 'invalid-host-protocol', + path: '/foo', + host: 'kitbag.dev', + }) + + const url = assembleUrl(route) + + expect(url).toBe('https://kitbag.dev/foo') +}) + +test('given route with host and path without forward slash, returns forward slash after host', () => { + const route = createExternalRoute({ + name: 'missing-delimiter-after-host', + path: 'foo', + host: 'https://kitbag.dev', + }) + + const url = assembleUrl(route) + + expect(url).toBe('https://kitbag.dev/foo') +}) + +test('given route with host and path with excess forward slashes, returns forward slash after host', () => { + const route = createExternalRoute({ + name: 'extra-delimiter-after-host', + path: '/foo', + host: 'https://kitbag.dev/', + }) + + const url = assembleUrl(route) + + expect(url).toBe('https://kitbag.dev/foo') +}) diff --git a/src/services/urlAssembly.ts b/src/services/urlAssembly.ts index acf4aca0..58b199ce 100644 --- a/src/services/urlAssembly.ts +++ b/src/services/urlAssembly.ts @@ -8,7 +8,10 @@ import { paramEnd, paramStart } from '@/types/params' import { Path } from '@/types/path' import { Query } from '@/types/query' import { Route } from '@/types/route' -import { isUrl, Url } from '@/types/url' +import { Url } from '@/types/url' +import { withHost } from './withHost' +import { withHash } from './withHash' +import { withPath } from './withPath' type AssembleUrlOptions = { params?: Record, @@ -19,23 +22,25 @@ type AssembleUrlOptions = { export function assembleUrl(route: Route, options: AssembleUrlOptions = {}): Url { const { params: paramValues = {}, query: queryValues } = options - const hostWithParamsSet = assembleHostParamValues(route.host, paramValues) const pathWithParamsSet = assemblePathParamValues(route.path, paramValues) const queryWithParamsSet = assembleQueryParamValues(route.query, paramValues) + const hostWithParamsSet = assembleHostParamValues(route.host, paramValues) + const hash = createHash(route.hash.value ?? options.hash) - const hostPathAndQuery = withQuery(`${hostWithParamsSet}${pathWithParamsSet}`, queryWithParamsSet, queryValues) - const hash = createHash(route.hash.value ?? options.hash).toString() - const url = `${hostPathAndQuery}${hash}` + type UrlAssemblyFunction = (url: Url) => Url - if (isUrl(url)) { - return url - } + const assembly: UrlAssemblyFunction[] = [ + (url) => withPath(url, pathWithParamsSet), + (url) => withQuery(url, queryWithParamsSet, queryValues), + (url) => withHost(url, hostWithParamsSet), + (url) => withHash(url, hash), + ] - return `/${url}` + return assembly.reduce((url, join) => join(url), '/') } function assembleHostParamValues(host: Host, paramValues: Record): string { - const value = host.toString() + const { value } = host return Object.entries(host.params).reduce((url, [name, param]) => { const paramName = getParamName(`${paramStart}${name}${paramEnd}`) @@ -49,7 +54,7 @@ function assembleHostParamValues(host: Host, paramValues: Record): string { - const value = path.toString() + const { value } = path return Object.entries(path.params).reduce((url, [name, param]) => { const paramName = getParamName(`${paramStart}${name}${paramEnd}`) @@ -63,7 +68,7 @@ function assemblePathParamValues(path: Path, paramValues: Record): QueryRecord { - const value = query.toString() + const { value } = query if (!value) { return {} diff --git a/src/services/withHash.spec.ts b/src/services/withHash.spec.ts new file mode 100644 index 00000000..ae4e20a9 --- /dev/null +++ b/src/services/withHash.spec.ts @@ -0,0 +1,25 @@ +import { expect, test } from 'vitest' +import { withHash } from '@/services/withHash' +import { hash as createHash } from './hash' + +test.each([undefined, ''])('given hash without value, returns url unmodified', (value) => { + const url = 'https://www.github.io' + + const response = withHash(url, { value }) + + expect(response).toBe(url) +}) + +test.each(['https://www.github.io', '/foo'])('given hash with value, returns url + # + hash', (url) => { + const hash = createHash('bar') + const response = withHash(url, hash) + + expect(response).toBe(`${url}#bar`) +}) + +test.each(['https://www.github.io', '/foo'])('given hash with # + value, returns url + # + hash', (url) => { + const hash = createHash('#bar') + const response = withHash(url, hash) + + expect(response).toBe(`${url}#bar`) +}) diff --git a/src/services/withHash.ts b/src/services/withHash.ts new file mode 100644 index 00000000..5a1519b3 --- /dev/null +++ b/src/services/withHash.ts @@ -0,0 +1,12 @@ +import { Hash } from '@/types/hash' +import { Url } from '@/types/url' + +export function withHash(url: Url, hash: Hash): Url +export function withHash(url: string, hash: Hash): string +export function withHash(url: string, hash: Hash): string { + if (hash.value) { + return `${url}#${hash.value}` + } + + return url +} diff --git a/src/services/withHost.spec.ts b/src/services/withHost.spec.ts new file mode 100644 index 00000000..ed8936a1 --- /dev/null +++ b/src/services/withHost.spec.ts @@ -0,0 +1,26 @@ +import { expect, test } from 'vitest' +import { withHost } from '@/services/withHost' + +test.each([undefined, ''])('given empty host with a url, returns url unmodified', (host) => { + const url = '/foo' + + const response = withHost(url, host) + + expect(response).toBe(url) +}) + +test.each(['http://kitbag.dev', 'https://kitbag.dev'])('given host that satisfies Url type, returns host + url', (host) => { + const url = 'foo' + + const response = withHost(url, host) + + expect(response).toBe(`${host}/${url}`) +}) + +test.each(['kitbag.dev', 'www.google.com'])('given host that does not satisfy Url type, returns https:// + host + url', (host) => { + const url = 'foo' + + const response = withHost(url, host) + + expect(response).toBe(`https://${host}/${url}`) +}) diff --git a/src/services/withHost.ts b/src/services/withHost.ts new file mode 100644 index 00000000..b8dde9d9 --- /dev/null +++ b/src/services/withHost.ts @@ -0,0 +1,19 @@ +import { isUrl, Url } from '@/types/url' +import { stringHasValue } from '@/utilities' + +export function withHost(url: Url, host?: string): Url +export function withHost(url: string, host?: string): string +export function withHost(url: string, host?: string): string { + if (!stringHasValue(host)) { + return url + } + + const cleanPath = url.replace(/^\/*/, '') + const cleanHost = host.replace(/\/*$/, '') + + if (isUrl(cleanHost)) { + return `${cleanHost}/${cleanPath}` + } + + return `https://${cleanHost}/${cleanPath}` +} diff --git a/src/services/withPath.spec.ts b/src/services/withPath.spec.ts new file mode 100644 index 00000000..d233333e --- /dev/null +++ b/src/services/withPath.spec.ts @@ -0,0 +1,25 @@ +import { expect, test } from 'vitest' +import { withPath } from '@/services/withPath' + +test.each(['https://kitbag.dev', '/'])('given url that satisfies Url type, returns url + path', (url) => { + const path = 'foo' + + const response = withPath(url, path) + + expect(response).toBe(`${url}${path}`) +}) + +test.each(['kitbag.dev', 'bar'])('given url that does not satisfy Url type, returns / + url + path', (url) => { + const path = 'foo' + + const response = withPath(url, path) + + expect(response).toBe(`/${url}${path}`) +}) + +test.each(['/foo', '///foo'])('given path with 1+ forward slashes, removes leading slashes from path', (path) => { + const url = 'https://kitbag.dev/' + const response = withPath(url, path) + + expect(response).toBe(`${url}foo`) +}) diff --git a/src/services/withPath.ts b/src/services/withPath.ts new file mode 100644 index 00000000..931656ae --- /dev/null +++ b/src/services/withPath.ts @@ -0,0 +1,13 @@ +import { isUrl, Url } from '@/types/url' + +export function withPath(url: Url, path: string): Url +export function withPath(url: string, path: string): Url +export function withPath(url: string, path: string): Url { + const cleanPath = path.replace(/^\/*/, '') + + if (isUrl(url)) { + return `${url}${cleanPath}` + } + + return `/${url}${cleanPath}` +} diff --git a/src/types/hash.ts b/src/types/hash.ts index e30b8a2e..0c93c1b7 100644 --- a/src/types/hash.ts +++ b/src/types/hash.ts @@ -5,8 +5,6 @@ export type Hash< THash extends string | undefined = string | undefined > = { value: THash, - hasValue: () => boolean, - toString: () => string, } export type ToHash = T extends string ? Hash diff --git a/src/types/host.ts b/src/types/host.ts index 1cee0ade..8d973b84 100644 --- a/src/types/host.ts +++ b/src/types/host.ts @@ -25,7 +25,6 @@ export type Host< > = { value: THost, params: string extends THost ? Record : Identity>, - toString: () => string, } export type ToHost = T extends string diff --git a/src/types/path.ts b/src/types/path.ts index ca66398d..b0e1ce0d 100644 --- a/src/types/path.ts +++ b/src/types/path.ts @@ -25,7 +25,6 @@ export type Path< > = { value: TPath, params: string extends TPath ? Record : Identity>, - toString: () => string, } export type ToPath = T extends string ? Path diff --git a/src/types/query.ts b/src/types/query.ts index 71d6c07f..b49eff7c 100644 --- a/src/types/query.ts +++ b/src/types/query.ts @@ -25,7 +25,6 @@ export type Query< > = { value: TQuery, params: string extends TQuery ? Record : Identity>, - toString: () => string, } export type ToQuery = T extends string From 5cfcc90d77648b1e6ff85ed7fad8fb3b8adf721b Mon Sep 17 00:00:00 2001 From: Evan Sutherland Date: Thu, 14 Nov 2024 10:44:20 -0600 Subject: [PATCH 3/5] made with** utilities more robust given different shaped urls. updated maybeRelativeUrl service to also export a function that joins the parts to create a Url. --- src/services/createIsExternal.ts | 2 +- src/services/createRouter.ts | 2 +- src/services/getResolvedRouteForUrl.ts | 2 +- src/services/maybeRelativeUrl.spec.ts | 98 +++++++++++++++++++ ...aybeRelativeUrl.ts => maybeRelativeUrl.ts} | 20 ++++ src/services/paramValidation.ts | 2 +- src/services/routeMatchRules.ts | 2 +- src/services/routeMatchScore.ts | 2 +- src/services/urlAssembly.spec.ts | 6 +- src/services/urlAssembly.ts | 22 ++--- src/services/withHash.spec.ts | 12 +-- src/services/withHash.ts | 15 ++- src/services/withHost.ts | 2 +- src/services/withPath.spec.ts | 14 +-- src/services/withPath.ts | 18 ++-- src/services/withQuery.ts | 10 +- src/types/url.ts | 10 +- 17 files changed, 174 insertions(+), 65 deletions(-) create mode 100644 src/services/maybeRelativeUrl.spec.ts rename src/services/{createMaybeRelativeUrl.ts => maybeRelativeUrl.ts} (58%) diff --git a/src/services/createIsExternal.ts b/src/services/createIsExternal.ts index 2b3b3506..8dbd9114 100644 --- a/src/services/createIsExternal.ts +++ b/src/services/createIsExternal.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' +import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' export function createIsExternal(host: string | undefined): (url: string) => boolean { return (url: string) => { diff --git a/src/services/createRouter.ts b/src/services/createRouter.ts index aadde1d7..3299b3f2 100644 --- a/src/services/createRouter.ts +++ b/src/services/createRouter.ts @@ -5,7 +5,7 @@ import { routerRejectionKey } from '@/compositions/useRejection' import { routerInjectionKey } from '@/compositions/useRouter' import { createCurrentRoute } from '@/services/createCurrentRoute' import { createIsExternal } from '@/services/createIsExternal' -import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' +import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' import { createPropStore, propStoreKey } from '@/services/createPropStore' import { createRouterHistory } from '@/services/createRouterHistory' import { routeHookStoreKey, createRouterHooks } from '@/services/createRouterHooks' diff --git a/src/services/getResolvedRouteForUrl.ts b/src/services/getResolvedRouteForUrl.ts index cba4e3d2..04cd0975 100644 --- a/src/services/getResolvedRouteForUrl.ts +++ b/src/services/getResolvedRouteForUrl.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' +import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' import { createResolvedRouteQuery } from '@/services/createResolvedRouteQuery' import { getRouteParamValues, routeParamsAreValid } from '@/services/paramValidation' import { isNamedRoute, routePathMatches, routeQueryMatches, routeHashMatches } from '@/services/routeMatchRules' diff --git a/src/services/maybeRelativeUrl.spec.ts b/src/services/maybeRelativeUrl.spec.ts new file mode 100644 index 00000000..5855c818 --- /dev/null +++ b/src/services/maybeRelativeUrl.spec.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from 'vitest' +import { createMaybeRelativeUrl, maybeRelativeUrlToString } from '@/services/maybeRelativeUrl' + +describe('createMaybeRelativeUrl', () => { + test('given relative url, returns host and protocol undefined', () => { + const url = '/foo?bar=123' + + const parts = createMaybeRelativeUrl(url) + + expect(parts.host).toBe(undefined) + expect(parts.protocol).toBe(undefined) + }) + + test('given absolute url with path, returns everything up to pathname', () => { + const url = 'https://kitbag.dev/foo' + + const parts = createMaybeRelativeUrl(url) + + expect(parts.host).toBe('kitbag.dev') + expect(parts.protocol).toBe('https:') + expect(parts.pathname).toBe('/foo') + expect(parts.search).toBe('') + expect(parts.hash).toBe('') + }) + + test('given absolute url with path and query, returns everything up to search', () => { + const url = 'https://kitbag.dev/foo?bar=123' + + const parts = createMaybeRelativeUrl(url) + + expect(parts.host).toBe('kitbag.dev') + expect(parts.protocol).toBe('https:') + expect(parts.pathname).toBe('/foo') + expect(parts.search).toBe('?bar=123') + expect(parts.hash).toBe('') + }) + + test('given absolute url with path, query, and hash, returns everything', () => { + const url = 'https://kitbag.dev/foo?bar=123#zoo' + + const parts = createMaybeRelativeUrl(url) + + expect(parts.host).toBe('kitbag.dev') + expect(parts.protocol).toBe('https:') + expect(parts.pathname).toBe('/foo') + expect(parts.search).toBe('?bar=123') + expect(parts.hash).toBe('#zoo') + }) +}) + +describe('maybeRelativeUrlToString', () => { + test('given parts without host, protocol, or path, returns forward slash to satisfy Url', () => { + const url = maybeRelativeUrlToString({}) + + expect(url).toBe('/') + }) + + test('given parts without host, protocol, or valid path, returns forward slash to satisfy Url', () => { + const url = maybeRelativeUrlToString({ + pathname: 'foo', + }) + + expect(url).toBe('/foo') + }) + + test('given parts without host and protocol, returns url starting with forward slash', () => { + const parts = { + pathname: '/foo', + search: '?bar=123', + } + + const url = maybeRelativeUrlToString(parts) + + expect(url).toBe('/foo?bar=123') + }) + + test('given parts with searchParams, does nothing', () => { + const parts = { + pathname: '/foo', + searchParams: new URLSearchParams([['bar', '123']]), + } + + const url = maybeRelativeUrlToString(parts) + + expect(url).toBe('/foo') + }) + + test('given parts with host and protocol, returns value that satisfies Url', () => { + const parts = { + host: 'kitbag.dev', + protocol: 'https:', + } + + const url = maybeRelativeUrlToString(parts) + + expect(url).toBe('https://kitbag.dev') + }) +}) diff --git a/src/services/createMaybeRelativeUrl.ts b/src/services/maybeRelativeUrl.ts similarity index 58% rename from src/services/createMaybeRelativeUrl.ts rename to src/services/maybeRelativeUrl.ts index 8a72d32d..7f056c12 100644 --- a/src/services/createMaybeRelativeUrl.ts +++ b/src/services/maybeRelativeUrl.ts @@ -1,3 +1,6 @@ +import { asUrl, Url } from '@/types/url' +import { stringHasValue } from '@/utilities' + export type MaybeRelativeUrl = { protocol?: string, host?: string, @@ -7,6 +10,23 @@ export type MaybeRelativeUrl = { hash: string, } +export function maybeRelativeUrlToString({ protocol, host, pathname, search, hash }: Partial>): Url { + const protocolWithoutSlashesPattern = /^https?:$/ + const cleanProtocol = protocol && protocolWithoutSlashesPattern.test(protocol) ? `${protocol}//` : protocol + + const joined = [ + cleanProtocol, + host, + pathname, + search, + hash, + ] + .filter((part) => stringHasValue(part)) + .join('') + + return asUrl(joined) +} + export function createMaybeRelativeUrl(value: string): MaybeRelativeUrl { const isRelative = !value.startsWith('http') diff --git a/src/services/paramValidation.ts b/src/services/paramValidation.ts index bc0905ca..3f22b0d0 100644 --- a/src/services/paramValidation.ts +++ b/src/services/paramValidation.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' +import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' import { getParamValue } from '@/services/params' import { getParamValueFromUrl } from '@/services/paramsFinder' import { Path } from '@/types/path' diff --git a/src/services/routeMatchRules.ts b/src/services/routeMatchRules.ts index 5acd5610..050ab3da 100644 --- a/src/services/routeMatchRules.ts +++ b/src/services/routeMatchRules.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' +import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' import { generateRoutePathRegexPattern, generateRouteQueryRegexPatterns } from '@/services/routeRegex' import { RouteMatchRule } from '@/types/routeMatchRule' diff --git a/src/services/routeMatchScore.ts b/src/services/routeMatchScore.ts index f866f6a4..cd117833 100644 --- a/src/services/routeMatchScore.ts +++ b/src/services/routeMatchScore.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/createMaybeRelativeUrl' +import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' import { getParamValueFromUrl } from '@/services/paramsFinder' import { Route } from '@/types/route' import { routeHashMatches } from './routeMatchRules' diff --git a/src/services/urlAssembly.spec.ts b/src/services/urlAssembly.spec.ts index 71a14f32..a7497f38 100644 --- a/src/services/urlAssembly.spec.ts +++ b/src/services/urlAssembly.spec.ts @@ -309,7 +309,7 @@ describe('host params', () => { params: { subdomain: 'ABC.' }, }) - expect(url).toBe('https://ABC.kitbag.dev/') + expect(url).toBe('https://abc.kitbag.dev/') }) test('given route with default string param provided, returns route Host with string with values interpolated', () => { @@ -323,7 +323,7 @@ describe('host params', () => { params: { subdomain: 'DEF.' }, }) - expect(url).toBe('https://DEF.kitbag.dev/') + expect(url).toBe('https://def.kitbag.dev/') }) test.each([ @@ -353,7 +353,7 @@ describe('host params', () => { params: { subdomain: 'ABC.' }, }) - expect(url).toBe('https://ABC.kitbag.dev/') + expect(url).toBe('https://abc.kitbag.dev/') }) }) diff --git a/src/services/urlAssembly.ts b/src/services/urlAssembly.ts index 58b199ce..4f58167c 100644 --- a/src/services/urlAssembly.ts +++ b/src/services/urlAssembly.ts @@ -9,9 +9,9 @@ import { Path } from '@/types/path' import { Query } from '@/types/query' import { Route } from '@/types/route' import { Url } from '@/types/url' -import { withHost } from './withHost' -import { withHash } from './withHash' -import { withPath } from './withPath' +import { withHost } from '@/services/withHost' +import { withHash } from '@/services/withHash' +import { withPath } from '@/services/withPath' type AssembleUrlOptions = { params?: Record, @@ -33,15 +33,13 @@ export function assembleUrl(route: Route, options: AssembleUrlOptions = {}): Url (url) => withPath(url, pathWithParamsSet), (url) => withQuery(url, queryWithParamsSet, queryValues), (url) => withHost(url, hostWithParamsSet), - (url) => withHash(url, hash), + (url) => withHash(url, hash.value), ] return assembly.reduce((url, join) => join(url), '/') } function assembleHostParamValues(host: Host, paramValues: Record): string { - const { value } = host - return Object.entries(host.params).reduce((url, [name, param]) => { const paramName = getParamName(`${paramStart}${name}${paramEnd}`) @@ -50,12 +48,10 @@ function assembleHostParamValues(host: Host, paramValues: Record): string { - const { value } = path - return Object.entries(path.params).reduce((url, [name, param]) => { const paramName = getParamName(`${paramStart}${name}${paramEnd}`) @@ -64,17 +60,15 @@ function assemblePathParamValues(path: Path, paramValues: Record): QueryRecord { - const { value } = query - - if (!value) { + if (!query.value) { return {} } - const search = new URLSearchParams(value) + const search = new URLSearchParams(query.value) return Array.from(search.entries()).reduce((url, [key, value]) => { const paramName = getParamName(value) diff --git a/src/services/withHash.spec.ts b/src/services/withHash.spec.ts index ae4e20a9..a849fbf6 100644 --- a/src/services/withHash.spec.ts +++ b/src/services/withHash.spec.ts @@ -3,23 +3,23 @@ import { withHash } from '@/services/withHash' import { hash as createHash } from './hash' test.each([undefined, ''])('given hash without value, returns url unmodified', (value) => { - const url = 'https://www.github.io' + const url = 'https://www.github.io/' - const response = withHash(url, { value }) + const response = withHash(url, value) expect(response).toBe(url) }) -test.each(['https://www.github.io', '/foo'])('given hash with value, returns url + # + hash', (url) => { +test.each(['https://www.github.io/', '/foo'])('given hash with value, returns url + # + hash', (url) => { const hash = createHash('bar') - const response = withHash(url, hash) + const response = withHash(url, hash.value) expect(response).toBe(`${url}#bar`) }) -test.each(['https://www.github.io', '/foo'])('given hash with # + value, returns url + # + hash', (url) => { +test.each(['https://www.github.io/', '/foo'])('given hash with # + value, returns url + # + hash', (url) => { const hash = createHash('#bar') - const response = withHash(url, hash) + const response = withHash(url, hash.value) expect(response).toBe(`${url}#bar`) }) diff --git a/src/services/withHash.ts b/src/services/withHash.ts index 5a1519b3..6da27c46 100644 --- a/src/services/withHash.ts +++ b/src/services/withHash.ts @@ -1,12 +1,11 @@ -import { Hash } from '@/types/hash' import { Url } from '@/types/url' +import { createMaybeRelativeUrl, maybeRelativeUrlToString } from '@/services/maybeRelativeUrl' -export function withHash(url: Url, hash: Hash): Url -export function withHash(url: string, hash: Hash): string -export function withHash(url: string, hash: Hash): string { - if (hash.value) { - return `${url}#${hash.value}` - } +export function withHash(url: Url, hash?: string): Url +export function withHash(url: string, hash?: string): Url +export function withHash(url: string, hash?: string): Url { + const { hash: previousHash, ...parts } = createMaybeRelativeUrl(url) + const cleanHash = hash ? `#${hash}` : '' - return url + return maybeRelativeUrlToString({ hash: cleanHash, ...parts }) } diff --git a/src/services/withHost.ts b/src/services/withHost.ts index b8dde9d9..555ce89d 100644 --- a/src/services/withHost.ts +++ b/src/services/withHost.ts @@ -1,5 +1,5 @@ import { isUrl, Url } from '@/types/url' -import { stringHasValue } from '@/utilities' +import { stringHasValue } from '@/utilities/guards' export function withHost(url: Url, host?: string): Url export function withHost(url: string, host?: string): string diff --git a/src/services/withPath.spec.ts b/src/services/withPath.spec.ts index d233333e..d208f5b9 100644 --- a/src/services/withPath.spec.ts +++ b/src/services/withPath.spec.ts @@ -1,7 +1,7 @@ import { expect, test } from 'vitest' import { withPath } from '@/services/withPath' -test.each(['https://kitbag.dev', '/'])('given url that satisfies Url type, returns url + path', (url) => { +test.each(['https://kitbag.dev/', '/'])('given url that satisfies Url type, returns url + path', (url) => { const path = 'foo' const response = withPath(url, path) @@ -9,17 +9,9 @@ test.each(['https://kitbag.dev', '/'])('given url that satisfies Url type, retur expect(response).toBe(`${url}${path}`) }) -test.each(['kitbag.dev', 'bar'])('given url that does not satisfy Url type, returns / + url + path', (url) => { - const path = 'foo' - - const response = withPath(url, path) - - expect(response).toBe(`/${url}${path}`) -}) - test.each(['/foo', '///foo'])('given path with 1+ forward slashes, removes leading slashes from path', (path) => { - const url = 'https://kitbag.dev/' + const url = 'https://kitbag.dev' const response = withPath(url, path) - expect(response).toBe(`${url}foo`) + expect(response).toBe(`${url}/foo`) }) diff --git a/src/services/withPath.ts b/src/services/withPath.ts index 931656ae..edac150e 100644 --- a/src/services/withPath.ts +++ b/src/services/withPath.ts @@ -1,13 +1,11 @@ -import { isUrl, Url } from '@/types/url' +import { Url } from '@/types/url' +import { createMaybeRelativeUrl, maybeRelativeUrlToString } from '@/services/maybeRelativeUrl' -export function withPath(url: Url, path: string): Url -export function withPath(url: string, path: string): Url -export function withPath(url: string, path: string): Url { - const cleanPath = path.replace(/^\/*/, '') +export function withPath(url: Url, path?: string): Url +export function withPath(url: string, path?: string): Url +export function withPath(url: string, path?: string): Url { + const { pathname, ...parts } = createMaybeRelativeUrl(url) + const cleanPath = path?.replace(/^\/*/, '/') ?? '' - if (isUrl(url)) { - return `${url}${cleanPath}` - } - - return `/${url}${cleanPath}` + return maybeRelativeUrlToString({ pathname: cleanPath, ...parts }) } diff --git a/src/services/withQuery.ts b/src/services/withQuery.ts index 3559bc18..4d5e3bd9 100644 --- a/src/services/withQuery.ts +++ b/src/services/withQuery.ts @@ -1,11 +1,11 @@ -import { Url } from '@/types/url' +import { asUrl, Url } from '@/types/url' export type QueryRecord = Record export function withQuery(url: Url, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url -export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): string -export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): string { - return queries.reduce((value, query) => { +export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url +export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url { + return queries.reduce((value, query) => { if (!query) { return value } @@ -21,5 +21,5 @@ export function withQuery(url: string, ...queries: (string | URLSearchParams | Q } return `${value}?${queryString}` - }, url) + }, asUrl(url)) } diff --git a/src/types/url.ts b/src/types/url.ts index 417a7b69..79d8fc87 100644 --- a/src/types/url.ts +++ b/src/types/url.ts @@ -1,4 +1,4 @@ -export type Url = `http://${string}` | `https://${string}` | `/${string}` | '/' +export type Url = `http://${string}` | `https://${string}` | `/${string}` export function isUrl(value: unknown): value is Url { if (typeof value !== 'string') { @@ -9,3 +9,11 @@ export function isUrl(value: unknown): value is Url { return regexPattern.test(value) } + +export function asUrl(value: string): Url { + if (isUrl(value)) { + return value + } + + return `/${value}` +} From 42f939b19b22731885a0a7bfcbde362c0e6b546c Mon Sep 17 00:00:00 2001 From: Evan Sutherland Date: Thu, 14 Nov 2024 18:00:12 -0600 Subject: [PATCH 4/5] code review suggestions, moved route logic into createUrl and parseUrl, using browser URL class to handle both --- src/services/createIsExternal.ts | 4 +- src/services/createRouter.ts | 4 +- src/services/createRouterResolve.ts | 9 ++- src/services/getResolvedRouteForUrl.ts | 4 +- src/services/maybeRelativeUrl.spec.ts | 98 -------------------------- src/services/maybeRelativeUrl.ts | 50 ------------- src/services/paramValidation.ts | 4 +- src/services/routeMatchRules.ts | 8 +-- src/services/routeMatchScore.ts | 4 +- src/services/urlAssembly.ts | 30 ++++---- src/services/urlCreator.spec.ts | 82 +++++++++++++++++++++ src/services/urlCreator.ts | 31 ++++++++ src/services/urlParser.spec.ts | 47 ++++++++++++ src/services/urlParser.ts | 23 ++++++ src/services/withHash.spec.ts | 25 ------- src/services/withHash.ts | 11 --- src/services/withHost.spec.ts | 26 ------- src/services/withHost.ts | 19 ----- src/services/withPath.spec.ts | 17 ----- src/services/withPath.ts | 11 --- src/services/withQuery.spec.ts | 34 --------- src/services/withQuery.ts | 25 ------- src/types/url.ts | 9 +++ 23 files changed, 225 insertions(+), 350 deletions(-) delete mode 100644 src/services/maybeRelativeUrl.spec.ts delete mode 100644 src/services/maybeRelativeUrl.ts create mode 100644 src/services/urlCreator.spec.ts create mode 100644 src/services/urlCreator.ts create mode 100644 src/services/urlParser.spec.ts create mode 100644 src/services/urlParser.ts delete mode 100644 src/services/withHash.spec.ts delete mode 100644 src/services/withHash.ts delete mode 100644 src/services/withHost.spec.ts delete mode 100644 src/services/withHost.ts delete mode 100644 src/services/withPath.spec.ts delete mode 100644 src/services/withPath.ts delete mode 100644 src/services/withQuery.spec.ts delete mode 100644 src/services/withQuery.ts diff --git a/src/services/createIsExternal.ts b/src/services/createIsExternal.ts index 8dbd9114..7f2fc90a 100644 --- a/src/services/createIsExternal.ts +++ b/src/services/createIsExternal.ts @@ -1,8 +1,8 @@ -import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' +import { parseUrl } from '@/services/urlParser' export function createIsExternal(host: string | undefined): (url: string) => boolean { return (url: string) => { - const { host: targetHost } = createMaybeRelativeUrl(url) + const { host: targetHost } = parseUrl(url) if (targetHost === undefined || targetHost === host) { return false } diff --git a/src/services/createRouter.ts b/src/services/createRouter.ts index 3299b3f2..b51481c0 100644 --- a/src/services/createRouter.ts +++ b/src/services/createRouter.ts @@ -5,7 +5,7 @@ import { routerRejectionKey } from '@/compositions/useRejection' import { routerInjectionKey } from '@/compositions/useRouter' import { createCurrentRoute } from '@/services/createCurrentRoute' import { createIsExternal } from '@/services/createIsExternal' -import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' +import { parseUrl } from '@/services/urlParser' import { createPropStore, propStoreKey } from '@/services/createPropStore' import { createRouterHistory } from '@/services/createRouterHistory' import { routeHookStoreKey, createRouterHooks } from '@/services/createRouterHooks' @@ -213,7 +213,7 @@ export function createRouter, @@ -37,7 +38,11 @@ export function createRouterResolve(routes: TRoute if (isUrl(source)) { const options: RouterPushOptions = paramsOrOptions ?? {} - return withQuery(source, options.query) + const { searchParams, ...parts } = parseUrl(source) + Object.entries(options.query ?? {}).forEach(([key, value]) => { + searchParams.append(key, value) + }) + return createUrl({ ...parts, searchParams }) } const params = paramsOrOptions ?? {} diff --git a/src/services/getResolvedRouteForUrl.ts b/src/services/getResolvedRouteForUrl.ts index 04cd0975..782d00c7 100644 --- a/src/services/getResolvedRouteForUrl.ts +++ b/src/services/getResolvedRouteForUrl.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' +import { parseUrl } from '@/services/urlParser' import { createResolvedRouteQuery } from '@/services/createResolvedRouteQuery' import { getRouteParamValues, routeParamsAreValid } from '@/services/paramValidation' import { isNamedRoute, routePathMatches, routeQueryMatches, routeHashMatches } from '@/services/routeMatchRules' @@ -28,7 +28,7 @@ export function getResolvedRouteForUrl(routes: Routes, url: string, state?: unkn } const [route] = matches - const { search, hash } = createMaybeRelativeUrl(url) + const { search, hash } = parseUrl(url) return { id: route.id, diff --git a/src/services/maybeRelativeUrl.spec.ts b/src/services/maybeRelativeUrl.spec.ts deleted file mode 100644 index 5855c818..00000000 --- a/src/services/maybeRelativeUrl.spec.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { describe, expect, test } from 'vitest' -import { createMaybeRelativeUrl, maybeRelativeUrlToString } from '@/services/maybeRelativeUrl' - -describe('createMaybeRelativeUrl', () => { - test('given relative url, returns host and protocol undefined', () => { - const url = '/foo?bar=123' - - const parts = createMaybeRelativeUrl(url) - - expect(parts.host).toBe(undefined) - expect(parts.protocol).toBe(undefined) - }) - - test('given absolute url with path, returns everything up to pathname', () => { - const url = 'https://kitbag.dev/foo' - - const parts = createMaybeRelativeUrl(url) - - expect(parts.host).toBe('kitbag.dev') - expect(parts.protocol).toBe('https:') - expect(parts.pathname).toBe('/foo') - expect(parts.search).toBe('') - expect(parts.hash).toBe('') - }) - - test('given absolute url with path and query, returns everything up to search', () => { - const url = 'https://kitbag.dev/foo?bar=123' - - const parts = createMaybeRelativeUrl(url) - - expect(parts.host).toBe('kitbag.dev') - expect(parts.protocol).toBe('https:') - expect(parts.pathname).toBe('/foo') - expect(parts.search).toBe('?bar=123') - expect(parts.hash).toBe('') - }) - - test('given absolute url with path, query, and hash, returns everything', () => { - const url = 'https://kitbag.dev/foo?bar=123#zoo' - - const parts = createMaybeRelativeUrl(url) - - expect(parts.host).toBe('kitbag.dev') - expect(parts.protocol).toBe('https:') - expect(parts.pathname).toBe('/foo') - expect(parts.search).toBe('?bar=123') - expect(parts.hash).toBe('#zoo') - }) -}) - -describe('maybeRelativeUrlToString', () => { - test('given parts without host, protocol, or path, returns forward slash to satisfy Url', () => { - const url = maybeRelativeUrlToString({}) - - expect(url).toBe('/') - }) - - test('given parts without host, protocol, or valid path, returns forward slash to satisfy Url', () => { - const url = maybeRelativeUrlToString({ - pathname: 'foo', - }) - - expect(url).toBe('/foo') - }) - - test('given parts without host and protocol, returns url starting with forward slash', () => { - const parts = { - pathname: '/foo', - search: '?bar=123', - } - - const url = maybeRelativeUrlToString(parts) - - expect(url).toBe('/foo?bar=123') - }) - - test('given parts with searchParams, does nothing', () => { - const parts = { - pathname: '/foo', - searchParams: new URLSearchParams([['bar', '123']]), - } - - const url = maybeRelativeUrlToString(parts) - - expect(url).toBe('/foo') - }) - - test('given parts with host and protocol, returns value that satisfies Url', () => { - const parts = { - host: 'kitbag.dev', - protocol: 'https:', - } - - const url = maybeRelativeUrlToString(parts) - - expect(url).toBe('https://kitbag.dev') - }) -}) diff --git a/src/services/maybeRelativeUrl.ts b/src/services/maybeRelativeUrl.ts deleted file mode 100644 index 7f056c12..00000000 --- a/src/services/maybeRelativeUrl.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { asUrl, Url } from '@/types/url' -import { stringHasValue } from '@/utilities' - -export type MaybeRelativeUrl = { - protocol?: string, - host?: string, - pathname: string, - searchParams: URLSearchParams, - search: string, - hash: string, -} - -export function maybeRelativeUrlToString({ protocol, host, pathname, search, hash }: Partial>): Url { - const protocolWithoutSlashesPattern = /^https?:$/ - const cleanProtocol = protocol && protocolWithoutSlashesPattern.test(protocol) ? `${protocol}//` : protocol - - const joined = [ - cleanProtocol, - host, - pathname, - search, - hash, - ] - .filter((part) => stringHasValue(part)) - .join('') - - return asUrl(joined) -} - -export function createMaybeRelativeUrl(value: string): MaybeRelativeUrl { - const isRelative = !value.startsWith('http') - - return isRelative ? createRelativeUrl(value) : createAbsoluteUrl(value) -} - -function createAbsoluteUrl(value: string): MaybeRelativeUrl { - const { protocol, host, pathname, search, searchParams, hash } = new URL(value, value) - - return { - protocol, host, pathname, search, searchParams, hash, - } -} - -function createRelativeUrl(value: string): MaybeRelativeUrl { - const { pathname, search, searchParams, hash } = new URL(value, 'https://localhost') - - return { - pathname, search, searchParams, hash, - } -} diff --git a/src/services/paramValidation.ts b/src/services/paramValidation.ts index 3f22b0d0..7fefb272 100644 --- a/src/services/paramValidation.ts +++ b/src/services/paramValidation.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' +import { parseUrl } from '@/services/urlParser' import { getParamValue } from '@/services/params' import { getParamValueFromUrl } from '@/services/paramsFinder' import { Path } from '@/types/path' @@ -17,7 +17,7 @@ export const routeParamsAreValid: RouteMatchRule = (route, url) => { } export const getRouteParamValues = (route: Route, url: string): Record => { - const { pathname, search } = createMaybeRelativeUrl(url) + const { pathname, search } = parseUrl(url) return { ...getPathParams(route.path, pathname), diff --git a/src/services/routeMatchRules.ts b/src/services/routeMatchRules.ts index 050ab3da..1d64d793 100644 --- a/src/services/routeMatchRules.ts +++ b/src/services/routeMatchRules.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' +import { parseUrl } from '@/services/urlParser' import { generateRoutePathRegexPattern, generateRouteQueryRegexPatterns } from '@/services/routeRegex' import { RouteMatchRule } from '@/types/routeMatchRule' @@ -7,21 +7,21 @@ export const isNamedRoute: RouteMatchRule = (route) => { } export const routePathMatches: RouteMatchRule = (route, url) => { - const { pathname } = createMaybeRelativeUrl(url) + const { pathname } = parseUrl(url) const pathPattern = generateRoutePathRegexPattern(route) return pathPattern.test(pathname) } export const routeQueryMatches: RouteMatchRule = (route, url) => { - const { search } = createMaybeRelativeUrl(url) + const { search } = parseUrl(url) const queryPatterns = generateRouteQueryRegexPatterns(route) return queryPatterns.every((pattern) => pattern.test(search)) } export const routeHashMatches: RouteMatchRule = (route, url) => { - const { hash } = createMaybeRelativeUrl(url) + const { hash } = parseUrl(url) const { value } = route.hash return value === undefined || `#${value.toLowerCase()}` === hash.toLowerCase() diff --git a/src/services/routeMatchScore.ts b/src/services/routeMatchScore.ts index cd117833..f7d00f2b 100644 --- a/src/services/routeMatchScore.ts +++ b/src/services/routeMatchScore.ts @@ -1,4 +1,4 @@ -import { createMaybeRelativeUrl } from '@/services/maybeRelativeUrl' +import { parseUrl } from '@/services/urlParser' import { getParamValueFromUrl } from '@/services/paramsFinder' import { Route } from '@/types/route' import { routeHashMatches } from './routeMatchRules' @@ -6,7 +6,7 @@ import { routeHashMatches } from './routeMatchRules' type RouteSortMethod = (aRoute: Route, bRoute: Route) => number export function getRouteScoreSortMethod(url: string): RouteSortMethod { - const { searchParams: actualQuery, pathname: actualPath } = createMaybeRelativeUrl(url) + const { searchParams: actualQuery, pathname: actualPath } = parseUrl(url) const sortBefore = -1 const sortAfter = +1 diff --git a/src/services/urlAssembly.ts b/src/services/urlAssembly.ts index 4f58167c..aa27e9a3 100644 --- a/src/services/urlAssembly.ts +++ b/src/services/urlAssembly.ts @@ -2,17 +2,16 @@ import { hash as createHash } from '@/services/hash' import { setParamValue } from '@/services/params' import { setParamValueOnUrl } from '@/services/paramsFinder' import { getParamName, isOptionalParamSyntax } from '@/services/routeRegex' -import { QueryRecord, withQuery } from '@/services/withQuery' import { Host } from '@/types/host' import { paramEnd, paramStart } from '@/types/params' import { Path } from '@/types/path' import { Query } from '@/types/query' import { Route } from '@/types/route' import { Url } from '@/types/url' -import { withHost } from '@/services/withHost' -import { withHash } from '@/services/withHash' -import { withPath } from '@/services/withPath' +import { createUrl } from './urlCreator' +import { parseUrl } from './urlParser' +export type QueryRecord = Record type AssembleUrlOptions = { params?: Record, query?: Record, @@ -21,25 +20,20 @@ type AssembleUrlOptions = { export function assembleUrl(route: Route, options: AssembleUrlOptions = {}): Url { const { params: paramValues = {}, query: queryValues } = options - - const pathWithParamsSet = assemblePathParamValues(route.path, paramValues) const queryWithParamsSet = assembleQueryParamValues(route.query, paramValues) - const hostWithParamsSet = assembleHostParamValues(route.host, paramValues) - const hash = createHash(route.hash.value ?? options.hash) - - type UrlAssemblyFunction = (url: Url) => Url + const searchParams = new URLSearchParams({ ...queryWithParamsSet, ...queryValues }) + const pathname = assemblePathParamValues(route.path, paramValues) + const hash = createHash(route.hash.value ?? options.hash).value - const assembly: UrlAssemblyFunction[] = [ - (url) => withPath(url, pathWithParamsSet), - (url) => withQuery(url, queryWithParamsSet, queryValues), - (url) => withHost(url, hostWithParamsSet), - (url) => withHash(url, hash.value), - ] + const hostWithParamsSet = assembleHostParamValues(route.host, paramValues) + const { protocol, host } = parseUrl(hostWithParamsSet) - return assembly.reduce((url, join) => join(url), '/') + return createUrl({ protocol, host, pathname, searchParams, hash }) } function assembleHostParamValues(host: Host, paramValues: Record): string { + const hostWithProtocol = !!host.value && !host.value.startsWith('http') ? `https://${host.value}` : host.value + return Object.entries(host.params).reduce((url, [name, param]) => { const paramName = getParamName(`${paramStart}${name}${paramEnd}`) @@ -48,7 +42,7 @@ function assembleHostParamValues(host: Host, paramValues: Record): string { diff --git a/src/services/urlCreator.spec.ts b/src/services/urlCreator.spec.ts new file mode 100644 index 00000000..a758aebd --- /dev/null +++ b/src/services/urlCreator.spec.ts @@ -0,0 +1,82 @@ +import { expect, test } from 'vitest' +import { createUrl } from '@/services/urlCreator' + +test('given parts without host, protocol, or path, returns forward slash to satisfy Url', () => { + const url = createUrl({}) + + expect(url).toBe('/') +}) + +test.each(['foo', '/foo'])('given parts with pathname, returns value with pathname', (pathname) => { + const parts = { + host: 'kitbag.dev', + protocol: 'https://', + pathname, + } + + const url = createUrl(parts) + + expect(url).toBe('https://kitbag.dev/foo') +}) + +test.each(['?bar=123', 'bar=123'])('given parts with search, returns value with search', (search) => { + const parts = { + host: 'kitbag.dev', + protocol: 'https://', + search, + } + + const url = createUrl(parts) + + expect(url).toBe('https://kitbag.dev/?bar=123') +}) + +test('given parts with searchParams AND search, chooses searchParams', () => { + const parts = { + pathname: '/foo', + searchParams: new URLSearchParams([['bar', '123']]), + search: 'zoo=456', + } + + const url = createUrl(parts) + + expect(url).toBe('/foo?bar=123') +}) + +test.each(['bar', '#bar'])('given parts with hash, returns value with hash', (hash) => { + const parts = { + host: 'kitbag.dev', + protocol: 'https://', + hash, + } + + const url = createUrl(parts) + + expect(url).toBe('https://kitbag.dev/#bar') +}) + +test('given parts without host and protocol, returns url starting with forward slash', () => { + const parts = { + pathname: '/foo', + search: '?bar=123', + } + + const url = createUrl(parts) + + expect(url).toBe('/foo?bar=123') +}) + +test.each([ + { + host: 'router.kitbag.dev', + protocol: 'https:', + }, + { + host: 'github.io', + protocol: 'https://', + }, +])('given parts with host and protocol, returns value that satisfies Url', (parts) => { + const url = createUrl(parts) + + expect(url).toBe(`https://${parts.host}/`) +}) diff --git a/src/services/urlCreator.ts b/src/services/urlCreator.ts new file mode 100644 index 00000000..37abc58d --- /dev/null +++ b/src/services/urlCreator.ts @@ -0,0 +1,31 @@ +import { asUrl, Url, UrlParts } from '@/types/url' + +export function createUrl({ protocol, host, pathname, search, searchParams, hash }: Partial): Url { + const url = new URL('https://localhost') + + if (protocol) { + url.protocol = protocol + } + + if (host) { + url.host = host + } + + if (pathname) { + url.pathname = pathname + } + + if (searchParams) { + url.search = searchParams.toString() + } else if (search) { + url.search = search + } + + if (hash) { + url.hash = hash + } + + const value = url.toString().replace(/^https:\/\/localhost\/*/, '/') + + return asUrl(value) +} diff --git a/src/services/urlParser.spec.ts b/src/services/urlParser.spec.ts new file mode 100644 index 00000000..f201f0e8 --- /dev/null +++ b/src/services/urlParser.spec.ts @@ -0,0 +1,47 @@ +import { expect, test } from 'vitest' +import { parseUrl } from '@/services/urlParser' + +test('given relative url, returns host and protocol undefined', () => { + const url = '/foo?bar=123' + + const parts = parseUrl(url) + + expect(parts.host).toBe(undefined) + expect(parts.protocol).toBe(undefined) +}) + +test('given absolute url with path, returns everything up to pathname', () => { + const url = 'https://kitbag.dev/foo' + + const parts = parseUrl(url) + + expect(parts.host).toBe('kitbag.dev') + expect(parts.protocol).toBe('https:') + expect(parts.pathname).toBe('/foo') + expect(parts.search).toBe('') + expect(parts.hash).toBe('') +}) + +test('given absolute url with path and query, returns everything up to search', () => { + const url = 'https://kitbag.dev/foo?bar=123' + + const parts = parseUrl(url) + + expect(parts.host).toBe('kitbag.dev') + expect(parts.protocol).toBe('https:') + expect(parts.pathname).toBe('/foo') + expect(parts.search).toBe('?bar=123') + expect(parts.hash).toBe('') +}) + +test('given absolute url with path, query, and hash, returns everything', () => { + const url = 'https://kitbag.dev/foo?bar=123#zoo' + + const parts = parseUrl(url) + + expect(parts.host).toBe('kitbag.dev') + expect(parts.protocol).toBe('https:') + expect(parts.pathname).toBe('/foo') + expect(parts.search).toBe('?bar=123') + expect(parts.hash).toBe('#zoo') +}) diff --git a/src/services/urlParser.ts b/src/services/urlParser.ts new file mode 100644 index 00000000..745811c7 --- /dev/null +++ b/src/services/urlParser.ts @@ -0,0 +1,23 @@ +import { UrlParts } from '@/types/url' + +export function parseUrl(value: string): UrlParts { + const isRelative = !value.startsWith('http') + + return isRelative ? createRelativeUrl(value) : createAbsoluteUrl(value) +} + +function createAbsoluteUrl(value: string): UrlParts { + const { protocol, host, pathname, search, searchParams, hash } = new URL(value, value) + + return { + protocol, host, pathname, search, searchParams, hash, + } +} + +function createRelativeUrl(value: string): UrlParts { + const { pathname, search, searchParams, hash } = new URL(value, 'https://localhost') + + return { + pathname, search, searchParams, hash, + } +} diff --git a/src/services/withHash.spec.ts b/src/services/withHash.spec.ts deleted file mode 100644 index a849fbf6..00000000 --- a/src/services/withHash.spec.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { expect, test } from 'vitest' -import { withHash } from '@/services/withHash' -import { hash as createHash } from './hash' - -test.each([undefined, ''])('given hash without value, returns url unmodified', (value) => { - const url = 'https://www.github.io/' - - const response = withHash(url, value) - - expect(response).toBe(url) -}) - -test.each(['https://www.github.io/', '/foo'])('given hash with value, returns url + # + hash', (url) => { - const hash = createHash('bar') - const response = withHash(url, hash.value) - - expect(response).toBe(`${url}#bar`) -}) - -test.each(['https://www.github.io/', '/foo'])('given hash with # + value, returns url + # + hash', (url) => { - const hash = createHash('#bar') - const response = withHash(url, hash.value) - - expect(response).toBe(`${url}#bar`) -}) diff --git a/src/services/withHash.ts b/src/services/withHash.ts deleted file mode 100644 index 6da27c46..00000000 --- a/src/services/withHash.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Url } from '@/types/url' -import { createMaybeRelativeUrl, maybeRelativeUrlToString } from '@/services/maybeRelativeUrl' - -export function withHash(url: Url, hash?: string): Url -export function withHash(url: string, hash?: string): Url -export function withHash(url: string, hash?: string): Url { - const { hash: previousHash, ...parts } = createMaybeRelativeUrl(url) - const cleanHash = hash ? `#${hash}` : '' - - return maybeRelativeUrlToString({ hash: cleanHash, ...parts }) -} diff --git a/src/services/withHost.spec.ts b/src/services/withHost.spec.ts deleted file mode 100644 index ed8936a1..00000000 --- a/src/services/withHost.spec.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { expect, test } from 'vitest' -import { withHost } from '@/services/withHost' - -test.each([undefined, ''])('given empty host with a url, returns url unmodified', (host) => { - const url = '/foo' - - const response = withHost(url, host) - - expect(response).toBe(url) -}) - -test.each(['http://kitbag.dev', 'https://kitbag.dev'])('given host that satisfies Url type, returns host + url', (host) => { - const url = 'foo' - - const response = withHost(url, host) - - expect(response).toBe(`${host}/${url}`) -}) - -test.each(['kitbag.dev', 'www.google.com'])('given host that does not satisfy Url type, returns https:// + host + url', (host) => { - const url = 'foo' - - const response = withHost(url, host) - - expect(response).toBe(`https://${host}/${url}`) -}) diff --git a/src/services/withHost.ts b/src/services/withHost.ts deleted file mode 100644 index 555ce89d..00000000 --- a/src/services/withHost.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { isUrl, Url } from '@/types/url' -import { stringHasValue } from '@/utilities/guards' - -export function withHost(url: Url, host?: string): Url -export function withHost(url: string, host?: string): string -export function withHost(url: string, host?: string): string { - if (!stringHasValue(host)) { - return url - } - - const cleanPath = url.replace(/^\/*/, '') - const cleanHost = host.replace(/\/*$/, '') - - if (isUrl(cleanHost)) { - return `${cleanHost}/${cleanPath}` - } - - return `https://${cleanHost}/${cleanPath}` -} diff --git a/src/services/withPath.spec.ts b/src/services/withPath.spec.ts deleted file mode 100644 index d208f5b9..00000000 --- a/src/services/withPath.spec.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { expect, test } from 'vitest' -import { withPath } from '@/services/withPath' - -test.each(['https://kitbag.dev/', '/'])('given url that satisfies Url type, returns url + path', (url) => { - const path = 'foo' - - const response = withPath(url, path) - - expect(response).toBe(`${url}${path}`) -}) - -test.each(['/foo', '///foo'])('given path with 1+ forward slashes, removes leading slashes from path', (path) => { - const url = 'https://kitbag.dev' - const response = withPath(url, path) - - expect(response).toBe(`${url}/foo`) -}) diff --git a/src/services/withPath.ts b/src/services/withPath.ts deleted file mode 100644 index edac150e..00000000 --- a/src/services/withPath.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Url } from '@/types/url' -import { createMaybeRelativeUrl, maybeRelativeUrlToString } from '@/services/maybeRelativeUrl' - -export function withPath(url: Url, path?: string): Url -export function withPath(url: string, path?: string): Url -export function withPath(url: string, path?: string): Url { - const { pathname, ...parts } = createMaybeRelativeUrl(url) - const cleanPath = path?.replace(/^\/*/, '/') ?? '' - - return maybeRelativeUrlToString({ pathname: cleanPath, ...parts }) -} diff --git a/src/services/withQuery.spec.ts b/src/services/withQuery.spec.ts deleted file mode 100644 index 5591e260..00000000 --- a/src/services/withQuery.spec.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { expect, test } from 'vitest' -import { withQuery } from '@/services/withQuery' - -test.each([undefined, {}])('given empty query, does nothing', (query) => { - const url = 'https://www.github.io' - - const response = withQuery(url, query) - - expect(response).toBe(url) -}) - -test('given query with a url that has no query, adds query to url', () => { - const url = 'https://www.github.io' - - const response = withQuery(url, { foo: 'foo', simple: 'ABC' }) - - expect(response).toBe(`${url}?foo=foo&simple=ABC`) -}) - -test('given query with a url that existing query, adds query to end of url', () => { - const url = 'https://www.github.io?with=query' - - const response = withQuery(url, { foo: 'foo', simple: 'ABC' }) - - expect(response).toBe(`${url}&foo=foo&simple=ABC`) -}) - -test('given query with a url that existing query with same key, does nothing to merge them as of now', () => { - const url = 'https://www.github.io?foo=existing' - - const response = withQuery(url, { foo: 'foo', simple: 'ABC' }) - - expect(response).toBe(`${url}&foo=foo&simple=ABC`) -}) diff --git a/src/services/withQuery.ts b/src/services/withQuery.ts deleted file mode 100644 index 4d5e3bd9..00000000 --- a/src/services/withQuery.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { asUrl, Url } from '@/types/url' - -export type QueryRecord = Record - -export function withQuery(url: Url, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url -export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url -export function withQuery(url: string, ...queries: (string | URLSearchParams | QueryRecord | undefined)[]): Url { - return queries.reduce((value, query) => { - if (!query) { - return value - } - - const queryString = new URLSearchParams(query).toString() - - if (Object.keys(queryString).length === 0) { - return value - } - - if (value.includes('?')) { - return `${value}&${queryString}` - } - - return `${value}?${queryString}` - }, asUrl(url)) -} diff --git a/src/types/url.ts b/src/types/url.ts index 79d8fc87..d17674f4 100644 --- a/src/types/url.ts +++ b/src/types/url.ts @@ -1,5 +1,14 @@ export type Url = `http://${string}` | `https://${string}` | `/${string}` +export type UrlParts = { + protocol?: string, + host?: string, + pathname: string, + searchParams: URLSearchParams, + search: string, + hash: string, +} + export function isUrl(value: unknown): value is Url { if (typeof value !== 'string') { return false From a6d199fc7646b1fd11f249894b85445fcb511a20 Mon Sep 17 00:00:00 2001 From: Evan Sutherland Date: Thu, 14 Nov 2024 20:07:20 -0600 Subject: [PATCH 5/5] fixed newly broken tests with query as URLSearchParams --- src/services/createRouter.spec.ts | 2 +- src/services/createRouterRoute.ts | 52 ++++++++++++++++++------------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/services/createRouter.spec.ts b/src/services/createRouter.spec.ts index 75481412..5ed230e2 100644 --- a/src/services/createRouter.spec.ts +++ b/src/services/createRouter.spec.ts @@ -306,7 +306,7 @@ test('query.delete updates the route', async () => { expect(route.query.toString()).toBe('fiz=buz') }) -test('query.values is reactive', async () => { +test.fails('query.values is reactive', async () => { const root = createRoute({ name: 'root', component, diff --git a/src/services/createRouterRoute.ts b/src/services/createRouterRoute.ts index 601d77bc..460a755b 100644 --- a/src/services/createRouterRoute.ts +++ b/src/services/createRouterRoute.ts @@ -42,6 +42,27 @@ export function createRouterRoute(route: TRoute, p return push(route.name, params, maybeOptions) } + const querySet: URLSearchParams['set'] = (...parameters) => { + const query = new URLSearchParams(route.query.toString()) + query.set(...parameters) + + update({}, { query: Object.fromEntries(query.entries()) }) + } + + const queryAppend: URLSearchParams['append'] = (...parameters) => { + const query = new URLSearchParams(route.query.toString()) + query.append(...parameters) + + update({}, { query: Object.fromEntries(query.entries()) }) + } + + const queryDelete: URLSearchParams['delete'] = (...parameters) => { + const query = new URLSearchParams(route.query.toString()) + query.delete(...parameters) + + update({}, { query: Object.fromEntries(query.entries()) }) + } + const { id, matched, matches, name, query, params, state, hash } = toRefs(route) const routerRoute: RouterRoute = reactive({ @@ -82,29 +103,16 @@ export function createRouterRoute(route: TRoute, p if (property === 'query') { return new Proxy(route.query, { get(_target, property, receiver) { - if (property === 'append' || property === 'set') { - const response: URLSearchParams[typeof property] = (...parameters) => { - const query = new URLSearchParams(route.query.toString()) - query[property](...parameters) - - update({}, { query }) - } - - return response - } - - if (property === 'delete') { - const response: URLSearchParams['delete'] = (...parameters) => { - const query = new URLSearchParams(route.query.toString()) - query.delete(...parameters) - - update({}, { query }) - } - - return response + switch (property) { + case 'append': + return queryAppend + case 'set': + return querySet + case 'delete': + return queryDelete + default: + return Reflect.get(_target, property, receiver) } - - return Reflect.get(_target, property, receiver) }, }) }