From fdd031604ffa7f6ef3b93ed0505b75277ceaaeac Mon Sep 17 00:00:00 2001 From: Giulio Canti Date: Thu, 13 Feb 2025 15:16:19 +0100 Subject: [PATCH] Preserve function `length` property in `Effect.fn` / `Effect.fnUntraced`, closes #4435 --- .changeset/nice-numbers-hammer.md | 47 ++++++++++++++++++++++++++ packages/effect/src/Effect.ts | 41 +++++++++++++--------- packages/effect/test/Effect/fn.test.ts | 26 ++++++++++++++ packages/effect/vitest.config.ts | 2 +- 4 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 .changeset/nice-numbers-hammer.md diff --git a/.changeset/nice-numbers-hammer.md b/.changeset/nice-numbers-hammer.md new file mode 100644 index 0000000000..7a36406a55 --- /dev/null +++ b/.changeset/nice-numbers-hammer.md @@ -0,0 +1,47 @@ +--- +"effect": patch +--- + +Preserve function `length` property in `Effect.fn` / `Effect.fnUntraced`, closes #4435 + +Previously, functions created with `Effect.fn` and `Effect.fnUntraced` always had a `.length` of `0`, regardless of their actual number of parameters. This has been fixed so that the `length` property correctly reflects the expected number of arguments. + +**Before** + +```ts +import { Effect } from "effect" + +const fn1 = Effect.fn("fn1")(function* (n: number) { + return n +}) + +console.log(fn1.length) +// Output: 0 ❌ (incorrect) + +const fn2 = Effect.fnUntraced(function* (n: number) { + return n +}) + +console.log(fn2.length) +// Output: 0 ❌ (incorrect) +``` + +**After** + +```ts +import { Effect } from "effect" + +const fn1 = Effect.fn("fn1")(function* (n: number) { + return n +}) + +console.log(fn1.length) +// Output: 1 ✅ (correct) + +const fn2 = Effect.fnUntraced(function* (n: number) { + return n +}) + +console.log(fn2.length) +// Output: 1 ✅ (correct) +``` diff --git a/packages/effect/src/Effect.ts b/packages/effect/src/Effect.ts index 77dc89aefb..59ad3980c8 100644 --- a/packages/effect/src/Effect.ts +++ b/packages/effect/src/Effect.ts @@ -13742,7 +13742,7 @@ export const fn: const errorDef = new Error() Error.stackTraceLimit = limit if (typeof nameOrBody !== "string") { - return function(this: any, ...args: Array) { + return defineLength(nameOrBody.length, function(this: any, ...args: Array) { const limit = Error.stackTraceLimit Error.stackTraceLimit = 2 const errorCall = new Error() @@ -13759,12 +13759,12 @@ export const fn: errorDef, errorCall }) - } as any + }) as any } const name = nameOrBody const options = pipeables[0] - return (body: Function, ...pipeables: Array) => { - return function(this: any, ...args: Array) { + return (body: Function, ...pipeables: Array) => + defineLength(body.length, function(this: any, ...args: Array) { const limit = Error.stackTraceLimit Error.stackTraceLimit = 2 const errorCall = new Error() @@ -13779,10 +13779,16 @@ export const fn: errorDef, errorCall }) - } - } + }) } +function defineLength(length: number, fn: F) { + return Object.defineProperty(fn, "length", { + value: length, + configurable: true + }) +} + function fnApply(options: { readonly self: any readonly body: Function @@ -13847,14 +13853,17 @@ function fnApply(options: { * @category Tracing */ export const fnUntraced: fn.Gen = (body: Function, ...pipeables: Array) => - pipeables.length === 0 - ? function(this: any, ...args: Array) { - return core.fromIterator(() => body.apply(this, args)) - } - : function(this: any, ...args: Array) { - let effect = core.fromIterator(() => body.apply(this, args)) - for (const x of pipeables) { - effect = x(effect) + defineLength( + body.length, + pipeables.length === 0 + ? function(this: any, ...args: Array) { + return core.fromIterator(() => body.apply(this, args)) } - return effect - } + : function(this: any, ...args: Array) { + let effect = core.fromIterator(() => body.apply(this, args)) + for (const x of pipeables) { + effect = x(effect) + } + return effect + } + ) diff --git a/packages/effect/test/Effect/fn.test.ts b/packages/effect/test/Effect/fn.test.ts index e4299a7137..a2ec5d6aa5 100644 --- a/packages/effect/test/Effect/fn.test.ts +++ b/packages/effect/test/Effect/fn.test.ts @@ -64,4 +64,30 @@ describe("Effect.fn", () => { assertInstanceOf(cause.right.defect, Error) strictEqual(cause.right.defect.message, "test2") })) + + it("should preserve the function length", () => { + const f = function*(n: number) { + return n + } + const fn1 = Effect.fn("fn1")(f) + strictEqual(fn1.length, 1) + strictEqual(Effect.runSync(fn1(2)), 2) + const fn2 = Effect.fn(f) + strictEqual(fn2.length, 1) + strictEqual(Effect.runSync(fn2(2)), 2) + }) +}) + +describe("Effect.fnUntraced", () => { + it("should preserve the function length", () => { + const f = function*(n: number) { + return n + } + const fn1 = Effect.fnUntraced(f) + strictEqual(fn1.length, 1) + strictEqual(Effect.runSync(fn1(2)), 2) + const fn2 = Effect.fnUntraced(f, (x) => x) + strictEqual(fn2.length, 1) + strictEqual(Effect.runSync(fn2(2)), 2) + }) }) diff --git a/packages/effect/vitest.config.ts b/packages/effect/vitest.config.ts index c33fc3f516..ba4df2d008 100644 --- a/packages/effect/vitest.config.ts +++ b/packages/effect/vitest.config.ts @@ -5,7 +5,7 @@ const config: ViteUserConfig = { test: { coverage: { reporter: ["html"], - include: ["src/Match.ts", "src/internal/matcher.ts"] + include: ["src/Effect.ts"] } } }