-
-
Notifications
You must be signed in to change notification settings - Fork 780
Description
Is your feature request related to a problem? Please describe.
In #1557 we removed support for stubbing undefined properties, aligning the behavior with how sandbox stubbing worked. Later on we have made the sinon methods use a default sandbox to further align how the default methods and sandboxes worked.
Still, as the thread in #1537 show, this remains a sorely missed feature for some, and while we had good reasons for removing it, maybe we could appease the ones wanting it by making it an opt-in? It remains a quite useful feature in some cases.
Describe the solution you'd like
I suggest adding a config field to the sandbox to allow defining fields (fakes) that are not originally defined by the objects we insert the fake into. Something like sinon.createSandbox({allowUndefined: true})? (Totally not sure about the actual name of the prop, though ... allowNonOwn?)
Given
var obj = {a: 1};it should allow
sinon.stub(obj, 'b').value(2); // not defined anywhere on the prototype chain
sinon.replace(obj, 'c', sinon.fake()); Related to this, but really a different case, is props on the prototype chain. As explained in the "Solution" paragraph in #2193 we deal with this differently today, depending on whether we use spies or fakes:
var parent = { foo: function() {} };
var child = Object.create(parent):
// disallowed
sinon.stub(child, 'foo');
// allowed
sinon.replace(child, 'foo', sinon.fake());
If we simplified the "undefined" and "props on an ancestor" problems into a single issue: props not defined by the object whose fields are being stubbed, we could align this behavior for our spies, stubs and fakes under a default non-permissive line and have an opt-in solution for those who want it. By default, you would then only be allowed to replace fields owned/defined by the object.
Having the opt-in for stubs and spies would not be a breaking change, but as the sinon.replace method today allows stubbing defined props owned by an ancestor it would be a breaking change to disallow this by default.
Describe alternatives you've considered
Two flags instead of one: allowUndefinedProps (props are not present anywhere on the prototype chain) and allowShadowingFieldsOnPrototype (to stub fields defined by ancestors). I don't really see the benefit, though.