From 8b25760c2b59bb24d272120c5993a49464ba3842 Mon Sep 17 00:00:00 2001 From: leovs09 Date: Thu, 2 Apr 2026 00:10:54 +0200 Subject: [PATCH 1/3] feat: add async hook support --- README.md | 58 +++++++++ src/effect-on-method.ts | 107 ++++++++++------ src/hook.types.ts | 30 +++-- tests/EffectOnMethod.spec.ts | 229 +++++++++++++++++++++++++++++++++++ 4 files changed, 377 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index f5f85bc..a070729 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,64 @@ class Service { } ``` +### Async hooks + +When the decorated method returns a `Promise`, all hooks may optionally return a `Promise` as well. `onReturn` receives the **unwrapped** resolved value, and the library automatically chains the returned promise so async hooks execute in the correct order. + +```typescript +import { Effect } from 'base-decorators'; + +const DelayedLog = () => Effect({ + // Returning a Promise delays the original method + onInvoke: async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + console.log('starting...'); + }, + + // `result` is the unwrapped string, not Promise + onReturn: async ({ result }) => { + console.log('done:', result); + return `${result} (modified)`; + }, +}); + +class Service { + @DelayedLog() + async greet(name: string): Promise { + return `Hello, ${name}`; + } +} + +const service = new Service(); +const value = await service.greet('World'); +// "Hello, World (modified)" +``` + +Async `onError` and `finally` hooks work the same way. Here, an async `finally` flushes a log buffer after every call: + +```typescript +import { Effect } from 'base-decorators'; + +const buffer: string[] = []; + +const FlushAfterCall = () => Effect({ + onInvoke: ({ args }) => { + buffer.push('invoke: ' + String(args)); + }, + finally: async () => { + await fetch('/log', { method: 'POST', body: JSON.stringify(buffer) }); + buffer.length = 0; + }, +}); + +class Worker { + @FlushAfterCall() + async doWork(id: number): Promise { + // async work + } +} +``` + ### Class and Method decorators `Effect` and all hook decorators can be used on both classes and methods out of the box. diff --git a/src/effect-on-method.ts b/src/effect-on-method.ts index f1479b5..5e0f59f 100644 --- a/src/effect-on-method.ts +++ b/src/effect-on-method.ts @@ -1,5 +1,5 @@ import { setMeta, SYM_META_PROP } from './set-meta.decorator'; -import type { EffectHooks, HookContext, HooksOrFactory } from './hook.types'; +import type { EffectHooks, HookContext, HooksOrFactory, MaybeAsync, UnwrapPromise } from './hook.types'; import { getParameterNames } from './getParameterNames'; /** @@ -75,23 +75,30 @@ export const EffectOnMethod = ( const hooks = resolveHooks(hooksOrFactory, context); - if (hooks.onInvoke) { - hooks.onInvoke(context); - } + const runMethod = (): unknown => { + try { + const result = originalMethod.apply(this, args); - try { - const result = originalMethod.apply(this, args); + if (result instanceof Promise) { + return chainAsyncHooks(result as Promise>, context, hooks); + } - if (result instanceof Promise) { - return chainAsyncHooks(result as Promise, context, hooks); + // If an error occurs in the onReturn hook, `finally` will be triggered 2 times. + // Intentional architecture choice due to TS limitations + return handleSyncSuccess(result as R, context, hooks); + } catch (error: unknown) { + return handleSyncError(error, context, hooks); } + }; - // If an error occurs in the onReturn hook, `finally` will be triggered 2 times. - // Intentional architecture choice due to TS limitations - return handleSyncSuccess(result as R, context, hooks); - } catch (error: unknown) { - return handleSyncError(error, context, hooks); + if (hooks.onInvoke) { + const invokeResult = hooks.onInvoke(context); + if (invokeResult instanceof Promise) { + return invokeResult.then(() => runMethod()); + } } + + return runMethod(); }; copySymMeta(originalMethod, wrapped); @@ -189,19 +196,18 @@ const resolveHooks = ( }; /** - * Handles the synchronous success + finally path. + * Runs a callback and ensures `finally` hook is invoked afterwards. * - * Separated to keep the main decorator body within line limits. + * Extracted to avoid duplicating the `try...finally` wrapper in both + * success and error handlers. */ -const handleSyncSuccess = ( - result: R, +const runWithFinally = ( + fn: () => MaybeAsync, context: HookContext, hooks: EffectHooks, -): R => { +): MaybeAsync => { try { - return hooks.onReturn - ? hooks.onReturn({ ...context, result }) - : result; + return fn(); } finally { if (hooks.finally) { hooks.finally(context); @@ -209,6 +215,23 @@ const handleSyncSuccess = ( } }; +/** + * Handles the synchronous success + finally path. + * + * Separated to keep the main decorator body within line limits. + */ +const handleSyncSuccess = ( + result: R, + context: HookContext, + hooks: EffectHooks, +): MaybeAsync => { + return runWithFinally(() => { + return hooks.onReturn + ? (hooks.onReturn({ ...context, result: result as UnwrapPromise }) as MaybeAsync) + : (result as MaybeAsync); + }, context, hooks); +}; + /** * Handles the synchronous error + finally path. * @@ -218,18 +241,14 @@ const handleSyncError = ( error: unknown, context: HookContext, hooks: EffectHooks, -): R => { - try { +): MaybeAsync => { + return runWithFinally(() => { if (hooks.onError) { - return hooks.onError({ ...context, error }); + return hooks.onError({ ...context, error }) as MaybeAsync; } throw error; - } finally { - if (hooks.finally) { - hooks.finally(context); - } - } + }, context, hooks); }; /** @@ -240,28 +259,42 @@ const handleSyncError = ( * and `finally` always fires last. */ const chainAsyncHooks = ( - promise: Promise, + promise: Promise>, context: HookContext, hooks: EffectHooks, -): Promise => { +): Promise> => { let chained = promise; if (hooks.onReturn) { chained = chained.then((value) => { - return hooks.onReturn!({ ...context, result: value }); - }); + return hooks.onReturn!({ ...context, result: value as UnwrapPromise }); + }) as Promise>; } if (hooks.onError) { chained = chained.catch((error: unknown) => { return hooks.onError!({ ...context, error }); - }); + }) as Promise>; } if (hooks.finally) { - chained = chained.finally(() => { - hooks.finally!(context); - }); + const finallyHook = hooks.finally; + chained = chained.then( + (value) => { + const finallyResult = finallyHook(context); + if (finallyResult instanceof Promise) { + return finallyResult.then(() => value); + } + return value; + }, + (error) => { + const finallyResult = finallyHook(context); + if (finallyResult instanceof Promise) { + return finallyResult.then(() => { throw error; }); + } + throw error; + }, + ) as Promise>; } return chained; diff --git a/src/hook.types.ts b/src/hook.types.ts index ef66156..dcb6593 100644 --- a/src/hook.types.ts +++ b/src/hook.types.ts @@ -26,10 +26,16 @@ export interface HookContext { descriptor: PropertyDescriptor; } +/** Extracts the resolved type from a Promise, or returns the type itself. */ +export type UnwrapPromise = T extends Promise ? U : T; + +/** Allows a Promise return only when the original type is already a Promise. */ +export type MaybeAsync = T extends Promise ? U | Promise : T; + /** Context for the onReturn hook, adding the method result. */ export interface OnReturnContext extends HookContext { - /** The value returned by the original method. */ - result: R; + /** The value returned by the original method (unwrapped if it was a Promise). */ + result: UnwrapPromise; } /** Context for the onError hook, adding the thrown error. */ @@ -40,35 +46,39 @@ export interface OnErrorContext extends HookContext { /** * Hook fired before the original method executes. - * @typeParam R - The return type of the decorated method (unused, for consistency) + * @typeParam R - The return type of the decorated method. When it extends Promise, + * the hook may also return a Promise. */ export type OnInvokeHookType = ( context: HookContext, -) => void; +) => R extends Promise ? void | Promise : void; /** * Hook fired after a successful return. Its return value replaces the method result. - * @typeParam R - The return type of the decorated method + * @typeParam R - The return type of the decorated method. When it extends Promise, + * the hook may also return a Promise of the resolved type. */ export type OnReturnHookType = ( context: OnReturnContext, -) => R; +) => MaybeAsync; /** * Hook fired when the method throws. May return a recovery value or re-throw. - * @typeParam R - The return type of the decorated method + * @typeParam R - The return type of the decorated method. When it extends Promise, + * the hook may also return a Promise of the resolved type. */ export type OnErrorHookType = ( context: OnErrorContext, -) => R; +) => MaybeAsync; /** * Hook fired after both success and error paths, regardless of outcome. - * @typeParam R - The return type of the decorated method (unused, for consistency) + * @typeParam R - The return type of the decorated method. When it extends Promise, + * the hook may also return a Promise. */ export type FinallyHookType = ( context: HookContext, -) => void; +) => R extends Promise ? void | Promise : void; /** * Lifecycle hooks for method decoration via Effect-based decorators. diff --git a/tests/EffectOnMethod.spec.ts b/tests/EffectOnMethod.spec.ts index 3b56135..d9ecb74 100644 --- a/tests/EffectOnMethod.spec.ts +++ b/tests/EffectOnMethod.spec.ts @@ -693,6 +693,235 @@ describe('EffectOnMethod', () => { }); }); + describe('async hooks support', () => { + it('should wait for async onInvoke before executing async method', async () => { + const callOrder: string[] = []; + + const hooks: EffectHooks> = { + onInvoke: async () => { + callOrder.push('onInvoke-start'); + await new Promise((resolve) => setTimeout(resolve, 10)); + callOrder.push('onInvoke-end'); + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async greet(name: string) { + callOrder.push('original'); + return `hello ${name}`; + } + } + + const service = new TestService(); + const result = await service.greet('world'); + + expect(result).toBe('hello world'); + expect(callOrder).toEqual(['onInvoke-start', 'onInvoke-end', 'original']); + }); + + it('should wait for async onReturn and replace async method result', async () => { + const hooks: EffectHooks> = { + onReturn: async ({ result }) => { + await new Promise((resolve) => setTimeout(resolve, 10)); + return `${result}-async`; + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async greet(name: string) { + return `hello ${name}`; + } + } + + const service = new TestService(); + const result = await service.greet('world'); + + expect(result).toBe('hello world-async'); + }); + + it('should allow async onError to return recovery value', async () => { + const hooks: EffectHooks> = { + onError: async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + return 'recovered-async'; + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async failing() { + throw new Error('fail'); + } + } + + const service = new TestService(); + const result = await service.failing(); + + expect(result).toBe('recovered-async'); + }); + + it('should wait for async finally on success', async () => { + const callOrder: string[] = []; + + const hooks: EffectHooks> = { + finally: async () => { + callOrder.push('finally-start'); + await new Promise((resolve) => setTimeout(resolve, 10)); + callOrder.push('finally-end'); + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async greet(name: string) { + callOrder.push('original'); + return `hello ${name}`; + } + } + + const service = new TestService(); + const result = await service.greet('world'); + + expect(result).toBe('hello world'); + expect(callOrder).toEqual(['original', 'finally-start', 'finally-end']); + }); + + it('should wait for async finally on error', async () => { + const callOrder: string[] = []; + + const hooks: EffectHooks> = { + finally: async () => { + callOrder.push('finally-start'); + await new Promise((resolve) => setTimeout(resolve, 10)); + callOrder.push('finally-end'); + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async failing() { + callOrder.push('original'); + throw new Error('fail'); + } + } + + const service = new TestService(); + await expect(service.failing()).rejects.toThrow('fail'); + expect(callOrder).toEqual(['original', 'finally-start', 'finally-end']); + }); + + it('should trigger finally when async onError re-throws on rejection path', async () => { + const callOrder: string[] = []; + const testError = new Error('original error'); + + const hooks: EffectHooks> = { + onError: async ({ error }) => { + callOrder.push('onError-start'); + await new Promise((resolve) => setTimeout(resolve, 10)); + callOrder.push('onError-end'); + throw error; + }, + finally: () => { + callOrder.push('finally'); + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async failing() { + callOrder.push('original'); + throw testError; + } + } + + const service = new TestService(); + await expect(service.failing()).rejects.toThrow(testError); + expect(callOrder).toEqual(['original', 'onError-start', 'onError-end', 'finally']); + }); + + it('should execute all async hooks in correct order', async () => { + const callOrder: string[] = []; + + const hooks: EffectHooks> = { + onInvoke: async () => { + callOrder.push('onInvoke'); + }, + onReturn: async ({ result }) => { + callOrder.push('onReturn'); + return result; + }, + onError: async ({ error }) => { + callOrder.push('onError'); + throw error; + }, + finally: async () => { + callOrder.push('finally'); + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async greet(name: string) { + callOrder.push('original'); + return `hello ${name}`; + } + } + + const service = new TestService(); + await service.greet('world'); + + expect(callOrder).toEqual(['onInvoke', 'original', 'onReturn', 'finally']); + }); + + it('should propagate async onInvoke rejection without calling method', async () => { + const invokeError = new Error('async onInvoke failed'); + const callOrder: string[] = []; + + const hooks: EffectHooks> = { + onInvoke: async () => { + callOrder.push('onInvoke'); + throw invokeError; + }, + }; + + class TestService { + @EffectOnMethod(hooks) + async greet() { + callOrder.push('original'); + return 'hello'; + } + } + + const service = new TestService(); + await expect(service.greet()).rejects.toThrow(invokeError); + expect(callOrder).toEqual(['onInvoke']); + }); + + it('should turn sync method into Promise when onInvoke is async', async () => { + const hooks: EffectHooks = { + onInvoke: async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + }, + }; + + class TestService { + @EffectOnMethod(hooks) + greet() { + return 'hello'; + } + } + + const service = new TestService(); + const result = service.greet(); + + expect(result).toBeInstanceOf(Promise); + expect(await result).toBe('hello'); + }); + + }); + describe('exclusionKey parameter', () => { it('should mark method with EFFECT_APPLIED_KEY by default', () => { class TestService { From ebd783ae174c70053a84ddd0e89abcd2fa0dd554 Mon Sep 17 00:00:00 2001 From: leovs09 Date: Tue, 7 Apr 2026 01:04:28 +0200 Subject: [PATCH 2/3] refactor: simplify effect-on-method logic --- ...preserve-type-safety-during-refactoring.md | 55 ++++++ src/effect-on-method.ts | 161 ++++++------------ 2 files changed, 105 insertions(+), 111 deletions(-) create mode 100644 .claude/rules/preserve-type-safety-during-refactoring.md diff --git a/.claude/rules/preserve-type-safety-during-refactoring.md b/.claude/rules/preserve-type-safety-during-refactoring.md new file mode 100644 index 0000000..6b43369 --- /dev/null +++ b/.claude/rules/preserve-type-safety-during-refactoring.md @@ -0,0 +1,55 @@ +--- +title: Preserve Type Safety During Refactoring +impact: MEDIUM +paths: + - "**/*.ts" +--- + +# Preserve Type Safety During Refactoring + +Count `as` type assertions before and after any refactoring. The refactored code must have equal or fewer assertions than the original. Increasing assertion count indicates the new abstractions fight the type system rather than working with it. + +## Incorrect + +Merging two typed functions into one with an `unknown` parameter, requiring new `as` casts at every usage site. + +```typescript +// Original: 2 functions, 0 extra assertions +const handleSuccess = (result: R, hooks: Hooks): R => { + return hooks.onReturn ? hooks.onReturn(result) : result; +}; +const handleError = (error: unknown, hooks: Hooks): R => { + if (hooks.onError) return hooks.onError(error); + throw error; +}; + +// Refactored: 1 function, but 3 new assertions +const settle = (succeeded: boolean, value: unknown, hooks: Hooks): R => { + if (succeeded) { + return hooks.onReturn + ? (hooks.onReturn(value as R) as R) // 2 new assertions + : (value as R); // 1 new assertion + } + // ... +}; +``` + +## Correct + +Use overloads or a discriminated union to preserve type information without additional casts. + +```typescript +type Outcome = + | { succeeded: true; value: R } + | { succeeded: false; error: unknown }; + +const settle = (outcome: Outcome, hooks: Hooks): R => { + if (outcome.succeeded) { + return hooks.onReturn + ? hooks.onReturn(outcome.value) // no cast needed, value is R + : outcome.value; + } + if (hooks.onError) return hooks.onError(outcome.error); + throw outcome.error; +}; +``` diff --git a/src/effect-on-method.ts b/src/effect-on-method.ts index 5e0f59f..b27b718 100644 --- a/src/effect-on-method.ts +++ b/src/effect-on-method.ts @@ -1,5 +1,5 @@ import { setMeta, SYM_META_PROP } from './set-meta.decorator'; -import type { EffectHooks, HookContext, HooksOrFactory, MaybeAsync, UnwrapPromise } from './hook.types'; +import type { EffectHooks, HookContext, HooksOrFactory, UnwrapPromise } from './hook.types'; import { getParameterNames } from './getParameterNames'; /** @@ -75,30 +75,46 @@ export const EffectOnMethod = ( const hooks = resolveHooks(hooksOrFactory, context); - const runMethod = (): unknown => { + // Executes the original method and applies sync/async lifecycle hooks. + // Kept as a closure so async onInvoke can defer execution via .then(). + // finally kept inline to avoid double-calling when onReturn or onError throw. + const executeMethod = (): unknown => { try { const result = originalMethod.apply(this, args); if (result instanceof Promise) { - return chainAsyncHooks(result as Promise>, context, hooks); + return chainAsyncHooks(result, context, hooks); } - // If an error occurs in the onReturn hook, `finally` will be triggered 2 times. - // Intentional architecture choice due to TS limitations - return handleSyncSuccess(result as R, context, hooks); + try { + return hooks.onReturn + ? hooks.onReturn({ ...context, result: result as UnwrapPromise }) + : result; + } finally { + hooks.finally?.(context); + } } catch (error: unknown) { - return handleSyncError(error, context, hooks); - } + try { + if (hooks.onError) { + return hooks.onError({ ...context, error }); + } + + throw error; + } finally { + hooks.finally?.(context); + } + } }; if (hooks.onInvoke) { const invokeResult = hooks.onInvoke(context); + if (invokeResult instanceof Promise) { - return invokeResult.then(() => runMethod()); + return invokeResult.then(executeMethod); } } - return runMethod(); + return executeMethod(); }; copySymMeta(originalMethod, wrapped); @@ -119,7 +135,7 @@ export const EffectOnMethod = ( * * @param parameterNames - Array of parameter names * @param args - Array of argument values - * @returns Object mapping parameter names to values + * @returns Object mapping parameter names to values, or undefined when empty * * @example * buildArgsObject(['id', 'name'], [1, 'John']) @@ -127,7 +143,10 @@ export const EffectOnMethod = ( * * @internal */ -export const buildArgsObject = (parameterNames: string[], args: unknown[]): Record | undefined => { +export const buildArgsObject = ( + parameterNames: string[], + args: unknown[], +): Record | undefined => { if (args.length === 0 && parameterNames.length === 0) { return undefined; } @@ -151,10 +170,7 @@ export const buildArgsObject = (parameterNames: string[], args: unknown[]): Reco * or `@NoLog`) must survive on the new wrapper so downstream consumers * (like `EffectOnClass`) can still read it. */ -const copySymMeta = ( - source: Function, - target: Function, -): void => { +const copySymMeta = (source: Function, target: Function): void => { const sourceRecord = source as unknown as Record; const sourceMap = sourceRecord[SYM_META_PROP] as | Map @@ -174,9 +190,7 @@ const copySymMeta = ( } const targetMap = targetRecord[SYM_META_PROP] as Map; - sourceMap.forEach((value, key) => { - targetMap.set(key, value); - }); + sourceMap.forEach((value, key) => targetMap.set(key, value)); }; /** @@ -196,106 +210,31 @@ const resolveHooks = ( }; /** - * Runs a callback and ensures `finally` hook is invoked afterwards. + * Applies lifecycle hooks (onReturn, onError, finally) to an async method result. * - * Extracted to avoid duplicating the `try...finally` wrapper in both - * success and error handlers. + * Uses async/await with try/catch/finally so that onReturn fires after + * resolution, onError fires after rejection, and finally always fires last. */ -const runWithFinally = ( - fn: () => MaybeAsync, +const chainAsyncHooks = async ( + promise: Promise, context: HookContext, hooks: EffectHooks, -): MaybeAsync => { +): Promise => { try { - return fn(); - } finally { - if (hooks.finally) { - hooks.finally(context); - } - } -}; + const value = await promise; -/** - * Handles the synchronous success + finally path. - * - * Separated to keep the main decorator body within line limits. - */ -const handleSyncSuccess = ( - result: R, - context: HookContext, - hooks: EffectHooks, -): MaybeAsync => { - return runWithFinally(() => { return hooks.onReturn - ? (hooks.onReturn({ ...context, result: result as UnwrapPromise }) as MaybeAsync) - : (result as MaybeAsync); - }, context, hooks); -}; - -/** - * Handles the synchronous error + finally path. - * - * Separated to keep the main decorator body within line limits. - */ -const handleSyncError = ( - error: unknown, - context: HookContext, - hooks: EffectHooks, -): MaybeAsync => { - return runWithFinally(() => { + ? await hooks.onReturn({ ...context, result: value as UnwrapPromise }) + : value; + } catch (error: unknown) { if (hooks.onError) { - return hooks.onError({ ...context, error }) as MaybeAsync; + return await hooks.onError({ ...context, error }); } - + throw error; - }, context, hooks); -}; - -/** - * Chains promise lifecycle hooks for asynchronous method execution. - * - * Uses `.then` / `.catch` / `.finally` on the returned promise so that - * `onReturn` fires after resolution, `onError` fires after rejection, - * and `finally` always fires last. - */ -const chainAsyncHooks = ( - promise: Promise>, - context: HookContext, - hooks: EffectHooks, -): Promise> => { - let chained = promise; - - if (hooks.onReturn) { - chained = chained.then((value) => { - return hooks.onReturn!({ ...context, result: value as UnwrapPromise }); - }) as Promise>; - } - - if (hooks.onError) { - chained = chained.catch((error: unknown) => { - return hooks.onError!({ ...context, error }); - }) as Promise>; - } - - if (hooks.finally) { - const finallyHook = hooks.finally; - chained = chained.then( - (value) => { - const finallyResult = finallyHook(context); - if (finallyResult instanceof Promise) { - return finallyResult.then(() => value); - } - return value; - }, - (error) => { - const finallyResult = finallyHook(context); - if (finallyResult instanceof Promise) { - return finallyResult.then(() => { throw error; }); - } - throw error; - }, - ) as Promise>; + } finally { + if (hooks.finally) { + await hooks.finally(context); + } } - - return chained; -}; \ No newline at end of file +}; From c0499a141d063099e7fa7471a9cff4d42dbad413 Mon Sep 17 00:00:00 2001 From: leovs09 Date: Tue, 7 Apr 2026 01:44:09 +0200 Subject: [PATCH 3/3] refactor: extract logic from effect on method decorator --- src/effect-on-method.ts | 152 +++++++++++++++++----------- tests/effect-on-method-base.spec.ts | 151 +++++++++++++++++++++++++++ 2 files changed, 245 insertions(+), 58 deletions(-) create mode 100644 tests/effect-on-method-base.spec.ts diff --git a/src/effect-on-method.ts b/src/effect-on-method.ts index b27b718..bbeb3d1 100644 --- a/src/effect-on-method.ts +++ b/src/effect-on-method.ts @@ -59,63 +59,13 @@ export const EffectOnMethod = ( // Extract parameter names at decoration time (once, not per-call) const parameterNames = getParameterNames(originalMethod); - const wrapped = function (this: object, ...args: unknown[]): unknown { - const argsObject = buildArgsObject(parameterNames, args); - const className = (this.constructor as { name: string }).name ?? ''; - - const context: HookContext = { - argsObject, - args, - target: this, - propertyKey, - descriptor, - parameterNames, - className, - }; - - const hooks = resolveHooks(hooksOrFactory, context); - - // Executes the original method and applies sync/async lifecycle hooks. - // Kept as a closure so async onInvoke can defer execution via .then(). - // finally kept inline to avoid double-calling when onReturn or onError throw. - const executeMethod = (): unknown => { - try { - const result = originalMethod.apply(this, args); - - if (result instanceof Promise) { - return chainAsyncHooks(result, context, hooks); - } - - try { - return hooks.onReturn - ? hooks.onReturn({ ...context, result: result as UnwrapPromise }) - : result; - } finally { - hooks.finally?.(context); - } - } catch (error: unknown) { - try { - if (hooks.onError) { - return hooks.onError({ ...context, error }); - } - - throw error; - } finally { - hooks.finally?.(context); - } - } - }; - - if (hooks.onInvoke) { - const invokeResult = hooks.onInvoke(context); - - if (invokeResult instanceof Promise) { - return invokeResult.then(executeMethod); - } - } - - return executeMethod(); - }; + const wrapped = wrapFunction( + originalMethod, + parameterNames, + propertyKey, + descriptor, + hooksOrFactory, + ); copySymMeta(originalMethod, wrapped); @@ -127,6 +77,8 @@ export const EffectOnMethod = ( }; }; + + /** * Builds an object mapping parameter names to their values. * @@ -162,6 +114,89 @@ export const buildArgsObject = ( return argsObject; }; +/** + * Builds the per-method wrapper used by {@link EffectOnMethod}: constructs + * {@link HookContext}, resolves hooks, and wires `onInvoke` plus execution. + */ +export const wrapFunction = ( + originalMethod: (...args: unknown[]) => unknown, + parameterNames: string[], + propertyKey: string | symbol, + descriptor: PropertyDescriptor, + hooksOrFactory: HooksOrFactory, +): ((this: object, ...args: unknown[]) => unknown) => + function (this: object, ...args: unknown[]): unknown { + const argsObject = buildArgsObject(parameterNames, args); + const className = (this.constructor as { name: string }).name ?? ''; + + const context: HookContext = { + argsObject, + args, + target: this, + propertyKey, + descriptor, + parameterNames, + className, + }; + + const hooks = resolveHooks(hooksOrFactory, context); + + const executeMethod = attachHooks(originalMethod, this, args, context, hooks); + + if (hooks.onInvoke) { + const invokeResult = hooks.onInvoke(context); + + if (invokeResult instanceof Promise) { + return invokeResult.then(executeMethod); + } + } + + return executeMethod(); + }; + +/** + * Returns a thunk that runs the original method and applies sync/async lifecycle hooks. + * + * Kept as a thunk so async `onInvoke` can defer execution via `.then()`. + * `finally` is applied inline on sync paths to avoid double-calling when + * `onReturn` or `onError` throw. + */ +export const attachHooks = ( + originalMethod: (...args: unknown[]) => unknown, + thisArg: object, + args: unknown[], + context: HookContext, + hooks: EffectHooks, +): (() => unknown) => () => { + try { + const result = originalMethod.apply(thisArg, args); + + if (result instanceof Promise) { + return chainAsyncHooks(result, context, hooks); + } + + try { + return hooks.onReturn + ? hooks.onReturn({ ...context, result: result as UnwrapPromise }) + : result; + } finally { + hooks.finally?.(context); + } + } catch (error: unknown) { + try { + if (hooks.onError) { + return hooks.onError({ ...context, error }); + } + + throw error; + } finally { + hooks.finally?.(context); + } + } +}; + + + /** * Copies the `_symMeta` Map from the original function to a new function. * @@ -230,7 +265,7 @@ const chainAsyncHooks = async ( if (hooks.onError) { return await hooks.onError({ ...context, error }); } - + throw error; } finally { if (hooks.finally) { @@ -238,3 +273,4 @@ const chainAsyncHooks = async ( } } }; + diff --git a/tests/effect-on-method-base.spec.ts b/tests/effect-on-method-base.spec.ts new file mode 100644 index 0000000..1af4a5a --- /dev/null +++ b/tests/effect-on-method-base.spec.ts @@ -0,0 +1,151 @@ +import { describe, it, expect, vi } from 'vitest'; + +import { attachHooks, wrapFunction } from '../src/effect-on-method'; +import type { EffectHooks, HookContext, OnReturnContext } from '../src/hook.types'; + +const makeDescriptor = (fn: (...args: unknown[]) => unknown): PropertyDescriptor => ({ + value: fn, + writable: true, + enumerable: true, + configurable: true, +}); + +const makeContext = (overrides: Partial = {}): HookContext => { + const original = () => undefined; + return { + args: [], + argsObject: undefined, + target: {}, + propertyKey: 'method', + parameterNames: [], + className: 'TestCls', + descriptor: makeDescriptor(original), + ...overrides, + }; +}; + +describe('attachHooks', () => { + it('runs the original method and returns its result when hooks are empty', () => { + const original = vi.fn((...args: unknown[]) => (args[0] as number) + 1); + const thisArg = {}; + const args = [2] as unknown[]; + const context = makeContext({ args, target: thisArg }); + + const run = attachHooks(original, thisArg, args, context, {}); + expect(run()).toBe(3); + expect(original).toHaveBeenCalledWith(2); + }); + + it('applies onReturn on sync success', () => { + const original = () => 'a'; + const context = makeContext(); + const run = attachHooks(original, {}, [], context, { + onReturn: ({ result }) => `${result}b`, + }); + expect(run()).toBe('ab'); + }); + + it('applies onError when the original throws', () => { + const original = () => { + throw new Error('fail'); + }; + const context = makeContext(); + const run = attachHooks(original, {}, [], context, { + onError: () => 'recovered', + }); + expect(run()).toBe('recovered'); + }); + + it('calls finally on sync success', () => { + const finallyHook = vi.fn(); + const original = () => 1; + const context = makeContext(); + const run = attachHooks(original, {}, [], context, { finally: finallyHook }); + expect(run()).toBe(1); + expect(finallyHook).toHaveBeenCalledTimes(1); + expect(finallyHook).toHaveBeenCalledWith(context); + }); + + it('resolves async path through chainAsyncHooks', async () => { + const original = () => Promise.resolve(10); + const context = makeContext(); + const run = attachHooks(original, {}, [], context, { + onReturn: ({ result }: OnReturnContext) => result * 2, + }); + await expect(run()).resolves.toBe(20); + }); +}); + +describe('wrapFunction', () => { + it('binds this and args like a decorated method', () => { + class C { + suffix = '!'; + } + const original = function (this: C, name: string) { + return `${name}${this.suffix}`; + }; + const descriptor = makeDescriptor(original as (...args: unknown[]) => unknown); + const wrapped = wrapFunction( + original as (...args: unknown[]) => unknown, + ['name'], + 'greet', + descriptor, + {}, + ); + + const instance = new C(); + expect(wrapped.call(instance, 'hi')).toBe('hi!'); + }); + + it('builds argsObject from parameter names', () => { + let seen: HookContext | undefined; + const original = () => 0; + const descriptor = makeDescriptor(original); + const wrapped = wrapFunction(original, ['a', 'b'], 'm', descriptor, { + onInvoke: (ctx) => { + seen = ctx; + }, + }); + + wrapped.call({}, 1, 2); + expect(seen?.argsObject).toEqual({ a: 1, b: 2 }); + expect(seen?.parameterNames).toEqual(['a', 'b']); + expect(seen?.propertyKey).toBe('m'); + }); + + it('runs async onInvoke then the original', async () => { + const order: string[] = []; + const original = () => { + order.push('original'); + return 1; + }; + const descriptor = makeDescriptor(original); + const wrapped = wrapFunction(original, [], 'm', descriptor, { + onInvoke: async () => { + order.push('onInvoke'); + }, + }); + + await wrapped.call({}); + expect(order).toEqual(['onInvoke', 'original']); + }); + + it('uses hooks factory with per-call context', () => { + const original = (n: number) => n; + const descriptor = makeDescriptor(original as (...args: unknown[]) => unknown); + const factory = vi.fn((_ctx: HookContext): EffectHooks => ({ + onReturn: ({ result }: OnReturnContext) => result + 1, + })); + const wrapped = wrapFunction( + original as (...args: unknown[]) => unknown, + ['n'], + 'm', + descriptor, + factory, + ); + + expect(wrapped.call({}, 5)).toBe(6); + expect(factory).toHaveBeenCalledTimes(1); + expect(factory.mock.calls[0]?.[0]?.args).toEqual([5]); + }); +});