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

.unwrap() does not restore prototype methods correcty #6

Open
overlookmotel opened this issue Jun 26, 2016 · 2 comments
Open

.unwrap() does not restore prototype methods correcty #6

overlookmotel opened this issue Jun 26, 2016 · 2 comments

Comments

@overlookmotel
Copy link

overlookmotel commented Jun 26, 2016

If you wrap an object method which is inherited from the object's prototype, and then .unwrap(), it doesn't remove the object's own method. So the object is left in a different state from how it began.

function C() {}
C.prototype.myMethod = function() { return 'A'; };

var c = new C();

console.log( Object.keys(c).length ); // outputs 0

shimmer.wrap(c, 'myMethod', function(_myMethod) {
    return function() {
       return _myMethod() + 'X';
    };
});

shimmer.unwrap(c, 'myMethod');

console.log( Object.keys(c).length ); // outputs 1

c.myMethod has been restored to its original value, but it's now present on the object, rather than inherited from the prototype.

Now, if you make a change to the prototype, it's not reflected in the behavior of the object method.

C.prototype.myMethod = function() { return 'B'; };
console.log( c.myMethod() ); // still outputs 'A'

The docs for .unwrap() say it is "for restoring the function back the way it was before you started". This is a bit ambiguous, but if you read this as "restoring everything back the way it was" then the current behavior is incorrect.

Perhaps this is a rather small point, but it bit me on something, so I thought I'd raise it.

@overlookmotel
Copy link
Author

Any interest in a PR to fix this?

@overlookmotel
Copy link
Author

Now that this module is a bit active again, would you accept a PR for this?

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

No branches or pull requests

1 participant