-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Conversation
For testing, it can be useful to run with other promise implementations like Bluebird.js + angular-bluebird-promises. This broke in angularjs 1.6 with a "TypeError: Cannot set property 'pur' of undefined". Closes angular#16164
@vbraun - thanks for sending this in. |
I'm using kriskowal's Q with jsdom for some tests, and I can confirm that it works with that combination. On #16164 the reporter indicated that it works with Bluebird but I haven't confirmed that personally. Not sure about unit tests, of course one could try various third-party libraries but would be quite some effort... Or just manually delete the q.$$state? |
We decided it is not worth adding tests for the specific issue that this PR solves, since substituting We can land this as is (with a comment explaining what these lines guard against). |
@@ -677,10 +677,11 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { | |||
} | |||
|
|||
function isStateExceptionHandled(state) { | |||
if (!state) return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm...I was going through the code looking for other places that might need fixing and I realized I am not sure why this should be necessary (i.e. where this function would be called from when using a 3rd-party promise library) 😕
Could you share a minimal demo (e.g. plnkr, repo, or live app (with non-minified sources 😁)) that showcases the current failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this change is indeed not necessary (relevant discussion).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share a minimal demo (e.g. plnkr, repo, or live app (with non-minified sources 😁)) that showcaes the current failures?
We are also using using Bluebird.js + angular-bluebird-promises. Setting ngModelOptions raises the TypeError: Cannot set property 'pur' of undefined. Please find a demo here http://plnkr.co/edit/9oDzLt?p=preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for the reproduction, @davidwilder. Based on the stack trace, the error happens in the markQStateExceptionHandled()
method and not isStateExceptionHandled()
. As mentioned in #16164 (comment), I don't think isStateExceptionHandled()
needs to be changed (but it won't hurt either).
Tweaked a bit and merged. BTW, if anyone can't upgrade to the latest AngularJS version, it is possible to work around this issue by adding a .config(Bluebird => Bluebird.prototype.$$state = {}) |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
For testing, it can be useful to run with other promise
implementations like Bluebird.js + angular-bluebird-promises. This
broke in angularjs 1.6 with a "TypeError: Cannot set property 'pur' of
undefined".
Closes #16164
What is the current behavior? (You can also link to an open issue here)
Raises TypeError: Cannot set property 'pur' of undefined
What is the new behavior (if this is a feature change)?
N/A
Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information: