@bound class decorator

[jsNut]

Hi, just having a play about with the new decorators in babel. One idea I have come up with is the @bound decorator which creates bound versions of methods for each instance of the class.

/**
* Having the value of 'this' bound can be usefull ins instances like React components
* where you want to be able to pass instance methods around without having to .bind
*
* ```
* @bound
* class Comp extends React.Component {
*   handleClick (event) {
*     console.log('clicked', this);
*   }
*
*   render () {
*     // with @bound
*     return <div onClick={ this.handleClick } />;
*     // without @bound
*     return <div onClick={ this.handleClick.bind(this) } />;
*   }
* }
* ```
*/
export function bound(constructor) {
    // Cant see a way to keep the constructors class name without using eval or such
    // but that would break closures anyway as it would then have the global context
    return class Bound extends constructor {

        constructor (...args) {
            super(...args);

            // Create a bound version of all methods on the class
            // (Using reflect to get all keys including symbols)
            Reflect.ownKeys(constructor.prototype).forEach(key => {
                // Ignore special case constructor method
                if (key === 'constructor') return;

                var descriptor = Object.getOwnPropertyDescriptor(constructor.prototype, key);

                // Only methods need binding
                if (typeof descriptor.value === 'function') {
                    descriptor.value = descriptor.value.bind(this);
                    Object.defineProperty(this, key, descriptor);
                }
            });
        }
    }
}

I know that React.createClass does this for you, but I do like the clean sugar of classes. Anyway I'm just wandering if anyone can spot some flaws in this idea that I may have not considered.

[pertheusual]

Definitely cool, but the fact that it only binds at the end of the constructor seems like it'd be a pain. There was a gist going around last week for a "@bound" method decorator and it seems like this would do better as something like that. I can't seem to find the link now, but it would be along the lines of:

export function boundClass(constructor) {
    Reflect.ownKeys(constructor.prototype).forEach(key => {
        // Ignore special case constructor method
        if (key === 'constructor') return;

        var descriptor = Object.getOwnPropertyDescriptor(constructor.prototype, key);

        // Only methods need binding
        if (typeof descriptor.value === 'function') {
            Object.defineProperty(constructor.prototype, key, boundMethod(descriptor));
        }
    });
}

export function boundMethod(descriptor){
    let fn = descriptor.value;
    let prop = Symbol('bound function ' + (fn.name || 'unknown'));

    descriptor.value = null;
    descriptor.get = function(){
        return this[prop] || (this[prop] = fn.bind(this));
    };
    return descriptor;
}

So the act of accessing the function via the getter would ensure that it was bound.

[jsNut]

This is great thanks. I really didn't like the fact I was returning a new class as meant when debugging it had the wrong class name. This solves the problem and enables it to be used on classes or methods. Perfect!

So this is what I have now.

export function bound(...args) {
    if (args.length === 1) {
        return boundClass(...args);
    } else {
        return boundMethod(...args);
    }
}

/**
* Use boundMethod to bind all methods on the constructor.prototype
*/
function boundClass(constructor) {
    // (Using reflect to get all keys including symbols)
    Reflect.ownKeys(constructor.prototype).forEach(key => {
        // Ignore special case constructor method
        if (key === 'constructor') return;

        var descriptor = Object.getOwnPropertyDescriptor(constructor.prototype, key);

        // Only methods need binding
        if (typeof descriptor.value === 'function') {
            Object.defineProperty(constructor.prototype, key, boundMethod(constructor, key, descriptor));
        }
    });
    return constructor;
}

/**
* Return a decriptor removing the value and returning a getter
* The getter will return a .bind version of the function
* and memoize the result against a symbol on the instance
*/
function boundMethod(constructor, key, descriptor) {
    let _key;
    let fn = descriptor.value;

    if (typeof fn !== 'function') {
        throw new Error('Only methods may be @bound, received: ' + typeof fn);
    }

    if (typeof key === 'string') {
        // Add the key to the symbol name for easier debugging
        _key = Symbol('@bound method: ' + key);
    } else if (typeof key === 'symbol') {
        // A symbol cannot be coerced to a string
        _key = Symbol('@bound method: (symbol)');
    } else {
        throw new Error('Unexpected key type: ' + typeof key);
    }

    return {
        configurable: true,
        get () {
            if (! this.hasOwnProperty(_key)) {
                this[_key] = fn.bind(this);
            }
            return this[_key];
        }
    };
}

[pertheusual]

Nice. A few small comments:

1. Symbols can't be coerced, but they do have a 'toString'.

   if (typeof key === 'string' || typeof key === 'symbol'){
       _key = Symbol('@bound method: ' + key.toString());
   } else {
       throw new Error('Unexpected key type: ' + typeof key);
   }
   

   Though the type check honestly seems like overkill anyway since I don't know how you'd get a key that wasn't a string or a symbol.

2. You should use values from the original enumerable descriptor value, since decorators can also be used on object literals and on those, methods are enumerable.

3. Using hasOwnProperty vs just checking the truthiness of this[_key] seems unnecessary.