Docs: add details about (not) stubbing non-existing own properties

[mroderick]

This should help remove frustration for end users and attract fewer
GitHub issues.

Once we are happy with the details in the changes to release-source, I'll update the PR to also update the existing releases.

[fatso83]

Hmm ... has this changed without me noticing? I thought we at some point prevented stubbing properties that were higher on the prototype chain, but an experiment seems this isn't happening. This makes the error a bit weird. Then we should just throw "Cannot stub non-existing property". I might be missing something though.

function SuperAPI(){}
SuperAPI.prototype.aMethod = function (){ console.log('super'); }

var myAPI = Object.create(SuperAPI.prototype);
console.log(Object.getOwnPropertyNames(myAPI)) // => '[]'

stub = sinon.stub(myAPI, 'aMethod'); // no error
console.log(Object.getOwnPropertyNames(myAPI)) // => '["aMethod"]'

[mroderick]

Yours is a good example, this is confusing behaviour, not to say the least that it creates local properties, that will shadow the properties in the prototype.

if (object && typeof property !== "undefined" && !Object.prototype.hasOwnProperty.call(object, property)) {

Changed in b4a3f42 to

if (object && typeof property !== "undefined" && !(property in object)) {

I think we need some better tests for the expected behaviour around stubbing non-existent own properties ... and a fix for this bug.

Do you want to create an issue yourself?

[fatso83]

Ah, that's it. It used to work to stub stuff on the prototype, then it didn't (for sandboxes), and then the regression was fixed (by me - I forgot 😄). But I am not sure about the bug. What is the bug; lack of documentation? Or do you think sinon should run up the prototype until it finds the prop somewhere? I am not sure what the confusing behavior is, just that the error is confusing.

[mroderick]

I think the first implementation was the correct one, where we used hasOwnProperty. If you want to stub something on a prototype, then you should target the prototype. Just running up the prototype chain and replacing methods is likely to have unforeseen consequences.

I think we should revert to using hasOwnProperty.

[fatso83]

@mroderick There is something very weird that has been going on, and I think I am guilty of whatever has been taking place, but I am not totally sure what has been going on. See the time-related, but totally opposing PR of #1516 and #1557. To me it seems we in a matter of a month inversed the support for stubbing on the prototype, though the intent was to align the support for normal stubs and sandboxed stubs. What was supported => removed support, and vice versa.

Am I right? I got totally confused by reading the discussions, as they went a bit back and forth.

• Sinon no longer throws when stubbing non-existent properties #1508 (comment)
• Sandbox throws error 'cannot stub non-existent own property' #1537 (comment)

No matter which solution we end up with, it will make some people annoyed and end up in new bug reports, as it will revert some functionality (again) that worked, then didn't, then did ... Oh, well. Time for a new major anyway.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.