Make baseUrl() more defensive in @effect/platform (#2903)

.changeset/tender-roses-exercise.md

@@ -0,0 +1,28 @@
+---
+"@effect/platform": patch
+---
+
+# Make baseUrl() more defensive in @effect/platform
+
+Sometimes, third party code may patch a missing global `location` to accommodate for non-browser JavaScript
+runtimes, e.g. Cloudflare Workers,
+Deno. [Such patch](https://github.com/jamsinclair/jSquash/pull/21/files#diff-322ca97cdcdd0d3b85c20a7d5cac703a2f9f3766fc762f98b9f6a9d4c5063ca3R21-R23)
+might not yield a fully valid `location`. This could
+break `baseUrl()`, which is called by `makeUrl()`.
+
+For example, the following code would log `Invalid URL: '/api/v1/users' with base 'NaN'`.
+
+```js
+import { makeUrl } from "@effect/platform/Http/UrlParams"
+
+globalThis.location = {href: ""}
+
+const url = makeUrl("/api/v1/users", [])
+
+// This would log "Invalid URL: '/api/v1/users' with base 'NaN'",
+// because location.origin + location.pathname return NaN in baseUrl()
+console.log(url.left.message)
+```
+
+Arguably, this is not an issue of Effect per se, but it's better to be defensive and handle such cases gracefully.
+So this change does that by checking if `location.orign` and `location.pathname` are available before accessing them.

packages/platform/src/Http/UrlParams.ts

@@ -224,7 +224,12 @@ export const makeUrl = (url: string, params: UrlParams): Either.Either<URL, Erro
 }
 
 const baseUrl = (): string | undefined => {
-  if ("location" in globalThis && globalThis.location !== undefined) {
+  if (
+    "location" in globalThis &&
+    globalThis.location !== undefined &&
+    globalThis.location.origin !== undefined &&
+    globalThis.location.pathname !== undefined
+  ) {
     return location.origin + location.pathname
   }
   return undefined

packages/platform/test/Http/UrlParams.test.ts

@@ -4,22 +4,67 @@ import { Effect } from "effect"
 
 describe("UrlParams", () => {
   describe("makeUrl", () => {
+    it.effect("makes a URL", () =>
+      Effect.gen(function*(_) {
+        const url = yield* _(UrlParams.makeUrl("https://example.com/test", []))
+        assert.strictEqual(url.toString(), "https://example.com/test")
+      }))
+
+    it.effect("supports relative URLs", () =>
+      Effect.gen(function*(_) {
+        const originalLocation = globalThis.location
+
+        globalThis.location = {
+          origin: "https://example.com",
+          pathname: "/path/"
+        } as Location
+        const url = yield* _(UrlParams.makeUrl("test", []))
+        assert.strictEqual(url.toString(), "https://example.com/path/test")
+
+        globalThis.location = originalLocation
+      }))
+
     it.effect("does not throw if `location` is set to `undefined`", () =>
       Effect.gen(function*(_) {
         const originalLocation = globalThis.location
 
         // `globalThis.location` is undefined
         // @ts-expect-error
         globalThis.location = undefined
-        let url = yield* _(UrlParams.makeUrl("http://example.com", []))
-        assert.strictEqual(url.toString(), "http://example.com/")
+        let url = yield* _(UrlParams.makeUrl("https://example.com", []))
+        assert.strictEqual(url.toString(), "https://example.com/")
 
         // `location` is not in globalThis
         // @ts-expect-error
         delete globalThis.location
         url = yield* _(UrlParams.makeUrl("http://example.com", []))
         assert.strictEqual(url.toString(), "http://example.com/")
 
+        globalThis.location = originalLocation
+      }))
+
+    it.effect("does not fail if `location` is partially defined", () =>
+      Effect.gen(function*(_) {
+        const originalLocation = globalThis.location
+
+        globalThis.location = { href: "" } as Location
+        const url1 = yield* _(UrlParams.makeUrl("https://example.com", []))
+        assert.strictEqual(url1.toString(), "https://example.com/")
+
+        globalThis.location = {
+          href: "",
+          origin: "https://example.com"
+        } as unknown as Location
+        const url2 = yield* _(UrlParams.makeUrl("https://example.com", []))
+        assert.strictEqual(url2.toString(), "https://example.com/")
+
+        globalThis.location = {
+          href: "",
+          pathname: "example_path"
+        } as unknown as Location
+        const url3 = yield* _(UrlParams.makeUrl("https://example.com", []))
+        assert.strictEqual(url3.toString(), "https://example.com/")
+
         globalThis.location = originalLocation
       }))
   })