Cannot stub Array.prototype.filter

[pomek]

• Sinon version : 3.1.0
• Environment: Node.js and Chrome 60.0.3112.78
• Example URL: https://gist.github.com/pomek/1f135ec4e1bbf872254a66fb9170da7b
• Other libraries you are using: Mocha

What did you expect to happen?
The tests should pass.

What actually happens

RangeError: Maximum call stack size exceeded

    at defineProperty (native)
    at Array.proxy (node_modules/sinon/lib/sinon/spy.js:69:79)
    at Function.matchingFakes (node_modules/sinon/lib/sinon/spy.js:343:35)
    at Function.invoke (node_modules/sinon/lib/sinon/spy.js:166:30)
    at Array.proxy (node_modules/sinon/lib/sinon/spy.js:69:54)
    at Function.matchingFakes (node_modules/sinon/lib/sinon/spy.js:343:35)
    at Function.invoke (node_modules/sinon/lib/sinon/spy.js:166:30)
    /* ... */   
    at Array.proxy (node_modules/sinon/lib/sinon/spy.js:69:54)
    at Function.matchingFakes (node_modules/sinon/lib/sinon/spy.js:343:35)
    at Function.invoke (node_modules/sinon/lib/sinon/spy.js:166:30)
    at Array.proxy (node_modules/sinon/lib/sinon/spy.js:69:54)
    at Context.it (packages/ckeditor5-dev-env/tests/asda.js:25:19)

How to reproduce

Just run the tests. It worked in Sinon 1.x and does not work in 2.x and 3.x.

[mroderick]

Thank you for the detailed error report 🏆

Using your test case I have been able to determine that this defect was introduced in e72fee4, which was part of #1313 which became v2.3.6.

Until this is fixed, I guess you can use v2.3.5.

[fatso83]

Looking at this line, it makes perfect sense that we get an infinite recursion when stubbing Array.prototype.filter, as we use that method internally. I suspect that would be the case for many other ES5 methods as well. The fix here would be as simple as rewriting it to a loop, or creating a filter utility method, but it leaves the question of what to do with all other methods we are using internally. Does this basically mean we have to refrain from using any of the standard library methods, as they may all be targets for stubbing? If so, this points to a bigger issue.

[mantoni]

We could alternatively add something like var filter = Array.prototype.filter; at the top of the file and use it like filter.call(array, function () {}).

[fatso83]

@mantoni : yeah, sure we could (and many other ways), but I was just saying that we should probably make this the general rule of the codebase, as any method of the standard library could be a target for stubbing. I could start by adding it to the pull request template:

All library methods ([].map, [].filter, toString, etc) needs to use cached references.

[mantoni]

@fatso83 Yes, I agree. I suggested this solution as it's already used heavily with Array.prototype.slice.

[pomek]

Thanks, guys!

[mroderick]

This has become part of v3.2.0