Skip to content

Commit

Permalink
Use __proto__ if available.
Browse files Browse the repository at this point in the history
  • Loading branch information
RubenVerborgh committed Jan 19, 2013
1 parent ccb2ac8 commit 514dd6c
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions lib/chai/utils/addChainableMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var transferFlags = require('./transferFlags');
* Module variables
*/

// This method will need to add properties to a function.
// Without `__proto__` support, this module will need to add properties to a function.
// However, some Function.prototype methods cannot be overwritten,
// and there seems no easy cross-platform way to detect them (@see chaijs/chai/issues/69).
var excludeNames = /^(?:length|name|arguments|caller)$/;
Expand Down Expand Up @@ -60,14 +60,20 @@ module.exports = function (ctx, name, method, chainingBehavior) {
return result === undefined ? this : result;
};

// Re-enumerate every time to better accomodate plugins.
var asserterNames = Object.getOwnPropertyNames(ctx);
asserterNames.forEach(function (asserterName) {
if (!excludeNames.test(asserterName)) {
var pd = Object.getOwnPropertyDescriptor(ctx, asserterName);
Object.defineProperty(assert, asserterName, pd);
}
});
// Use `__proto__` if available
if ('__proto__' in assert) {
assert.__proto__ = this;
}
// Otherwise, redefine all properties (slow!)
else {
var asserterNames = Object.getOwnPropertyNames(ctx);
asserterNames.forEach(function (asserterName) {
if (!excludeNames.test(asserterName)) {
var pd = Object.getOwnPropertyDescriptor(ctx, asserterName);
Object.defineProperty(assert, asserterName, pd);
}
});
}

transferFlags(this, assert);
return assert;
Expand Down

5 comments on commit 514dd6c

@domenic
Copy link
Contributor

@domenic domenic commented on 514dd6c Feb 9, 2013

Choose a reason for hiding this comment

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

Ugh, this caused chaijs/chai-as-promised#18 due to the function asserters no longer having apply and call methods:

> chai.expect("foo").to.contain.apply
undefined

@RubenVerborgh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Should I add call and apply to the prototype?

@domenic
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really sure. It's a bit of an edge case, but a pretty hairy one. Maybe adding most of Function.prototype manually would be a good idea, e.g.

assert.__proto__ = Object.create(this);
assert.__proto__.call = Function.prototype.call;
// etc.

@RubenVerborgh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created pull request #140 for this. Could you help debate the necessity of this patch there?

BTW, are you sure that the modification on line 326 was necessary?
My tests for #140 seem to indicate otherwise, but this might be a test error then.

@RubenVerborgh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fix for #140 has been merged.

Please sign in to comment.