From 533bf87f555ce218980be21447fed79d43747370 Mon Sep 17 00:00:00 2001 From: Roc Wong Date: Sun, 14 Apr 2024 23:42:13 +1200 Subject: [PATCH 1/5] Fix HTTP Client's compatibility with Deno due to globalThis.location. --- .changeset/stale-bugs-roll.md | 5 ++++ packages/platform/src/Http/UrlParams.ts | 7 +++-- packages/platform/test/Http/UrlParams.test.ts | 26 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 .changeset/stale-bugs-roll.md create mode 100644 packages/platform/test/Http/UrlParams.test.ts diff --git a/.changeset/stale-bugs-roll.md b/.changeset/stale-bugs-roll.md new file mode 100644 index 00000000000..7337cd0f8a2 --- /dev/null +++ b/.changeset/stale-bugs-roll.md @@ -0,0 +1,5 @@ +--- +"@effect/platform": patch +--- + +Fix HTTP Client's compatibility with Deno due to globalThis.location. diff --git a/packages/platform/src/Http/UrlParams.ts b/packages/platform/src/Http/UrlParams.ts index 8ee462c6ec8..45ad3141202 100644 --- a/packages/platform/src/Http/UrlParams.ts +++ b/packages/platform/src/Http/UrlParams.ts @@ -207,8 +207,11 @@ export const makeUrl = (url: string, params: UrlParams, onError: (e: unknown) catch: onError }) -const baseUrl = (): string | undefined => { - if ("location" in globalThis) { +export const baseUrl = (): string | undefined => { + // Need to both "in" and "undefined" for location to support Deno. + // As by default, Deno has "globalThis.location" defined but with value "undefined". + // See https://docs.deno.com/runtime/manual/runtime/location_api + if ("location" in globalThis && globalThis.location !== undefined) { return location.origin + location.pathname } return undefined diff --git a/packages/platform/test/Http/UrlParams.test.ts b/packages/platform/test/Http/UrlParams.test.ts new file mode 100644 index 00000000000..df5cc4dc00f --- /dev/null +++ b/packages/platform/test/Http/UrlParams.test.ts @@ -0,0 +1,26 @@ +import * as UrlParams from "@effect/platform/Http/UrlParams" +import { assert, describe, it } from "vitest" + +describe("UrlParams", () => { + describe("baseUrl", () => { + it("should return undefined when `location` is not in `globalThis` or `globalThis.location` is undefined", () => { + const originalLocation = globalThis.location + + // `globalThis.location` is undefined + // @ts-expect-error + globalThis.location = undefined + assert.strictEqual("location" in globalThis, true) + assert.strictEqual(globalThis.location, undefined) + assert.strictEqual(UrlParams.baseUrl(), undefined) + + // `location` is not in globalThis + // @ts-expect-error + delete globalThis.location + assert.strictEqual("location" in globalThis, false) + assert.strictEqual(globalThis.location, undefined) + assert.strictEqual(UrlParams.baseUrl(), undefined) + + globalThis.location = originalLocation + }) + }) +}) From 8408da808a40584651b0b4dab97102aab3b48fc2 Mon Sep 17 00:00:00 2001 From: Roc Wong Date: Mon, 15 Apr 2024 00:12:38 +1200 Subject: [PATCH 2/5] Revise the comment in baseUrl() --- packages/platform/src/Http/UrlParams.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/platform/src/Http/UrlParams.ts b/packages/platform/src/Http/UrlParams.ts index 45ad3141202..64cf7a56ba4 100644 --- a/packages/platform/src/Http/UrlParams.ts +++ b/packages/platform/src/Http/UrlParams.ts @@ -208,8 +208,8 @@ export const makeUrl = (url: string, params: UrlParams, onError: (e: unknown) }) export const baseUrl = (): string | undefined => { - // Need to both "in" and "undefined" for location to support Deno. - // As by default, Deno has "globalThis.location" defined but with value "undefined". + // Need to check both "in" and "undefined" for location to support Deno. + // As Deno has "globalThis.location" defined but with value "undefined" by default. // See https://docs.deno.com/runtime/manual/runtime/location_api if ("location" in globalThis && globalThis.location !== undefined) { return location.origin + location.pathname From 9b3567c99793c2a302328d0ea3807817833a5e12 Mon Sep 17 00:00:00 2001 From: Roc Wong Date: Mon, 15 Apr 2024 00:27:16 +1200 Subject: [PATCH 3/5] Add docs --- packages/platform/src/Http/UrlParams.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/platform/src/Http/UrlParams.ts b/packages/platform/src/Http/UrlParams.ts index 64cf7a56ba4..3c58b717aa7 100644 --- a/packages/platform/src/Http/UrlParams.ts +++ b/packages/platform/src/Http/UrlParams.ts @@ -207,6 +207,12 @@ export const makeUrl = (url: string, params: UrlParams, onError: (e: unknown) catch: onError }) +/** + * Get the base URL out of `globalThis.location`. + * + * @since 1.0.0 + * @category utils + */ export const baseUrl = (): string | undefined => { // Need to check both "in" and "undefined" for location to support Deno. // As Deno has "globalThis.location" defined but with value "undefined" by default. From e5d8128f20ee02a09121c5928f5822e9c9700714 Mon Sep 17 00:00:00 2001 From: Roc Wong Date: Mon, 15 Apr 2024 01:05:28 +1200 Subject: [PATCH 4/5] Mark baseUrl() as internal. --- packages/platform/src/Http/UrlParams.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/platform/src/Http/UrlParams.ts b/packages/platform/src/Http/UrlParams.ts index 3c58b717aa7..5b84325bf72 100644 --- a/packages/platform/src/Http/UrlParams.ts +++ b/packages/platform/src/Http/UrlParams.ts @@ -210,8 +210,7 @@ export const makeUrl = (url: string, params: UrlParams, onError: (e: unknown) /** * Get the base URL out of `globalThis.location`. * - * @since 1.0.0 - * @category utils + * @internal */ export const baseUrl = (): string | undefined => { // Need to check both "in" and "undefined" for location to support Deno. From 98ff8b580e9237704e8d0d3645ac5112dd049e6a Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 16 Apr 2024 14:50:07 +1200 Subject: [PATCH 5/5] wip --- .changeset/stale-bugs-roll.md | 2 +- packages/platform/src/Http/UrlParams.ts | 10 +----- packages/platform/test/Http/UrlParams.test.ts | 36 +++++++++---------- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/.changeset/stale-bugs-roll.md b/.changeset/stale-bugs-roll.md index 7337cd0f8a2..862ab7d8c7f 100644 --- a/.changeset/stale-bugs-roll.md +++ b/.changeset/stale-bugs-roll.md @@ -2,4 +2,4 @@ "@effect/platform": patch --- -Fix HTTP Client's compatibility with Deno due to globalThis.location. +Fix UrlParams.makeUrl when globalThis.location is set to `undefined` diff --git a/packages/platform/src/Http/UrlParams.ts b/packages/platform/src/Http/UrlParams.ts index 5b84325bf72..615558c4bdf 100644 --- a/packages/platform/src/Http/UrlParams.ts +++ b/packages/platform/src/Http/UrlParams.ts @@ -207,15 +207,7 @@ export const makeUrl = (url: string, params: UrlParams, onError: (e: unknown) catch: onError }) -/** - * Get the base URL out of `globalThis.location`. - * - * @internal - */ -export const baseUrl = (): string | undefined => { - // Need to check both "in" and "undefined" for location to support Deno. - // As Deno has "globalThis.location" defined but with value "undefined" by default. - // See https://docs.deno.com/runtime/manual/runtime/location_api +const baseUrl = (): string | undefined => { if ("location" in globalThis && globalThis.location !== undefined) { return location.origin + location.pathname } diff --git a/packages/platform/test/Http/UrlParams.test.ts b/packages/platform/test/Http/UrlParams.test.ts index df5cc4dc00f..243f3b609ae 100644 --- a/packages/platform/test/Http/UrlParams.test.ts +++ b/packages/platform/test/Http/UrlParams.test.ts @@ -1,26 +1,26 @@ import * as UrlParams from "@effect/platform/Http/UrlParams" -import { assert, describe, it } from "vitest" +import { assert, describe, it } from "@effect/vitest" +import { Effect } from "effect" describe("UrlParams", () => { - describe("baseUrl", () => { - it("should return undefined when `location` is not in `globalThis` or `globalThis.location` is undefined", () => { - const originalLocation = globalThis.location + describe("makeUrl", () => { + 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 - assert.strictEqual("location" in globalThis, true) - assert.strictEqual(globalThis.location, undefined) - assert.strictEqual(UrlParams.baseUrl(), undefined) + // `globalThis.location` is undefined + // @ts-expect-error + globalThis.location = undefined + let url = yield* _(UrlParams.makeUrl("http://example.com", [], () => "error")) + assert.strictEqual(url.toString(), "http://example.com/") - // `location` is not in globalThis - // @ts-expect-error - delete globalThis.location - assert.strictEqual("location" in globalThis, false) - assert.strictEqual(globalThis.location, undefined) - assert.strictEqual(UrlParams.baseUrl(), undefined) + // `location` is not in globalThis + // @ts-expect-error + delete globalThis.location + url = yield* _(UrlParams.makeUrl("http://example.com", [], () => "error")) + assert.strictEqual(url.toString(), "http://example.com/") - globalThis.location = originalLocation - }) + globalThis.location = originalLocation + })) }) })