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

Intercepting Promise.resolve fails in enumerator.js#L82 #335

Open
codeworrior opened this issue Jul 7, 2018 · 0 comments
Open

Intercepting Promise.resolve fails in enumerator.js#L82 #335

codeworrior opened this issue Jul 7, 2018 · 0 comments

Comments

@codeworrior
Copy link
Contributor

In a testing framework that we use, Promise.resolve is intercepted (among others) to implement some bookkeeping for pending promises, timers, requests etc. The intercepting code looks something like this:

(function() {
  const originalResolve = Promise.resolve;  // *1*
  Promise.resolve = function(value) {

    // propagate the current call to the original function, 
    // using the same context and argument
    let result = originalResolve.call(this, value); // *2*

    // bookkeeping is implemented around the original call, doesn't matter here
    return result;   

  };
}();

Recently, we tried to upgrade es6-promise from 2.3.0 to 4.2.4 to benefit from Promise.prototype.finally. But then, the above intercepting code failed with a

TypeError: Constructor is not a constructor
at resolve$1 (es6-promise.js:231:17)

The reason seems to be that es6-promise's current Promise.resolve implementation relies on this (to allow inheritance), but the Enumerator.prototype._eachEntry implementation ignores this fact here in combination with line 82.

To cite from #286 (IMO applies to resolve as well):

If you wish to destructure all, it can be, but must be first bound.

Well, I understand that this applies to our test framework (line *1*) and binding originalResolve would fix the issue. But I think, the strategy of calling the originalResolve with the current this and argument (line *2*) is also fine and a bit more universal. It even should work if someone would re-use the wrapped Promise.resolve in a subclass of Promise.

IMO it would be fair if Enumerator.prototype._eachEntry either would follow the "if you wish to destructure ... it first must be bound" rule itself or would call Constructor.resolve in line 82.

What do you think?

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