Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore the call and apply methods when adding a chainable method #140

Merged
merged 1 commit into from
Feb 15, 2013
Merged

Restore the call and apply methods when adding a chainable method #140

merged 1 commit into from
Feb 15, 2013

Conversation

RubenVerborgh
Copy link
Contributor

@domenic found that properties of Function.prototype where not present anymore on chainable methods after __proto__ support was added to speed things up.

This pull request fixes this. However, I would like to debate first whether it should be added.
Below are my arguments.

pro

  • It makes chai behave identically on all platforms, regardless of the presence of __proto__.
  • It fixes the bug this caused in chai as promised (Tests failures chai-as-promised#18).

contra

  • The bug in chai as promised has already been fixed.
  • chai as promised is probably the only plugin affected. [citation needed]
  • It makes things more complicated.

Note that the associated commit currently only adds call and apply, and not others such as length, arguments, name, caller, bind, toString. If you think this is necessary, let me know.

Also note that the property case was never broken (i.e., the assertions on lines 235 and 236 pass without the modification). Only when the property was called as a method, things didn't go as expected.

…nable method.

This was broken on platforms that have `__proto__` support.
Details at 514dd6ce4#commitcomment-2593383
@logicalparadox
Copy link
Member

I think the first pro makes it worth it, even if we are only addressing edge cases. I don't currently see the need for the others (caller, bind, etc) but this has gone far beyond my original expectations for chainable methods.

logicalparadox added a commit that referenced this pull request Feb 15, 2013
Restore the `call` and `apply` methods when adding a chainable method
@logicalparadox logicalparadox merged commit 039d8c6 into chaijs:master Feb 15, 2013
@RubenVerborgh RubenVerborgh deleted the function-prototype branch February 15, 2013 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants