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

Stub doesn't return the correct value when used with new #1676

Closed
Rhysjc opened this issue Jan 26, 2018 · 8 comments
Closed

Stub doesn't return the correct value when used with new #1676

Rhysjc opened this issue Jan 26, 2018 · 8 comments

Comments

@Rhysjc
Copy link

Rhysjc commented Jan 26, 2018

  • Sinon version : 4.2.1
  • Environment : Node 6.10.3

What did you expect to happen?
Stub to return the correct value when used with new.

What actually happens
Returns a function

How to reproduce

const stub = sinon.stub().returns('a value');
console.log(stub()); // 'a value'
console.log(new stub()); // 'functionStub {}'
@mantoni mantoni added Bug semver:patch changes will cause a new patch version labels Jan 26, 2018
@mantoni
Copy link
Member

mantoni commented Jan 26, 2018

In case you want to scratch your own itch, here is the piece of code where I would start looking into it:

sinon/lib/sinon/spy.js

Lines 197 to 204 in a8171c3

if (thisCall.calledWithNew()) {
// Call through with `new`
returnValue = new (bind.apply(this.func || func, [thisValue].concat(args)))();
if (typeof returnValue !== "object") {
returnValue = thisValue;
}
} else {

@lo1tuma
Copy link
Contributor

lo1tuma commented Feb 20, 2018

I can reproduce this. I was using a Symbol in returns but after looking at the code only objects are supported. So my workaround is to use an empty object instead of a symbol to indicate the the correct value is passed through some calls.

@LRagji
Copy link

LRagji commented Mar 18, 2020

Is there a way around this since "new" keyword is like default with ES6 can it support stubs themselves with objects..
EG:
const returnStub = sinon.stub(); const stub = sinon.stub().returns(returnStub);

@fatso83
Copy link
Contributor

fatso83 commented Mar 18, 2020

since "new" keyword is like default with ES6 can it support stubs themselves with objects..

I am sorry, but I do not understand what this means. Can you elaborate on what you mean? I especially do not understand what you mean by saying "new is like default with ES6".

@LRagji
Copy link

LRagji commented Mar 18, 2020

Sorry i wrote in jiffy, I was trying to do following

const instanceStub = sinon.stub(); 
const type = sinon.stub();
type.returns(instanceStub);
const instance = new type();
console.log(instance); //prints proxy function, expecting instanceStub

The above failed if instanceStub was a Stub but if instanceStub is normal object then it passed.

@fatso83
Copy link
Contributor

fatso83 commented Mar 19, 2020

I am guessing returnStub is supposed to be instanceStub? In any case, seeing that this issue is over 2 years old, I think it is quite likely that this will only happen when someone that is affected takes a stab at doing it ... Max gave some good hints at where to start a change. We are very friendly and accept pull requests 😺

Here's how to start

@LRagji
Copy link

LRagji commented Mar 19, 2020

Corrected code, ohh i saw the date now 🙂

@fatso83
Copy link
Contributor

fatso83 commented Aug 9, 2023

The only issue I really see here is that we should perhaps be stricter about what we accept as a return value when newing objects. The only meaningful thing to return when invoking with new is an object, so the current implementation is correct when forcing it to be the thisValue.

TBH, I think an improvement would be to throw with a meaningful error message when encountering return values that are not objects, as that does not make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants