Skip to content

Commit

Permalink
Bug Fixed: Missing this context in method calls
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrey Goncharov committed Apr 8, 2019
1 parent 99e3d43 commit 83cd8bb
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 11 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "class-logger",
"version": "1.0.2",
"version": "1.0.3",
"description": "Boilerplate-free decorator-based class logging",
"keywords": [
"decorator",
Expand Down
32 changes: 27 additions & 5 deletions src/class-wrapper.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe(ClassWrapperService.name, () => {
})

describe('wrapFunction', () => {
const logsSuccess = async (fn: jest.Mock, fnRes: any) => {
const logsSuccess = async (fn: jest.Mock, fnRes: any, ctx?: any) => {
const config = ConfigService.config
const className = 'Test'
const propertyName = 'test'
Expand All @@ -73,8 +73,14 @@ describe(ClassWrapperService.name, () => {
const spyLogError = jest.spyOn(config, 'logError')

const classWrapperService: any = new ClassWrapperService()
const fnWrapped = classWrapperService.wrapFunction(config, fn, className, propertyName, classInstance)
const fnWrappedRes = await fnWrapped(...argsTest)
const fnWrapped: (...args: any) => any = classWrapperService.wrapFunction(
config,
fn,
className,
propertyName,
classInstance,
)
const fnWrappedRes = await fnWrapped.apply(ctx, argsTest)
expect(fnWrappedRes).toBe(fnRes)
expect(fn).toBeCalledTimes(1)
expect(fn).toBeCalledWith(...argsTest)
Expand Down Expand Up @@ -121,7 +127,13 @@ describe(ClassWrapperService.name, () => {
const spyLogError = jest.spyOn(config, 'logError')

const classWrapperService: any = new ClassWrapperService()
const fnWrapped = classWrapperService.wrapFunction(config, fn, className, propertyName, classInstance)
const fnWrapped: (...args: any[]) => any = classWrapperService.wrapFunction(
config,
fn,
className,
propertyName,
classInstance,
)
const fnWrappedPromise = (async () => fnWrapped(...argsTest))()
await expect(fnWrappedPromise).rejects.toThrow(error)
expect(fn).toBeCalledTimes(1)
Expand Down Expand Up @@ -164,6 +176,16 @@ describe(ClassWrapperService.name, () => {
})
await logError(fn, error)
})
test('preserves this context', async () => {
const fnRes = Symbol()
const ctx = {}
// tslint:disable-next-line only-arrow-functions
const fn = jest.fn(function(this: any) {
expect(this).toBe(ctx)
return fnRes
})
await logsSuccess(fn, fnRes, ctx)
})
})

describe('asynchronous target function', () => {
Expand Down Expand Up @@ -268,7 +290,7 @@ describe(ClassWrapperService.name, () => {
})
expect(spyWrapClassInstance).toBeCalledTimes(1)
})
test("wraps class and doesn't log its construction", () => {
test("wraps class and doesn't log its construction", () => {
const config: IClassLoggerConfig = {
include: {
construct: false,
Expand Down
4 changes: 2 additions & 2 deletions src/class-wrapper.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class ClassWrapperService {
const classWrapper = this
// Use non-arrow function to allow dynamic context
// tslint:disable-next-line only-arrow-functions
return function(...args: any[]) {
return function(this: any, ...args: any[]) {
const messageStart = config.formatter.start({
args,
classInstance,
Expand Down Expand Up @@ -56,7 +56,7 @@ export class ClassWrapperService {
}

try {
const res = fn(...args)
const res = fn.apply(this, args)
if (classWrapper.isPromise(res)) {
res
.then((result: any) => {
Expand Down
11 changes: 9 additions & 2 deletions src/formatter.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe(ClassLoggerFormatterService.name, () => {
prop3: valTestClassProp3,
})
const dataStart: IClassLoggerFormatterStartData = {
args: ['test', Symbol(), { test: '123' }],
args: ['test', Symbol(), { test: '123' }, undefined],
classInstance: new TestClass(),
className: 'ClassNameTest',
include: {
Expand Down Expand Up @@ -93,7 +93,7 @@ describe(ClassLoggerFormatterService.name, () => {
test('returns stringified args', () => {
const argsStr = (classLoggerFormatterService as any).args(dataStart)
expect(argsStr).toBe(
`. Args: [${dataStart.args[0]}, ${dataStart.args[1].toString()}, ${stringify(dataStart.args[2])}]`,
`. Args: [${dataStart.args[0]}, ${dataStart.args[1].toString()}, ${stringify(dataStart.args[2])}, undefined]`,
)
})
})
Expand All @@ -115,6 +115,13 @@ describe(ClassLoggerFormatterService.name, () => {
const resStr = (classLoggerFormatterService as any).result(dataEnd)
expect(resStr).toBe(`. Res: ${dataEnd.result}`)
})
test('returns undefined', () => {
const resStr = (classLoggerFormatterService as any).result({
...dataEnd,
result: undefined,
})
expect(resStr).toBe(`. Res: undefined`)
})
test('returns stringified object result', () => {
const resultObj = {
test: 42,
Expand Down
6 changes: 5 additions & 1 deletion src/formatter.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface IClassLoggerIncludeConfig {

export class ClassLoggerFormatterService implements IClassLoggerFormatter {
protected readonly placeholderNotAvailable = 'N/A'
protected readonly placeholderUndefined = 'undefined'

public start(data: IClassLoggerFormatterStartData) {
let message = this.base(data)
Expand Down Expand Up @@ -92,14 +93,17 @@ export class ClassLoggerFormatterService implements IClassLoggerFormatter {
return stringify(classInsanceFiltered)
}
protected valueToString(res: any): string {
if (res === undefined) {
return this.placeholderUndefined
}
if (typeof res !== 'object') {
return res.toString()
}
if (res instanceof Error) {
res = this.errorFormat(res)
}
if (Array.isArray(res)) {
const arrayWithStringifiedElements = res.map(this.valueToString)
const arrayWithStringifiedElements = res.map(this.valueToString.bind(this))
return `[${arrayWithStringifiedElements.join(', ')}]`
}
return stringify(res)
Expand Down

0 comments on commit 83cd8bb

Please sign in to comment.