From d0cc021e6060afb0c86c6999f1e4e8c46a6cbfeb Mon Sep 17 00:00:00 2001 From: "Sam A. Horvath-Hunt" Date: Wed, 1 May 2024 18:28:27 +0100 Subject: [PATCH] Make URLPath.fromString infallible --- CHANGELOG.md | 2 + docs/modules/URLPath.ts.md | 44 +++---------------- src/URLPath.ts | 66 ++++++++++++----------------- test/URLPath.test.ts | 86 +++++++++++++++++--------------------- 4 files changed, 74 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2abbf6f9..fc19ceac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ This project adheres to semantic versioning. ## _Unreleased_ +- Improve `fromString` in `URLPath` to infallibly parse all strings. +- Remove `fromStringO` from `URLPath`. - Fix `toURL`/`toURLO` in `URLPath` failing to prioritise the retention of the provided base URL. ## 0.19.1 (2024-04-08) diff --git a/docs/modules/URLPath.ts.md b/docs/modules/URLPath.ts.md index 5aa093eb..ab58a2fd 100644 --- a/docs/modules/URLPath.ts.md +++ b/docs/modules/URLPath.ts.md @@ -26,7 +26,6 @@ Added in v0.17.0 - [clone](#clone) - [fromPathname](#frompathname) - [fromString](#fromstring) - - [fromStringO](#fromstringo) - [fromURL](#fromurl) - [getHash](#gethash) - [getParams](#getparams) @@ -158,17 +157,17 @@ Added in v0.17.0 ## fromString -Build a `URLPath` from a string containing any parts. For an infallible -alternative taking only a pathname, consider `fromPathname`. +Build a `URLPath` from a relative or absolute string containing any parts. +Consider also `fromPathname` where only a pathname needs to be parsed. **Signature** ```ts -export declare const fromString: (f: (e: TypeError) => E) => (x: string) => Either +export declare const fromString: (x: string) => URLPath ``` ```hs -fromString :: (TypeError -> e) -> string -> Either e URLPath +fromString :: string -> URLPath ``` **Example** @@ -178,41 +177,10 @@ import { pipe, constant } from 'fp-ts/function' import * as E from 'fp-ts/Either' import { fromString, fromPathname, setHash } from 'fp-ts-std/URLPath' -const f = fromString(constant('oops')) - -const expected = pipe('/foo', fromPathname, setHash('bar')) - -assert.deepStrictEqual(f('/foo#bar'), E.right(expected)) -assert.deepStrictEqual(f('//'), E.left('oops')) -``` - -Added in v0.17.0 - -## fromStringO - -Build a `URLPath` from a string containing any parts, forgoing the error. - -**Signature** - -```ts -export declare const fromStringO: (x: string) => Option -``` - -```hs -fromStringO :: string -> Option URLPath -``` - -**Example** - -```ts -import { pipe } from 'fp-ts/function' -import * as O from 'fp-ts/Option' -import { fromStringO, fromPathname, setHash } from 'fp-ts-std/URLPath' - const expected = pipe('/foo', fromPathname, setHash('bar')) -assert.deepStrictEqual(fromStringO('/foo#bar'), O.some(expected)) -assert.deepStrictEqual(fromStringO('//'), O.none) +assert.deepStrictEqual(fromString('/foo#bar'), expected) +assert.deepStrictEqual(fromString('https://samhh.com/foo#bar'), expected) ``` Added in v0.17.0 diff --git a/src/URLPath.ts b/src/URLPath.ts index 185e1024..a7300604 100644 --- a/src/URLPath.ts +++ b/src/URLPath.ts @@ -14,6 +14,7 @@ import type { Endomorphism } from "fp-ts/Endomorphism" import * as Eq_ from "fp-ts/Eq" import type { Option } from "fp-ts/Option" import * as O from "fp-ts/Option" +import type { Predicate } from "fp-ts/Predicate" import type { Refinement } from "fp-ts/Refinement" import { flow, identity, pipe } from "fp-ts/function" import type { Newtype } from "newtype-ts" @@ -39,7 +40,7 @@ export type URLPath = Newtype const phonyBase = new globalThis.URL("https://urlpath.fp-ts-std.samhh.com") -const ensureBase: Endomorphism = x => +const ensurePhonyBase: Endomorphism = x => pipe(phonyBase, URL.clone, b => { b.pathname = x.pathname b.search = Params.toString(x.searchParams) @@ -47,6 +48,13 @@ const ensureBase: Endomorphism = x => return b }) +const hasPhonyBase: Predicate = x => x.origin === phonyBase.origin + +const assertPhonyBase: Endomorphism = x => { + if (!hasPhonyBase(x)) throw new Error("Invalid phony base") + return x +} + /** * Check if a foreign value is a `URLPath`. * @@ -101,7 +109,7 @@ export const clone: Endomorphism = over(URL.clone) * @since 0.17.0 */ export const fromURL = (x: URL): URLPath => - pipe(new globalThis.URL(x.href, phonyBase), ensureBase, pack) + pipe(new globalThis.URL(x.href, phonyBase), ensurePhonyBase, pack) /** * Convert a `URLPath` to a `URL` with the provided `baseUrl`. @@ -168,57 +176,37 @@ export const toURLO = (baseUrl: string): ((x: URLPath) => Option) => flow(toURL(identity)(baseUrl), O.fromEither) /** - * Build a `URLPath` from a string containing any parts. For an infallible - * alternative taking only a pathname, consider `fromPathname`. + * Build a `URLPath` from a relative or absolute string containing any parts. + * Consider also `fromPathname` where only a pathname needs to be parsed. * * @example * import { pipe, constant } from 'fp-ts/function'; * import * as E from 'fp-ts/Either'; * import { fromString, fromPathname, setHash } from 'fp-ts-std/URLPath' * - * const f = fromString(constant('oops')) - * * const expected = pipe('/foo', fromPathname, setHash('bar')) * - * assert.deepStrictEqual(f('/foo#bar'), E.right(expected)) - * assert.deepStrictEqual(f('//'), E.left('oops')) + * assert.deepStrictEqual(fromString('/foo#bar'), expected) + * assert.deepStrictEqual(fromString('https://samhh.com/foo#bar'), expected) * * @category 3 Functions * @since 0.17.0 */ -export const fromString = - (f: (e: TypeError) => E) => - (x: string): Either => - pipe( - // It should only throw some sort of `TypeError`: - // https://developer.mozilla.org/en-US/docs/Web/API/URL/URL +export const fromString = (x: string): URLPath => + pipe( + E.tryCatch( + () => assertPhonyBase(new globalThis.URL(x, phonyBase)), + identity, + ), + E.alt(() => E.tryCatch( - () => ensureBase(new globalThis.URL(x, phonyBase)), - e => f(e as TypeError), + () => assertPhonyBase(new globalThis.URL(`${phonyBase.origin}${x}`)), + identity, ), - E.map(pack), - ) - -/** - * Build a `URLPath` from a string containing any parts, forgoing the error. - * - * @example - * import { pipe } from 'fp-ts/function'; - * import * as O from 'fp-ts/Option'; - * import { fromStringO, fromPathname, setHash } from 'fp-ts-std/URLPath' - * - * const expected = pipe('/foo', fromPathname, setHash('bar')) - * - * assert.deepStrictEqual(fromStringO('/foo#bar'), O.some(expected)) - * assert.deepStrictEqual(fromStringO('//'), O.none) - * - * @category 3 Functions - * @since 0.17.0 - */ -export const fromStringO: (x: string) => Option = flow( - fromString(identity), - O.fromEither, -) + ), + E.getOrElse(() => new globalThis.URL(`${phonyBase.origin}/${x}`)), + pack, + ) /** * Build a `URLPath` from a path. Characters such as `?` will be encoded. diff --git a/test/URLPath.test.ts b/test/URLPath.test.ts index 8a60cb74..90ef2e2a 100644 --- a/test/URLPath.test.ts +++ b/test/URLPath.test.ts @@ -3,7 +3,7 @@ import fc from "fast-check" import * as laws from "fp-ts-laws" import * as E from "fp-ts/Either" import * as O from "fp-ts/Option" -import { apply, constant, flip, flow, identity, pipe } from "fp-ts/function" +import { apply, flip, flow, identity, pipe } from "fp-ts/function" import { unsafeUnwrapLeft, unsafeUnwrap as unsafeUnwrapRight, @@ -22,7 +22,6 @@ import { clone, fromPathname, fromString, - fromStringO, fromURL, getHash, getParams, @@ -50,8 +49,7 @@ const phonyBase = "https://urlpath.fp-ts-std.samhh.com" const validBase = "https://samhh.com" const invalidBase = "samhh.com" -const validPath = "/f/g.h?i=j&k=l&i=m#n" -const invalidPath = "//" +const examplePath = "/f/g.h?i=j&k=l&i=m#n" describe("URLPath", () => { describe("isURLPath", () => { @@ -61,7 +59,7 @@ describe("URLPath", () => { expect(f("foo")).toBe(false) expect(f(new URL(validBase))).toBe(false) expect(f(new URL(phonyBase))).toBe(true) - expect(f(new URL(phonyBase + validPath))).toBe(true) + expect(f(new URL(phonyBase + examplePath))).toBe(true) }) }) @@ -69,14 +67,14 @@ describe("URLPath", () => { const f = clone it("clones to an identical URLPath", () => { - const x = fromPathname(validPath) + const x = fromPathname(examplePath) expect(f(x)).toEqual(x) expect(toString(f(x))).toBe(toString(x)) }) it("clones without references", () => { - const x = fromPathname(validPath) + const x = fromPathname(examplePath) ;(x as unknown as URL).pathname = "/foo" ;(x as unknown as URL).search = "?foo=food&bar=bard&foo=fool" const y = f(x) @@ -99,7 +97,7 @@ describe("URLPath", () => { const f = fromURL it("retains the path, params, and hash", () => { - const x = new URL(validBase + validPath) + const x = new URL(validBase + examplePath) const y = pipe(x, f, unpack) expect(y.pathname).toEqual(x.pathname) @@ -179,60 +177,54 @@ describe("URLPath", () => { }) describe("fromString", () => { - const f = fromString(constant("e")) + const f = fromString + const g = flow(f, u => (u as unknown as URL).href) - it("succeeds for valid paths", () => { - expect(f("")).toEqual(E.right(new URL("", phonyBase))) - expect(f(validPath)).toEqual(E.right(new URL(validPath, phonyBase))) - }) + it("parses all parts", () => { + const [x, y, z] = Fn.fork([getPathname, getParams, getHash])( + f("/foo?bar=yes#baz"), + ) - it("passes a TypeError to the callback on failure", () => { - const e = pipe(invalidPath, fromString(identity), unsafeUnwrapLeft) + expect(x).toBe("/foo") + expect(y).toEqual(new URLSearchParams({ bar: "yes" })) + expect(z).toBe("#baz") + }) - // This doesn't work. I suspect a tooling bug. Sanity check in the REPL. - // expect(e).toBeInstanceOf(TypeError) + it("parses typical paths", () => { + expect(g("/")).toBe(`${phonyBase}/`) + expect(g("/foo.bar")).toBe(`${phonyBase}/foo.bar`) + expect(g(examplePath)).toBe(`${phonyBase}${examplePath}`) + }) - expect(e.name).toBe("TypeError") + it("parses atypical paths", () => { + expect(g("")).toBe(`${phonyBase}/`) + expect(g("//")).toBe(`${phonyBase}//`) + expect(g("//x")).toBe(`${phonyBase}//x`) + expect(g("///")).toBe(`${phonyBase}///`) + expect(g(":123")).toBe(`${phonyBase}/:123`) + expect(g(".net")).toBe(`${phonyBase}/.net`) + expect(g("a:")).toBe(`${phonyBase}/a:`) + expect(g("//samhh.com/foo/bar")).toBe(`${phonyBase}//samhh.com/foo/bar`) }) - it("accepts all valid paths", () => { + it("is infallible", () => { fc.assert( - fc.property(fc.webPath(), x => - expect(f(x)).toEqual(E.right(new URL(x, phonyBase))), - ), + fc.property(fc.oneof(fc.string(), fc.webPath()), x => { + f(x) + }), ) }) - it("parses but does not retain origins", () => { - const g = flow( - fromString(identity), - E.map(x => (x as unknown as URL).href), - ) - - expect(g("//x")).toEqual(E.right(`${phonyBase}/`)) - expect(g("https://samhh.com/foo")).toEqual(E.right(`${phonyBase}/foo`)) - + it("always keeps phony base", () => { fc.assert( - fc.property(fc.string().map(f).filter(E.isRight), ({ right: x }) => - expect((x as unknown as URL).origin).toBe(phonyBase), + fc.property(fc.oneof(fc.string(), fc.webPath()), x => + expect((f(x) as unknown as URL).origin).toBe(phonyBase), ), + { numRuns: 10000 }, ) }) }) - describe("fromStringO", () => { - const f = fromStringO - - it("succeeds for valid paths", () => { - expect(f("")).toEqual(O.some(new URL("", phonyBase))) - expect(f(validPath)).toEqual(O.some(new URL(validPath, phonyBase))) - }) - - it("fails for invalid paths", () => { - expect(f(invalidPath)).toEqual(O.none) - }) - }) - describe("toString", () => { const f = toString @@ -420,7 +412,7 @@ describe("URLPath", () => { describe("Eq", () => { const f = Eq.equals - const g = flow(fromStringO, unsafeUnwrapO) + const g = fromString it("works", () => { const x = "/foo.bar#baz"