Implement using Proxy

[frangio]

Fixes #29. Fixes #57.

This implementation changes the behavior when the wrapped object is mutated. It will act as a promisified "view" of the underlying object, rather than a promisified clone. Thus, if properties (including methods) change in the underlying object, the promisified view will also be updated. This change is reflected in a change in one of the existing tests.

I also fixed a bug where symbol properties were incorrectly handled, although that's unrelated to using Proxy.

Regarding this issue in #32:

[[Get]] for proxy objects enforces the following invariants: The value reported for a property must be the same as the value of the corresponding target object property if the target object property is a non-writable, non-configurable own data property.

As far as I could tell, addressing this is complicated because you need to get the property descriptor which may be inherited, and there is no native API to do that so you have manually walk the prototype chain.

Where should we document the caveat that pify will not work in this case?

---

IssueHunt Summary

Referenced issues

This pull request has been submitted to:

• #29: Return ES2015 Proxy object

---

IssueHunt has been backed by the following sponsors. Become a sponsor

[sindresorhus]

Thanks for working on this 🙌

As far as I could tell, addressing this is complicated because you need to get the property descriptor which may be inherited, and there is no native API to do that so you have manually walk the prototype chain.

Would be nice to just handle this though. Imagine using pify on an object from a package you don't control, then that package makes a property non-writable in a patch release, and suddenly pify is crashing. While we could document it, it's hard for users to protect themselves against it.

See sindresorhus/on-change#16 for how it was handled on another package of mine.

target object property is a non-writable, non-configurable own data property.

Doesn't the "own" word imply it doesn't apply when inherited?

[sindresorhus]

Use t.notThrows instead.

[sindresorhus]

I think this also fixes:

• losing bound this in promise #63
• feature: all bind-all when passed an object #48

Can you add some more tests?

[frangio]

Doesn't the "own" word imply it doesn't apply when inherited?

Seems so! I totally missed that.

[frangio]

I think #63 was fixed in #69. However, it's good that you brough it up because this PR breaks bind when pify is used on a function, because it's also promisifying the function's methods. So pify(fn).bind is actually a promisified version of bind, and pify(fn).bind(obj) is not a function but a promise, and throws an error if it's called as a function.

To fix this I can add an additional condition that we will not promisify a property if its value is taken from Function.prototype. What do you think about this? It feels ad-hoc but since the library is meant to be used with functions in this way, I think it's justified.

---

#48 is fixed-ish. The problem is that a method called through the pify proxy will receive as this the proxy and not the underlying object, and this will likely break the object because internally it doesn't expect to have promisified methods. I think this can be fixed by switching the proxy for its target in apply, but it has to be done carefully to not break bind. I will look into this.

[sindresorhus]

To fix this I can add an additional condition that we will not promisify a property if its value is taken from Function.prototype. What do you think about this? It feels ad-hoc but since the library is meant to be used with functions in this way, I think it's justified.

👍

I think this can be fixed by switching the proxy for its target in apply, but it has to be done carefully to not break bind. I will look into this.

👍

[frangio]

@sindresorhus Can you please clarify what tests you’d like me to add?

[frangio]

I've tackled the remaining issues mentioned, and added tests for those things. I feel that the existing test suite is quite comprehensive. If you have concrete tests you would like me to add let me know though.

[sindresorhus]

Looks good now. Great work 👌