Skip to content

Commit

Permalink
fix(bindAll): fix constructor not being called with new
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Properties will use the bind decorator to apply getter/setters
on the prototype instead of the instance. This could cause
issues with consumers currently using the implementain that
assigns the bound properties to the instance through the constructor
rather than on the prototype. The value on the prototype will now be
a getter instead of the original function value.
  • Loading branch information
steelsojka committed Jun 17, 2018
1 parent 17caeb6 commit 4e72d0c
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/bindAll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('bindAll', () => {

const myClass = new MyClass();

expect(myClass.hasOwnProperty('fn')).to.be.true;
expect(accessed).to.be.false;
expect(myClass.hasOwnProperty('prop')).to.be.false;
expect(myClass.hasOwnProperty('prop2')).to.be.false;
});
Expand Down
28 changes: 10 additions & 18 deletions src/bindAll.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import isFunction = require('lodash/isFunction');

import { copyMetadata, wrapConstructor } from './utils';
import { InstanceChainMap } from './factory';
import { Bind } from './bind';

/**
* Binds methods of an object to the object itself, overwriting the existing method.
Expand All @@ -27,24 +27,22 @@ import { InstanceChainMap } from './factory';
* myClass.unbound.call(null); // => MyClass {}
*/
export function BindAll(methods: string[] = []): ClassDecorator {
return (target: Function): Function => {
return wrapConstructor(target, function(Ctor: Function, ...args: any[]) {
bindAllMethods(target, this, methods);

Ctor.apply(this, args);
});
return (target: any) => {
bindAllMethods(target, methods);
};
}

function bindAllMethods(target: Function, instance: any, methods: string[] = []): void {
function bindAllMethods(target: Function, methods: string[] = []): void {
const targetProto = target.prototype;
let proto = target.prototype;
const boundKeys: string[] = [];

while (proto && proto !== Object.prototype) {
for (const key of Object.getOwnPropertyNames(proto)) {
const include = methods.length ? methods.indexOf(key) !== -1 : true;
const descriptor = Object.getOwnPropertyDescriptor(proto, key);

if (include && key !== 'constructor' && !instance.hasOwnProperty(key)) {
if (include && key !== 'constructor') {
// If this property is a getter and it's NOT an instance decorated
// method, ignore it. Instance decorators are getters until first accessed.
if (descriptor.get) {
Expand All @@ -55,15 +53,9 @@ function bindAllMethods(target: Function, instance: any, methods: string[] = [])
}
}

const value = instance[key];

if (isFunction(value)) {
Object.defineProperty(instance, key, {
configurable: true,
enumerable: descriptor.enumerable,
value: copyMetadata(value.bind(instance), value),
writable: descriptor.writable
});
if (isFunction(proto[key]) && boundKeys.indexOf(key) === -1) {
Object.defineProperty(targetProto, key, Bind(proto, key, descriptor)!);
boundKeys.push(key);
}
}
}
Expand Down

0 comments on commit 4e72d0c

Please sign in to comment.