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

Spying on Getters and Setters post v2 #1392

Closed
lucasfcosta opened this issue May 3, 2017 · 1 comment
Closed

Spying on Getters and Setters post v2 #1392

lucasfcosta opened this issue May 3, 2017 · 1 comment
Labels
Property accessors Property Getters/Setters

Comments

@lucasfcosta
Copy link
Member

The Problem

I was taking a look at #1205, and @muuki88 has noticed that we don't have a way to spy on getters and setters without having deprecation messages thrown at us.

The Cause

Before v2 we were just using wrapMethod and passing it a third argument that could be a descriptor.
Right now we are still calling wrapMethod when we pass a descriptor object as the third argument to the spy function (as you can see at this line).

My Suggestion

We should have a way to spy on descriptors since they can trigger important behaviors for objects and the way we currently have to work aroung this by using getOwnPropertyDescriptor().get and then passing it to stub().get is too hackish.

I think that in order to keep this API consistent with the stub api we must either do it the same way (which I think is bad) or spy on getters and setters by default. I think that by adding an extra argument we just end up going full-circle and doing the same thing we've just deprecated and by having another call after creating a spy it gets too cumbersome (this last argument is just based in my opinion lol).

I'd love to hear what other people have to say so that we can discuss and implement this if the maintainers agree it's needed.

If I missed something let me know, otherwise I'll be more than happy to implement changes.

@mantoni
Copy link
Member

mantoni commented May 27, 2017

Can this be closed now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Property accessors Property Getters/Setters
Projects
None yet
Development

No branches or pull requests

4 participants