Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return the original response in @exec-time #168

Open
wants to merge 3 commits into
base: 2.11.0
Choose a base branch
from
Open

Return the original response in @exec-time #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b08ce9b
fix(execTime): Return the original method's response
masoud-msk Sep 30, 2024
1d76e3a
fix(utils): Improve isPromise to reflect the specs
masoud-msk Sep 30, 2024
006f895
Merge branch 'master' into fix-exec-time
vlio20 Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,9 @@
<div class="ud-section-content">
<div class="ud-subtitle">Description:</div>
<p class="ud-description">
Measures the time that takes for the decorated method to response.
By default will log the result using <i>console.info()</i> but this can be changed by providing your own
Measures the time that takes for the decorated method to respond.
Also returns the original response.
By default, will log the result using <i>console.info()</i> but this can be changed by providing your own
reporter
function.
</p>
Expand Down
2 changes: 1 addition & 1 deletion src/common/utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export function isPromise(obj: any): obj is Promise<any> {
return !!(obj && obj.then !== undefined);
return !!(obj && typeof obj.then === 'function');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this had to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, to exactly reflect the specs.
Secondly, the checked variable was used with await keyword and it doesn't error on a non-Promise but I changed the code to call the .then method on the variable and the runtime throws if it's not a valid Promise. The reason for this change is that I think it's impossible to implement otherwise.

For clarification see this:

const o1 = { then: func => func(42) }
const o2 = { then: 42 }

await o1 // 42
await o1 // { then: 42 }

o1.then(console.log) // 42
o2.then(console.log) // Uncaught TypeError: o2.then is not a function

P.S. Maybe it's better to write test for this utility function.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind to write this utility function as you suggested?

}

export function sleep(n: number): Promise<void> {
Expand Down
14 changes: 9 additions & 5 deletions src/exec-time/exec-time.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ describe('exec-time', () => {
}

const t = new T();
t.foo('a');
const returned = t.foo('a');

expect(reporter).toHaveBeenCalledTimes(1);
const args: ExactTimeReportData = reporter.mock.calls[0][0];
expect(args.args).toEqual(['a']);
expect(returned).toEqual('ab');
expect(args.result).toEqual('ab');
expect(args.execTime).toBeGreaterThanOrEqual(0);
expect(args.execTime).toBeLessThan(10);
Expand All @@ -54,11 +55,12 @@ describe('exec-time', () => {
}

const t = new T();
await t.foo('a');
const returned = await t.foo('a');

expect(reporter).toHaveBeenCalledTimes(1);
const args: ExactTimeReportData = reporter.mock.calls[0][0];
expect(args.args).toEqual(['a']);
expect(returned).toEqual('ab');
expect(args.result).toEqual('ab');
expect(args.execTime).toBeGreaterThanOrEqual(8);
expect(args.execTime).toBeLessThan(20);
Expand All @@ -75,14 +77,15 @@ describe('exec-time', () => {
}

const t = new T();
await t.foo('a');
const returned = await t.foo('a');
expect(logSpy).toHaveBeenCalledTimes(1);
const clogSpyArgs = logSpy.mock.calls[0][0];
expect(clogSpyArgs).toBeGreaterThanOrEqual(0);
expect(returned).toEqual('ab');
logSpy.mockRestore();
});

it('should make sure that the reporter is called when provided as sting', async () => {
it('should make sure that the reporter is called when provided as string', async () => {
class T {
goo = jest.fn();

Expand All @@ -95,11 +98,12 @@ describe('exec-time', () => {
}

const t = new T();
await t.foo('a');
const returned = await t.foo('a');

expect(t.goo).toHaveBeenCalledTimes(1);
const args: ExactTimeReportData = t.goo.mock.calls[0][0];
expect(args.args).toEqual(['a']);
expect(returned).toEqual('ab');
expect(args.result).toEqual('ab');
expect(args.execTime).toBeGreaterThanOrEqual(8);
expect(args.execTime).toBeLessThan(20);
Expand Down
18 changes: 14 additions & 4 deletions src/exec-time/exec-timify.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,32 @@ const reporter: ReportFunction = function (data: ExactTimeReportData): void {
export function execTimify<D = any, A extends any[] = any[]>(
originalMethod: Method<D, A> | AsyncMethod<D, A>,
arg?: ReportFunction | string,
): AsyncMethod<void, A> {
): typeof originalMethod {
vlio20 marked this conversation as resolved.
Show resolved Hide resolved
const input: ReportFunction | string = arg ?? reporter;

return async function (...args: A): Promise<void> {
return function (...args: A) {
vlio20 marked this conversation as resolved.
Show resolved Hide resolved
const repFunc: ReportFunction = typeof input === 'string' ? this[input].bind(this) : input;
const start = Date.now();
let result = originalMethod.apply(this, args);
const result = originalMethod.apply(this, args);

if (isPromise(result)) {
result = await result;
return result.then(resolved => {
repFunc({
args,
result: resolved,
execTime: Date.now() - start,
});

return resolved;
});
}

repFunc({
args,
result,
execTime: Date.now() - start,
});

return result;
};
}
Loading