andreypopp / autobind-decorator

Decorator to automatically bind methods to class instances
MIT License
1.45k stars 66 forks source link

1.3.1 build appears to be broken #12

Closed joshschumacher closed 9 years ago

joshschumacher commented 9 years ago

I can't find anything in the source code that would be generating the code that is breaking the current package build so maybe the code was manually modified before it was pushed to npm?

When you npm install autobind-decorator, line 39 of node_modules/autobind-decorator/lib/index.js contains this line: console.log(Object.getOwnPropertyNames(), Object.getOwnProperySymbols()); which blows up because both of those Object.* calls expect an argument.

Not seeing this console.log in the source code on GitHub but it's coming down in the package source when you npm install.

Here's the full boundClass method in the npm install'ed code:

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

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

    // Only methods need binding
    if (typeof descriptor.value === 'function') {
      Object.defineProperty(target.prototype, key, boundMethod(target, key, descriptor));
    }
  });
  return target;
}
andreypopp commented 9 years ago

Indeed, fixed in 1.3.2. Thanks for reporting.

domarmstrong commented 9 years ago

Big appology for that. Would running the tests against lib instead of src be an idea, just compile pre test?

andreypopp commented 9 years ago

@domarmstrong good idea! I left test to run tests against sources as it simplifies development but added build-test which runs before release-* and is against built sources.

joshschumacher commented 9 years ago

Thanks!